[PATCH v2] fix start_command() bug when stdin is closed
- From
Paolo Bonzini <bonzini@gnu.org>
- Date
- Aug 25, 2008, 12:00 UTC
- Message-ID
- <E1KXawS-0001gg-Ty@fencepost.gnu.org>
- In-Reply-To
- <48B29C52.8040901@gnu.org>
There is a problem in the use of dup2+close in start_command() when one or more of file descriptors 0/1/2 are closed. In order to rename a pipe file descriptor to 0, it does
dup2(from, 0);
close(from);... but if stdin was closed (for example) from == 0, so that
dup2(0, 0);
close(0);just ends up closing the pipe. This patch fixes it by opening all of the "low" descriptors to /dev/null.
In most cases this patch will not cause any additional system calls; actually by reusing the /dev/null descriptor when possible (instead of opening a fresh one in dup_devnull) it may even save a handful in some cases. :-)
Signed-off-by: Paolo Bonzini <bonzini@gnu.org> --- run-command.c | 35 ++++++++++++++++++++++------------- 1 files changed, 22 insertions(+), 13 deletions(-)
diff --git a/run-command.c b/run-command.c index caab374..4619494 100644 --- a/run-command.c +++ b/run-command.c @@ -2,25 +2,34 @@ #include "run-command.h" #include "exec_cmd.h" +static int devnull_fd = -1; + static inline void close_pair(int fd[2]) { close(fd[0]); close(fd[1]); } -static inline void dup_devnull(int to) -{ - int fd = open("/dev/null", O_RDWR); - dup2(fd, to); - close(fd); -} - int start_command(struct child_process *cmd) { int need_in, need_out, need_err; int fdin[2], fdout[2], fderr[2]; /* + * Make sure that all file descriptors <= 2 are open, otherwise we + * mess them up when dup'ing pipes onto stdin/stdout/stderr. Since + * we are at it, open a file descriptor on /dev/null to use it later. + */ + if (devnull_fd == -1) + { + devnull_fd = open("/dev/null", O_RDWR); + while (devnull_fd >= 0 && devnull_fd <= 2) + devnull_fd = dup(devnull_fd); + if (devnull_fd == -1) + die("opening /dev/null failed (%s)", strerror(errno)); + } + + /* * In case of errors we must keep the promise to close FDs * that have been passed in via ->in and ->out. */ @@ -72,7 +81,7 @@ int start_command(struct child_process *cmd) cmd->pid = fork(); if (!cmd->pid) { if (cmd->no_stdin) - dup_devnull(0); + dup2(devnull_fd, 0); else if (need_in) { dup2(fdin[0], 0); close_pair(fdin); @@ -82,14 +91,14 @@ int start_command(struct child_process *cmd) } if (cmd->no_stderr) - dup_devnull(2); + dup2(devnull_fd, 2); else if (need_err) { dup2(fderr[1], 2); close_pair(fderr); } if (cmd->no_stdout) - dup_devnull(1); + dup2(devnull_fd, 1); else if (cmd->stdout_to_stderr) dup2(2, 1); else if (need_out) { @@ -127,7 +136,7 @@ int start_command(struct child_process *cmd) if (cmd->no_stdin) { s0 = dup(0); - dup_devnull(0); + dup2(devnull_fd, 0); } else if (need_in) { s0 = dup(0); dup2(fdin[0], 0); @@ -138,7 +147,7 @@ int start_command(struct child_process *cmd) if (cmd->no_stderr) { s2 = dup(2); - dup_devnull(2); + dup2(devnull_fd, 2); } else if (need_err) { s2 = dup(2); dup2(fderr[1], 2); @@ -146,7 +155,7 @@ int start_command(struct child_process *cmd) if (cmd->no_stdout) { s1 = dup(1); - dup_devnull(1); + dup2(devnull_fd, 1); } else if (cmd->stdout_to_stderr) { s1 = dup(1); dup2(2, 1);
-- 1.5.5