From d3d2242ba8d975f8d7ddaa0348953dfd6f45ffce Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 12 Nov 2018 16:11:18 +1030 Subject: [PATCH 1/1] pipecmd: reverse order of args to match documentation; add pipecmd_preserve. Signed-off-by: Rusty Russell --- ccan/pipecmd/_info | 2 +- ccan/pipecmd/pipecmd.c | 29 +++++++--- ccan/pipecmd/pipecmd.h | 7 +++ ccan/pipecmd/test/run-fdleak.c | 2 +- ccan/pipecmd/test/run-preserve.c | 98 ++++++++++++++++++++++++++++++++ ccan/pipecmd/test/run.c | 10 ++-- 6 files changed, 132 insertions(+), 16 deletions(-) create mode 100644 ccan/pipecmd/test/run-preserve.c diff --git a/ccan/pipecmd/_info b/ccan/pipecmd/_info index a560bfea..8c49511a 100644 --- a/ccan/pipecmd/_info +++ b/ccan/pipecmd/_info @@ -32,7 +32,7 @@ * exit(1); * exit(0); * } - * child = pipecmd(&outputfd, NULL, NULL, argv[0], "ignoredarg", NULL); + * child = pipecmd(NULL, &outputfd, 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 32772a83..c2b514df 100644 --- a/ccan/pipecmd/pipecmd.c +++ b/ccan/pipecmd/pipecmd.c @@ -6,6 +6,8 @@ #include #include +int pipecmd_preserve; + static char **gather_args(const char *arg0, va_list ap) { size_t n = 1; @@ -26,7 +28,7 @@ static char **gather_args(const char *arg0, va_list ap) return arr; } -pid_t pipecmdv(int *fd_fromchild, int *fd_tochild, int *fd_errfromchild, +pid_t pipecmdv(int *fd_tochild, int *fd_fromchild, int *fd_errfromchild, const char *cmd, va_list ap) { char **arr = gather_args(cmd, ap); @@ -36,12 +38,12 @@ pid_t pipecmdv(int *fd_fromchild, int *fd_tochild, int *fd_errfromchild, errno = ENOMEM; return -1; } - ret = pipecmdarr(fd_fromchild, fd_tochild, fd_errfromchild, arr); + ret = pipecmdarr(fd_tochild, fd_fromchild, fd_errfromchild, arr); free_noerr(arr); return ret; } -pid_t pipecmdarr(int *fd_fromchild, int *fd_tochild, int *fd_errfromchild, +pid_t pipecmdarr(int *fd_tochild, int *fd_fromchild, int *fd_errfromchild, char *const *arr) { int tochild[2], fromchild[2], errfromchild[2], execfail[2]; @@ -49,7 +51,10 @@ pid_t pipecmdarr(int *fd_fromchild, int *fd_tochild, int *fd_errfromchild, int err; if (fd_tochild) { - if (pipe(tochild) != 0) + if (fd_tochild == &pipecmd_preserve) { + tochild[0] = STDIN_FILENO; + fd_tochild = NULL; + } else if (pipe(tochild) != 0) goto fail; } else { tochild[0] = open("/dev/null", O_RDONLY); @@ -57,7 +62,10 @@ pid_t pipecmdarr(int *fd_fromchild, int *fd_tochild, int *fd_errfromchild, goto fail; } if (fd_fromchild) { - if (pipe(fromchild) != 0) + if (fd_fromchild == &pipecmd_preserve) { + fromchild[1] = STDOUT_FILENO; + fd_fromchild = NULL; + } else if (pipe(fromchild) != 0) goto close_tochild_fail; } else { fromchild[1] = open("/dev/null", O_WRONLY); @@ -65,7 +73,10 @@ pid_t pipecmdarr(int *fd_fromchild, int *fd_tochild, int *fd_errfromchild, goto close_tochild_fail; } if (fd_errfromchild) { - if (fd_errfromchild == fd_fromchild) { + if (fd_errfromchild == &pipecmd_preserve) { + errfromchild[1] = STDERR_FILENO; + fd_errfromchild = NULL; + } else if (fd_errfromchild == fd_fromchild) { errfromchild[0] = fromchild[0]; errfromchild[1] = fromchild[1]; } else { @@ -77,7 +88,7 @@ pid_t pipecmdarr(int *fd_fromchild, int *fd_tochild, int *fd_errfromchild, if (errfromchild[1] < 0) goto close_fromchild_fail; } - + if (pipe(execfail) != 0) goto close_errfromchild_fail; @@ -168,14 +179,14 @@ fail: return -1; } -pid_t pipecmd(int *fd_fromchild, int *fd_tochild, int *fd_errfromchild, +pid_t pipecmd(int *fd_tochild, int *fd_fromchild, int *fd_errfromchild, const char *cmd, ...) { pid_t childpid; va_list ap; va_start(ap, cmd); - childpid = pipecmdv(fd_fromchild, fd_tochild, fd_errfromchild, cmd, ap); + childpid = pipecmdv(fd_tochild, fd_fromchild, fd_errfromchild, cmd, ap); va_end(ap); return childpid; diff --git a/ccan/pipecmd/pipecmd.h b/ccan/pipecmd/pipecmd.h index 48169505..5bbaefc0 100644 --- a/ccan/pipecmd/pipecmd.h +++ b/ccan/pipecmd/pipecmd.h @@ -18,6 +18,7 @@ * If @errfd is NULL, the child's stderr is (write-only) /dev/null. * * If @errfd == @outfd (and non-NULL) they will be shared. + * If @infd, @outfd or @errfd is &pipecmd_preserve, it is unchanged. * * The return value is the pid of the child, or -1. */ @@ -41,4 +42,10 @@ pid_t pipecmdv(int *infd, int *outfd, int *errfd, const char *cmd, va_list ap); * @arr: NULL-terminated array for arguments (first is program to run). */ pid_t pipecmdarr(int *infd, int *outfd, int *errfd, char *const *arr); + +/** + * pipecmd_preserve - special value for fds to indicate it is unchanged + */ +extern int pipecmd_preserve; + #endif /* CCAN_PIPECMD_H */ diff --git a/ccan/pipecmd/test/run-fdleak.c b/ccan/pipecmd/test/run-fdleak.c index 775ef333..6b772906 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, NULL, argv[0], "out", NULL); + child = pipecmd(NULL, &outfd, 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-preserve.c b/ccan/pipecmd/test/run-preserve.c new file mode 100644 index 00000000..c9e7cd96 --- /dev/null +++ b/ccan/pipecmd/test/run-preserve.c @@ -0,0 +1,98 @@ +#include +/* Include the C files directly. */ +#include +#include +#include +#include +#include + +int main(int argc, char *argv[]) +{ + pid_t child; + int fd, oldfd, status; + char buf[5] = "test"; + char template[] = "/tmp/run-preserve.XXXXXX"; + + /* We call ourselves, to test pipe. */ + if (argc == 2) { + if (strcmp(argv[1], "out") == 0) { + if (write(STDOUT_FILENO, buf, sizeof(buf)) != sizeof(buf)) + exit(2); + } else if (strcmp(argv[1], "in") == 0) { + if (read(STDIN_FILENO, buf, sizeof(buf)) != sizeof(buf)) + exit(3); + if (memcmp(buf, "test", sizeof(buf)) != 0) + exit(4); + } else if (strcmp(argv[1], "err") == 0) { + if (write(STDERR_FILENO, buf, sizeof(buf)) != sizeof(buf)) + exit(5); + } else + abort(); + exit(0); + } + + /* This is how many tests you plan to run */ + plan_tests(25); + + /* Preserve stdin test. */ + fd = mkstemp(template); + ok1(write(fd, buf, sizeof(buf)) == sizeof(buf)); + ok1(fd >= 0); + ok1(dup2(fd, STDIN_FILENO) == STDIN_FILENO); + ok1(lseek(STDIN_FILENO, 0, SEEK_SET) == 0); + child = pipecmd(&pipecmd_preserve, NULL, NULL, argv[0], "in", NULL); + if (!ok1(child > 0)) + exit(1); + ok1(waitpid(child, &status, 0) == child); + ok1(WIFEXITED(status)); + ok1(WEXITSTATUS(status) == 0); + + close(STDIN_FILENO); + + /* Preserve stdout test */ + fd = open(template, O_WRONLY|O_TRUNC); + ok1(fd >= 0); + oldfd = dup(STDOUT_FILENO); + /* Can't use OK after this, since we mug stdout */ + if (dup2(fd, STDOUT_FILENO) != STDOUT_FILENO) + exit(1); + child = pipecmd(NULL, &pipecmd_preserve, NULL, argv[0], "out", NULL); + if (child == -1) + exit(1); + /* Restore stdout */ + dup2(oldfd, STDOUT_FILENO); + close(oldfd); + ok1(waitpid(child, &status, 0) == child); + ok1(WIFEXITED(status)); + ok1(WEXITSTATUS(status) == 0); + + fd = open(template, O_RDONLY); + ok1(read(fd, buf, sizeof(buf)) == sizeof(buf)); + ok1(close(fd) == 0); + ok1(memcmp(buf, "test", sizeof(buf)) == 0); + + /* Preserve stderr test. */ + fd = open(template, O_WRONLY|O_TRUNC); + ok1(fd >= 0); + oldfd = dup(STDERR_FILENO); + ok1(dup2(fd, STDERR_FILENO) == STDERR_FILENO); + child = pipecmd(NULL, NULL, &pipecmd_preserve, argv[0], "err", NULL); + if (!ok1(child > 0)) + exit(1); + + /* Restore stderr. */ + ok1(dup2(oldfd, STDERR_FILENO)); + ok1(waitpid(child, &status, 0) == child); + ok1(WIFEXITED(status)); + ok1(WEXITSTATUS(status) == 0); + close(oldfd); + + fd = open(template, O_RDONLY); + ok1(read(fd, buf, sizeof(buf)) == sizeof(buf)); + ok1(close(fd) == 0); + ok1(memcmp(buf, "test", sizeof(buf)) == 0); + unlink(template); + + /* This exits depending on whether all tests passed */ + return exit_status(); +} diff --git a/ccan/pipecmd/test/run.c b/ccan/pipecmd/test/run.c index 45b96ab2..e446da6d 100644 --- a/ccan/pipecmd/test/run.c +++ b/ccan/pipecmd/test/run.c @@ -48,7 +48,7 @@ int main(int argc, char *argv[]) /* This is how many tests you plan to run */ plan_tests(67); - child = pipecmd(&outfd, &infd, &errfd, argv[0], "inout", NULL); + child = pipecmd(&infd, &outfd, &errfd, argv[0], "inout", NULL); if (!ok1(child > 0)) exit(1); ok1(write(infd, buf, sizeof(buf)) == sizeof(buf)); @@ -63,7 +63,7 @@ int main(int argc, char *argv[]) ok1(WIFEXITED(status)); ok1(WEXITSTATUS(status) == 0); - child = pipecmd(NULL, &infd, NULL, argv[0], "in", NULL); + child = pipecmd(&infd, NULL, NULL, argv[0], "in", NULL); if (!ok1(child > 0)) exit(1); ok1(write(infd, buf, sizeof(buf)) == sizeof(buf)); @@ -72,7 +72,7 @@ int main(int argc, char *argv[]) ok1(WIFEXITED(status)); ok1(WEXITSTATUS(status) == 0); - child = pipecmd(&outfd, NULL, NULL, argv[0], "out", NULL); + child = pipecmd(NULL, &outfd, NULL, argv[0], "out", NULL); if (!ok1(child > 0)) exit(1); ok1(read(outfd, buf, sizeof(buf)) == sizeof(buf)); @@ -94,7 +94,7 @@ int main(int argc, char *argv[]) ok1(WEXITSTATUS(status) == 0); /* errfd == outfd should work with both. */ - child = pipecmd(&errfd, NULL, &errfd, argv[0], "err", NULL); + child = pipecmd(NULL, &errfd, &errfd, argv[0], "err", NULL); if (!ok1(child > 0)) exit(1); ok1(read(errfd, buf, sizeof(buf)) == sizeof(buf)); @@ -104,7 +104,7 @@ int main(int argc, char *argv[]) ok1(WIFEXITED(status)); ok1(WEXITSTATUS(status) == 0); - child = pipecmd(&outfd, NULL, &outfd, argv[0], "out", NULL); + child = pipecmd(NULL, &outfd, &outfd, argv[0], "out", NULL); if (!ok1(child > 0)) exit(1); ok1(read(outfd, buf, sizeof(buf)) == sizeof(buf)); -- 2.39.2