pipecmd: add stderr fd arg.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/ccan/pipecmd/_info b/ccan/pipecmd/_info
index 1af377bf757bfcffefbed3e1901405e3a06e5600..4621d72fc3b94d78192a8eb33fb9bec60309a600 100644 (file)
--- a/ccan/pipecmd/_info
+++ b/ccan/pipecmd/_info
@@ -31,7 +31,7 @@
  *             write(STDOUT_FILENO, "hello world\n", 12);
  *             exit(0);
  *     }
- *     child = pipecmd(&outputfd, NULL, argv[0], "ignoredarg", NULL);
+ *     child = pipecmd(&outputfd, NULL, NULL, argv[0], "ignoredarg", NULL);
  *     if (child < 0)
  *             err(1, "Creating child");
  *     if (read(outputfd, input, sizeof(input)) != sizeof(input))

diff --git a/ccan/pipecmd/pipecmd.c b/ccan/pipecmd/pipecmd.c
index c97fa312396dbb728e9cdceb6b795e21f6ee33da..42c6689445876e5b832520bd55a1e1aecf288c43 100644 (file)
--- a/ccan/pipecmd/pipecmd.c
+++ b/ccan/pipecmd/pipecmd.c
@@ -23,7 +23,8 @@ static char **gather_args(const char *arg0, va_list ap)
        return arr;
 }
 
-pid_t pipecmdv(int *fd_fromchild, int *fd_tochild, const char *cmd, va_list ap)
+pid_t pipecmdv(int *fd_fromchild, int *fd_tochild, int *fd_errfromchild,
+              const char *cmd, va_list ap)
 {
        char **arr = gather_args(cmd, ap);
        pid_t ret;
@@ -32,14 +33,15 @@ pid_t pipecmdv(int *fd_fromchild, int *fd_tochild, const char *cmd, va_list ap)
                errno = ENOMEM;
                return -1;
        }
-       ret = pipecmdarr(fd_fromchild, fd_tochild, arr);
+       ret = pipecmdarr(fd_fromchild, fd_tochild, fd_errfromchild, arr);
        free_noerr(arr);
        return ret;
 }
 
-pid_t pipecmdarr(int *fd_fromchild, int *fd_tochild, char *const *arr)
+pid_t pipecmdarr(int *fd_fromchild, int *fd_tochild, int *fd_errfromchild,
+                char *const *arr)
 {
-       int tochild[2], fromchild[2], execfail[2];
+       int tochild[2], fromchild[2], errfromchild[2], execfail[2];
        pid_t childpid;
        int err;
 
@@ -59,8 +61,22 @@ pid_t pipecmdarr(int *fd_fromchild, int *fd_tochild, char *const *arr)
                if (fromchild[1] < 0)
                        goto close_tochild_fail;
        }
+       if (fd_errfromchild) {
+               if (fd_errfromchild == fd_fromchild) {
+                       errfromchild[0] = fromchild[0];
+                       errfromchild[1] = fromchild[1];
+               } else {
+                       if (pipe(errfromchild) != 0)
+                               goto close_fromchild_fail;
+               }
+       } else {
+               errfromchild[1] = open("/dev/null", O_WRONLY);
+               if (errfromchild[1] < 0)
+                       goto close_fromchild_fail;
+       }
+               
        if (pipe(execfail) != 0)
-               goto close_fromchild_fail;
+               goto close_errfromchild_fail;
 
        if (fcntl(execfail[1], F_SETFD, fcntl(execfail[1], F_GETFD)
                  | FD_CLOEXEC) < 0)
@@ -75,6 +91,9 @@ pid_t pipecmdarr(int *fd_fromchild, int *fd_tochild, char *const *arr)
                        close(tochild[1]);
                if (fd_fromchild)
                        close(fromchild[0]);
+               if (fd_errfromchild && fd_errfromchild != fd_fromchild)
+                       close(errfromchild[0]);
+
                close(execfail[0]);
 
                // Child runs command.
@@ -88,6 +107,14 @@ pid_t pipecmdarr(int *fd_fromchild, int *fd_tochild, char *const *arr)
                                goto child_errno_fail;
                        close(fromchild[1]);
                }
