From: Rusty Russell Date: Tue, 19 Jan 2016 03:45:25 +0000 (+1030) Subject: pipecmd: add stderr fd arg. X-Git-Url: https://git.ozlabs.org/?p=ccan;a=commitdiff_plain;h=97a1ba3d6fac93e680e3d95cef930dea4fdd0822 pipecmd: add stderr fd arg. Signed-off-by: Rusty Russell --- diff --git a/ccan/pipecmd/_info b/ccan/pipecmd/_info index 1af377bf..4621d72f 100644 --- 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 c97fa312..42c66894 100644 --- 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 87e3dcf8..48169505 100644 --- 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 9ab9d1bb..775ef333 100644 --- 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 425e7d00..45b96ab2 100644 --- 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(); }