- improved error handling in SystemCmd

diff --git a/snapper/SystemCmd.cc b/snapper/SystemCmd.cc
index 68982c8c0be6f5fbd02fdc47df043bc5a64ad068..d3bded76b0c94558596230a0218b173e2f1409b7 100644 (file)
--- a/snapper/SystemCmd.cc
+++ b/snapper/SystemCmd.cc
@@ -78,17 +78,6 @@ SystemCmd::~SystemCmd()
     }
 
 
-void
-SystemCmd::closeOpenFds() const
-    {
-    int max_fd = getdtablesize();
-    for( int fd = 3; fd < max_fd; fd++ )
-       {
-       close(fd);
-       }
-    }
-
-
     void
     SystemCmd::execute()
     {
@@ -123,38 +112,50 @@ SystemCmd::closeOpenFds() const
            }
        y2deb("sout:" << pfds[0].fd << " serr:" << pfds[1].fd);
 
+       const int max_fd = getdtablesize();
+
        const TmpForExec args_p(make_args());
        const TmpForExec env_p(make_env());
 
        switch( (Pid_i=fork()) )
            {
            case 0:
+           {
+               // Do not use exit() here. Use _exit() instead.
+
+               // Only use async‐signal‐safe functions here, see fork(2) and
+               // signal-safety(7).
+
                if( dup2( sout[1], STDOUT_FILENO )<0 )
-                   {
-                   y2err("dup2 stdout child failed errno:" << errno << " (" << stringerror(errno) << ")");
-                   }
+                   _exit(125);
                if( dup2( serr[1], STDERR_FILENO )<0 )
-                   {
-                   y2err("dup2 stderr child failed errno:" << errno << " (" << stringerror(errno) << ")");
-                   }
+                   _exit(125);
                if( close( sout[0] )<0 )
-                   {
-                   y2err("close child failed errno:" << errno << " (" << stringerror(errno) << ")");
-                   }
+                   _exit(125);
                if( close( serr[0] )<0 )
-                   {
-                   y2err("close child failed errno:" << errno << " (" << stringerror(errno) << ")");
-                   }
+                   _exit(125);
+
+               for (int fd = 3; fd < max_fd; ++fd)
+                   close(fd);
+
+               execvpe(args.get_values()[0].c_str(), args_p.get(), env_p.get());
 
-               closeOpenFds();
+               if (errno == ENOENT)
+                   _exit(127);
+               if (errno == ENOEXEC || errno == EACCES || errno == EISDIR)
+                   _exit(126);
+               _exit(125);
+           }
+           break;
 
-               Ret_i = execvpe(args.get_values()[0].c_str(), args_p.get(), env_p.get());
-               y2err("SHOULD NOT HAPPEN execvpe returned ret=" << Ret_i << " errno=" << errno);
-               break;
            case -1:
+           {
                Ret_i = -1;
-               break;
+           }
+           break;
+
            default:
+           {
                if( close( sout[1] )<0 )
                    {
                    y2err("close parent failed errno:" << errno << " (" << stringerror(errno) << ")");
@@ -177,8 +178,8 @@ SystemCmd::closeOpenFds() const
 
                doWait( Ret_i );
                y2mil("stopwatch " << stopwatch << " for \"" << cmd() << "\"");
-
-               break;
+           }
+           break;
            }
        }
     else

diff --git a/snapper/SystemCmd.h b/snapper/SystemCmd.h
index 88172b73f1165fc66c5b2c0da86a5d70539bb7ce..f8bcede1a24d31dcb13a886b01153e0354a0f688 100644 (file)
--- a/snapper/SystemCmd.h
+++ b/snapper/SystemCmd.h
@@ -104,7 +104,6 @@ namespace snapper
     private:
 
        void invalidate();
-       void closeOpenFds() const;
        void execute();
        bool doWait(int& Ret_ir);
        void checkOutput();
@@ -140,17 +139,14 @@ namespace snapper
        /**
         * Constructs the args for the child process.
         *
-        * Must not be called after exec since allocating the memory
-        * for the vector is not allowed then (in a multithreaded
-        * program), see fork(2) and signal-safety(7). So simply call
-        * it right before fork.
+        * Not async‐signal‐safe, see fork(2) and signal-safety(7).
         */
        TmpForExec make_args() const;
 
        /**
         * Constructs the environment for the child process.
         *
-        * Same not as for make_args().
+        * Not async‐signal‐safe, see fork(2) and signal-safety(7).
         */
        TmpForExec make_env() const;