+               if (fd_errfromchild && fd_errfromchild == fd_fromchild) {
+                       if (dup2(STDOUT_FILENO, STDERR_FILENO) == -1)
+                               goto child_errno_fail;
+               } else if (errfromchild[1] != STDERR_FILENO) {
+                       if (dup2(errfromchild[1], STDERR_FILENO) == -1)
+                               goto child_errno_fail;
+                       close(errfromchild[1]);
+               }
                execvp(arr[0], arr);
 
        child_errno_fail:
@@ -98,6 +125,7 @@ pid_t pipecmdarr(int *fd_fromchild, int *fd_tochild, char *const *arr)
 
        close(tochild[0]);
        close(fromchild[1]);
+       close(errfromchild[1]);
        close(execfail[1]);
        /* Child will close this without writing on successful exec. */
        if (read(execfail[0], &err, sizeof(err)) == sizeof(err)) {
@@ -111,11 +139,17 @@ pid_t pipecmdarr(int *fd_fromchild, int *fd_tochild, char *const *arr)
                *fd_tochild = tochild[1];
        if (fd_fromchild)
                *fd_fromchild = fromchild[0];
+       if (fd_errfromchild)
+               *fd_errfromchild = errfromchild[0];
        return childpid;
 
 close_execfail_fail:
        close_noerr(execfail[0]);
        close_noerr(execfail[1]);
+close_errfromchild_fail:
+       if (fd_errfromchild)
+               close_noerr(errfromchild[0]);
+       close_noerr(errfromchild[1]);
 close_fromchild_fail:
        if (fd_fromchild)
                close_noerr(fromchild[0]);
@@ -128,13 +162,14 @@ fail:
        return -1;
 }
 
-pid_t pipecmd(int *fd_fromchild, int *fd_tochild, const char *cmd, ...)
+pid_t pipecmd(int *fd_fromchild, int *fd_tochild, int *fd_errfromchild,
+             const char *cmd, ...)
 {
        pid_t childpid;
 
        va_list ap;
        va_start(ap, cmd);
-       childpid = pipecmdv(fd_fromchild, fd_tochild, cmd, ap);
+       childpid = pipecmdv(fd_fromchild, fd_tochild, fd_errfromchild, cmd, ap);
        va_end(ap);
 
        return childpid;

diff --git a/ccan/pipecmd/pipecmd.h b/ccan/pipecmd/pipecmd.h
index 87e3dcf8193ddf17985f0c33d372be34928566f6..4816950502f2c949183ea72fcaacb60976a995cb 100644 (file)
--- a/ccan/pipecmd/pipecmd.h
+++ b/ccan/pipecmd/pipecmd.h
@@ -10,29 +10,35 @@
  * pipecmd - run a command, optionally connect pipes.
  * @infd: input fd to write to child (if non-NULL)
  * @outfd: output fd to read from child (if non-NULL)
+ * @errfd: error-output fd to read from child (if non-NULL)
  * @cmd...: NULL-terminate list of command and arguments.
  *
  * If @infd is NULL, the child's input is (read-only) /dev/null.
  * If @outfd is NULL, the child's output is (write-only) /dev/null.
+ * If @errfd is NULL, the child's stderr is (write-only) /dev/null.
+ *
+ * If @errfd == @outfd (and non-NULL) they will be shared.
  *
  * The return value is the pid of the child, or -1.
  */
-pid_t pipecmd(int *infd, int *outfd, const char *cmd, ...);
+pid_t pipecmd(int *infd, int *outfd, int *errfd, const char *cmd, ...);
 
 /**
  * pipecmdv - run a command, optionally connect pipes (stdarg version)
  * @infd: input fd to write to child (if non-NULL)
  * @outfd: output fd to read from child (if non-NULL)
+ * @errfd: error-output fd to read from child (if non-NULL)
  * @cmd: command to run.
  * @ap: argument list for arguments.
  */
-pid_t pipecmdv(int *infd, int *outfd, const char *cmd, va_list ap);
+pid_t pipecmdv(int *infd, int *outfd, int *errfd, const char *cmd, va_list ap);
 
 /**
  * pipecmdarr - run a command, optionally connect pipes (char arry version)
  * @infd: input fd to write to child (if non-NULL)
  * @outfd: output fd to read from child (if non-NULL)
+ * @errfd: error-output fd to read from child (if non-NULL)
  * @arr: NULL-terminated array for arguments (first is program to run).
  */
-pid_t pipecmdarr(int *infd, int *outfd, char *const *arr);
+pid_t pipecmdarr(int *infd, int *outfd, int *errfd, char *const *arr);
 #endif /* CCAN_PIPECMD_H */

diff --git a/ccan/pipecmd/test/run-fdleak.c b/ccan/pipecmd/test/run-fdleak.c
index 9ab9d1bbbdbce977617cf9363fe5ab524234af99..775ef333f8be42864fa4b65b247bc4420a4ed5a8 100644 (file)
--- a/ccan/pipecmd/test/run-fdleak.c
+++ b/ccan/pipecmd/test/run-fdleak.c
@@ -21,7 +21,7 @@ int main(int argc, char *argv[])
 
        /* This is how many tests you plan to run */
        plan_tests(13);
-       child = pipecmd(&outfd, NULL, argv[0], "out", NULL);
+       child = pipecmd(&outfd, NULL, NULL, argv[0], "out", NULL);
        if (!ok1(child > 0))
                exit(1);
        ok1(read(outfd, buf, sizeof(buf)) == sizeof(buf));

diff --git a/ccan/pipecmd/test/run.c b/ccan/pipecmd/test/run.c
index 425e7d00bbd79729d5897a6161550bbb0bab2a61..45b96ab2f03b2d5d2145f9cdc6a3ae990778b528 100644 (file)
--- a/ccan/pipecmd/test/run.c
+++ b/ccan/pipecmd/test/run.c
@@ -9,7 +9,7 @@
 int main(int argc, char *argv[])
 {
        pid_t child;
-       int infd, outfd, status;
+       int infd, outfd, errfd, status;
        char buf[5] = "test";
 
        /* We call ourselves, to test pipe. */
@@ -28,43 +28,94 @@ int main(int argc, char *argv[])
                        buf[0]++;
                        if (write(STDOUT_FILENO, buf, sizeof(buf)) != sizeof(buf))
                                exit(1);
+               } else if (strcmp(argv[1], "err") == 0) {
+                       if (write(STDERR_FILENO, buf, sizeof(buf)) != sizeof(buf))
+                               exit(1);
                } else
                        abort();
                exit(0);
        }
 
