distinguish child errors from failing to exec

Use the old pipe/cloexec trick to separate errors from the spawned
program from such that occur before execv[p]e(). Only errors on
the far side of exec() may be handled by the user.

diff --git a/src/pipestream.cpp b/src/pipestream.cpp
index 7816279..57b1dbe 100644 (file)
--- a/src/pipestream.cpp
+++ b/src/pipestream.cpp
@@ -210,22 +210,26 @@ mk_argv (const std::vector<std::string> &command)
  *              acquire one.
  *
  *  @param fd         The open file descriptor to operate on.
+ *  @param save_errno Out parameter: stores errno here after a syscall failure.
  *
  *  @returns          \c true on success, \c false otherwise (the call to
- *                    either \c open(2) or \c dup2(2) failed).
+ *                    either \c open(2) or \c dup2(2) failed), with errno
+ *                    communicated through saved_errno.
  */
 static bool
-redirect_devnull (const int fd)
+redirect_devnull (const int fd, int &save_errno)
 {
     static int nullfd = -1;
     
     errno = 0;
     if (nullfd == -1 && (nullfd = open ("/dev/null", O_RDWR)) == -1) {
+        save_errno = errno;
         return false;
     }
 
     errno = 0;
     if (dup2 (nullfd, fd) == -1) {
+        save_errno = errno;
         return false;
     }
 
@@ -248,7 +252,8 @@ inpipebuf::init_without_shell (const char *const *argv,
                                const bool path) const
 {
     FILE *pipeobj = NULL;
-    int pipefd [2];
+    int pipefd [2]; /* for reading output from the child */
+    int errfd  [2]; /* for determining a successful exec() */
     sigset_t oldmask, newmask;
 
     if (!out && !err) {
@@ -257,7 +262,8 @@ inpipebuf::init_without_shell (const char *const *argv,
     }
 
     errno = 0;
-    if (::pipe2 (pipefd, O_CLOEXEC) == -1) {
+    if (   ::pipe2 (pipefd, O_CLOEXEC) == -1
+        || ::pipe2 (errfd , O_CLOEXEC) == -1) {
         PIPE_CTOR_FAIL("pipe2");
     }
 
@@ -274,27 +280,29 @@ inpipebuf::init_without_shell (const char *const *argv,
         }
         case 0: {
             close (pipefd [0]);
+            close (errfd  [0]);
 
             fcntl (pipefd [1], F_SETFD, 0);
 
+            int save_errno = 0;
             if (!out) {
-                if (!redirect_devnull (STDOUT_FILENO)) {
-                    fprintf(stderr, "redirect_devnull/stdout: %m\n");
-                    /* XXX should we bail here? */
+                if (!redirect_devnull (STDOUT_FILENO, save_errno)) {
+                    (void)write (errfd [1], (char *)&save_errno, sizeof(save_errno));
+                    exit (EXIT_FAILURE);
                 }
             } else if (dup2 (pipefd[1], STDOUT_FILENO) == -1) {
-                fprintf(stderr, "dup2/stdout: %m\n");
-                exit(EXIT_FAILURE);
+                (void)write (errfd [1], (char *)&save_errno, sizeof(save_errno));
+                exit (EXIT_FAILURE);
             }
 
             if (!err) {
-                if (!redirect_devnull (STDERR_FILENO)) {
-                    fprintf(stderr, "redirect_devnull/stderr: %m\n");
-                    /* XXX should we bail here? */
+                if (!redirect_devnull (STDERR_FILENO, save_errno)) {
+                    (void)write (errfd [1], (char *)&save_errno, sizeof(save_errno));
+                    exit (EXIT_FAILURE);
                 }
             } else if (dup2 (pipefd[1], STDERR_FILENO) == -1) {
-                fprintf(stderr, "dup2/stderr: %m\n");
-                exit(EXIT_FAILURE);
+                (void)write (errfd [1], (char *)&save_errno, sizeof(save_errno));
+                exit (EXIT_FAILURE);
             }
 
             close (pipefd [1]);
@@ -307,22 +315,65 @@ inpipebuf::init_without_shell (const char *const *argv,
             } else {
                 execve (argv [0], const_cast <char *const *>(argv), NULL);
             }
-            exit(EXIT_FAILURE);
+
+            (void)write (errfd [1], (char *)&errno, sizeof(errno));
+            exit (EXIT_FAILURE);
             break;
         }
         default: {
-            close (pipefd [1]);
-
-            sigprocmask (SIG_SETMASK, &oldmask, NULL);
-
-            errno = 0;
-            if ((pipeobj = fdopen (pipefd [0], "r")) == NULL) {
-                PIPE_CTOR_FAIL("fdopen");
-            }
             break;
         }
     }
 
+    close (pipefd [1]);
+    close (errfd  [1]);
+
+    /*
+     * Check whether the child exec()’ed by reading from the error pipe.
+     * The call to read(2) will block, uninterruptible due to signals being
+     * blocked. If all went well, the read(2) will return zero bytes and we can
+     * ditch the error channel.
+     *
+     * Otherwise either the read(2) failed or we actually received something
+     * through the error pipe. Both cases are treated as errors and cause an
+     * exit from the ctor.
+     */
+    char buf [sizeof (errno)];
+    int ret;
+    memset (buf, 0, sizeof (buf));
+    errno = 0;
+    if ((ret = read (errfd [0], buf, sizeof (buf))) != 0) {
+        close (pipefd [0]);
+        close (errfd  [0]);
+        sigprocmask (SIG_SETMASK, &oldmask, NULL);
+        if (ret == - 1) {
+            /* read(2) failed */
+            PIPE_CTOR_FAIL("read");
+        } else {
+            /*
+             * We received data on the error channel indicating the child
+             * process never successfully exec()’ed. We grab the error code
+             * from the buffer and bail.
+             */
+            errno = *((int *)&buf[0]);
+            PIPE_CTOR_FAIL("child failed to exec()");
+        }
+    }
+
+    /*
+     * read(2) yielded zero bytes; it’s safe to use the pipe so close our end
+     * and continue.
+     */
+    close (errfd [0]);
+
+    sigprocmask (SIG_SETMASK, &oldmask, NULL);
+
+    errno = 0;
+    if ((pipeobj = fdopen (pipefd [0], "r")) == NULL) {
+        close (pipefd [0]);
+        PIPE_CTOR_FAIL("fdopen");
+    }
+
     return std::make_pair (childpid, pipeobj);
 }
 

diff --git a/test/test_pipestream.cpp b/test/test_pipestream.cpp
index 1b4bf8e..b03d6c0 100644 (file)
--- a/test/test_pipestream.cpp
+++ b/test/test_pipestream.cpp
@@ -123,9 +123,12 @@ BOOST_AUTO_TEST_SUITE(pipestream)
             ExecResult exres = ExecResult ();
             const std::string result = capture_exec (bad_command, exres);
 
-            BOOST_CHECK(exres.normal_exit);
+            BOOST_CHECK(!exres.normal_exit); /* failed to exec() */
             BOOST_CHECK(!exres.terminated_by_signal);
             BOOST_CHECK_EQUAL(result.size (), 0);
+            BOOST_CHECK_EQUAL(exres.error_message,
+                              "child failed to exec(): "
+                              "error 2 (No such file or directory)");
         }
 
         BOOST_AUTO_TEST_CASE(abspath_bad_noshell_stderr)
@@ -135,10 +138,12 @@ BOOST_AUTO_TEST_SUITE(pipestream)
             ExecResult exres = ExecResult ();
             const std::string result = capture_exec (bad_command, exres, false, true);
 
-            BOOST_CHECK(exres.normal_exit);
+            BOOST_CHECK(!exres.normal_exit); /* failed to exec() */
             BOOST_CHECK(!exres.terminated_by_signal);
-            BOOST_CHECK_EQUAL(exres.return_code, EXIT_FAILURE);
             BOOST_CHECK_EQUAL(result.size (), 0);
+            BOOST_CHECK_EQUAL(exres.error_message,
+                              "child failed to exec(): "
+                              "error 2 (No such file or directory)");
         }
 
         const char *const false_argv_abs [] = { "/bin/false", NULL };
@@ -185,9 +190,12 @@ BOOST_AUTO_TEST_SUITE(pipestream)
             const std::string result =
                 capture_exec (true_argv_rel, exres, true, false, false);
 
-            BOOST_CHECK(exres.normal_exit);
-            /* no return code check since we couln't exit */
+            BOOST_CHECK(!exres.normal_exit); /* failed to exec() */
+            /* no return code check since we couldn't exit */
             BOOST_CHECK_EQUAL(result.size (), 0);
+            BOOST_CHECK_EQUAL(exres.error_message,
+                              "child failed to exec(): "
+                              "error 2 (No such file or directory)");
         }
 
         BOOST_AUTO_TEST_CASE(abspath_true_noshell_ok)
@@ -208,7 +216,7 @@ BOOST_AUTO_TEST_SUITE(pipestream)
                 capture_exec (false_argv_rel, exres, true, false, true);
 
             BOOST_CHECK(exres.normal_exit);
-            /* no return code check since we couln't exit */
+            /* no return code check since we couldn't exit */
             BOOST_CHECK_EQUAL(result.size (), 0);
         }