+       /* We assume no fd leaks, so close them now. */
+       close(3);
+       close(4);
+       close(5);
+       close(6);
+       close(7);
+       close(8);
+       close(9);
+       close(10);
+       
        /* This is how many tests you plan to run */
-       plan_tests(28);
-       child = pipecmd(&outfd, &infd, argv[0], "inout", NULL);
+       plan_tests(67);
+       child = pipecmd(&outfd, &infd, &errfd, argv[0], "inout", NULL);
        if (!ok1(child > 0))
                exit(1);
        ok1(write(infd, buf, sizeof(buf)) == sizeof(buf));
        ok1(read(outfd, buf, sizeof(buf)) == sizeof(buf));
+       ok1(read(errfd, buf, sizeof(buf)) == 0);
+       ok1(close(infd) == 0);
+       ok1(close(outfd) == 0);
+       ok1(close(errfd) == 0);
        buf[0]--;
        ok1(memcmp(buf, "test", sizeof(buf)) == 0);
        ok1(waitpid(child, &status, 0) == child);
        ok1(WIFEXITED(status));
        ok1(WEXITSTATUS(status) == 0);
 
-       child = pipecmd(NULL, &infd, argv[0], "in", NULL);
+       child = pipecmd(NULL, &infd, NULL, argv[0], "in", NULL);
        if (!ok1(child > 0))
                exit(1);
        ok1(write(infd, buf, sizeof(buf)) == sizeof(buf));
+       ok1(close(infd) == 0);
+       ok1(waitpid(child, &status, 0) == child);
+       ok1(WIFEXITED(status));
+       ok1(WEXITSTATUS(status) == 0);
+
+       child = pipecmd(&outfd, NULL, NULL, argv[0], "out", NULL);
+       if (!ok1(child > 0))
+               exit(1);
+       ok1(read(outfd, buf, sizeof(buf)) == sizeof(buf));
+       ok1(close(outfd) == 0);
+       ok1(memcmp(buf, "test", sizeof(buf)) == 0);
+       ok1(waitpid(child, &status, 0) == child);
+       ok1(WIFEXITED(status));
+       ok1(WEXITSTATUS(status) == 0);
+
+       /* Errfd only should be fine. */
+       child = pipecmd(NULL, NULL, &errfd, argv[0], "err", NULL);
+       if (!ok1(child > 0))
+               exit(1);
+       ok1(read(errfd, buf, sizeof(buf)) == sizeof(buf));
+       ok1(close(errfd) == 0);
+       ok1(memcmp(buf, "test", sizeof(buf)) == 0);
+       ok1(waitpid(child, &status, 0) == child);
+       ok1(WIFEXITED(status));
+       ok1(WEXITSTATUS(status) == 0);
+
+       /* errfd == outfd should work with both. */
+       child = pipecmd(&errfd, NULL, &errfd, argv[0], "err", NULL);
+       if (!ok1(child > 0))
+               exit(1);
+       ok1(read(errfd, buf, sizeof(buf)) == sizeof(buf));
+       ok1(close(errfd) == 0);
+       ok1(memcmp(buf, "test", sizeof(buf)) == 0);
        ok1(waitpid(child, &status, 0) == child);
        ok1(WIFEXITED(status));
        ok1(WEXITSTATUS(status) == 0);
 
-       child = pipecmd(&outfd, NULL, argv[0], "out", NULL);
+       child = pipecmd(&outfd, NULL, &outfd, argv[0], "out", NULL);
        if (!ok1(child > 0))
                exit(1);
        ok1(read(outfd, buf, sizeof(buf)) == sizeof(buf));
+       ok1(close(outfd) == 0);
        ok1(memcmp(buf, "test", sizeof(buf)) == 0);
        ok1(waitpid(child, &status, 0) == child);
        ok1(WIFEXITED(status));
        ok1(WEXITSTATUS(status) == 0);
 
        // Writing to /dev/null should be fine.
-       child = pipecmd(NULL, NULL, argv[0], "out", NULL);
+       child = pipecmd(NULL, NULL, NULL, argv[0], "out", NULL);
        if (!ok1(child > 0))
                exit(1);
        ok1(waitpid(child, &status, 0) == child);
@@ -72,18 +123,35 @@ int main(int argc, char *argv[])
        ok1(WEXITSTATUS(status) == 0);
 
        // Reading should fail.
-       child = pipecmd(NULL, NULL, argv[0], "in", NULL);
+       child = pipecmd(NULL, NULL, NULL, argv[0], "in", NULL);
        if (!ok1(child > 0))
                exit(1);
        ok1(waitpid(child, &status, 0) == child);
        ok1(WIFEXITED(status));
        ok1(WEXITSTATUS(status) == 1);
 
+       child = pipecmd(NULL, NULL, NULL, argv[0], "err", NULL);
+       if (!ok1(child > 0))
+               exit(1);
+       ok1(waitpid(child, &status, 0) == child);
+       ok1(WIFEXITED(status));
+       ok1(WEXITSTATUS(status) == 0);
+
        // Can't run non-existent file, but errno set correctly.
-       child = pipecmd(NULL, NULL, "/doesnotexist", "in", NULL);
+       child = pipecmd(NULL, NULL, NULL, "/doesnotexist", "in", NULL);
        ok1(errno == ENOENT);
        ok1(child < 0);
 
+       /* No fd leaks! */
+       ok1(close(3) == -1 && errno == EBADF);
+       ok1(close(4) == -1 && errno == EBADF);
+       ok1(close(5) == -1 && errno == EBADF);
+       ok1(close(6) == -1 && errno == EBADF);
+       ok1(close(7) == -1 && errno == EBADF);
+       ok1(close(8) == -1 && errno == EBADF);
+       ok1(close(9) == -1 && errno == EBADF);
+       ok1(close(10) == -1 && errno == EBADF);
+
        /* This exits depending on whether all tests passed */
        return exit_status();
 }