From 75301f90159a505f3683a5eba10174928dc30fb1 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Fri, 15 Jan 2010 21:12:18 +0100 Subject: Windows: avoid the "dup dance" when spawning a child process When stdin, stdout, or stderr must be redirected for a child process that on Windows is spawned using one of the spawn() functions of Microsoft's C runtime, then there is no choice other than to 1. make a backup copy of fd 0,1,2 with dup 2. dup2 the redirection source fd into 0,1,2 3. spawn 4. dup2 the backup back into 0,1,2 5. close the backup copy and the redirection source We used this idiom as well -- but we are not using the spawn() functions anymore! Instead, we have our own implementation. We had hardcoded that stdin, stdout, and stderr of the child process were inherited from the parent's fds 0, 1, and 2. But we can actually specify any fd. With this patch, the fds to inherit are passed from start_command()'s WIN32 section to our spawn implementation. This way, we can avoid the backup copies of the fds. The backup copies were a bug waiting to surface: The OS handles underlying the dup()ed fds were inherited by the child process (but were not associated with a file descriptor in the child). Consequently, the file or pipe represented by the OS handle remained open even after the backup copy was closed in the parent process until the child exited. Since our implementation of pipe() creates non-inheritable OS handles, we still dup() file descriptors in start_command() because dup() happens to create inheritable duplicates. (A nice side effect is that the fd cleanup in start_command is the same for Windows and Unix and remains unchanged.) Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- run-command.c | 71 ++++++++++++++++++++++++++--------------------------------- 1 file changed, 31 insertions(+), 40 deletions(-) (limited to 'run-command.c') diff --git a/run-command.c b/run-command.c index cf2d8f7fa..d27066402 100644 --- a/run-command.c +++ b/run-command.c @@ -8,12 +8,14 @@ static inline void close_pair(int fd[2]) close(fd[1]); } +#ifndef WIN32 static inline void dup_devnull(int to) { int fd = open("/dev/null", O_RDWR); dup2(fd, to); close(fd); } +#endif int start_command(struct child_process *cmd) { @@ -135,42 +137,30 @@ fail_pipe: strerror(failed_errno = errno)); #else { - int s0 = -1, s1 = -1, s2 = -1; /* backups of stdin, stdout, stderr */ + int fhin = 0, fhout = 1, fherr = 2; const char **sargv = cmd->argv; char **env = environ; - if (cmd->no_stdin) { - s0 = dup(0); - dup_devnull(0); - } else if (need_in) { - s0 = dup(0); - dup2(fdin[0], 0); - } else if (cmd->in) { - s0 = dup(0); - dup2(cmd->in, 0); - } - - if (cmd->no_stderr) { - s2 = dup(2); - dup_devnull(2); - } else if (need_err) { - s2 = dup(2); - dup2(fderr[1], 2); - } - - if (cmd->no_stdout) { - s1 = dup(1); - dup_devnull(1); - } else if (cmd->stdout_to_stderr) { - s1 = dup(1); - dup2(2, 1); - } else if (need_out) { - s1 = dup(1); - dup2(fdout[1], 1); - } else if (cmd->out > 1) { - s1 = dup(1); - dup2(cmd->out, 1); - } + if (cmd->no_stdin) + fhin = open("/dev/null", O_RDWR); + else if (need_in) + fhin = dup(fdin[0]); + else if (cmd->in) + fhin = dup(cmd->in); + + if (cmd->no_stderr) + fherr = open("/dev/null", O_RDWR); + else if (need_err) + fherr = dup(fderr[1]); + + if (cmd->no_stdout) + fhout = open("/dev/null", O_RDWR); + else if (cmd->stdout_to_stderr) + fhout = dup(fherr); + else if (need_out) + fhout = dup(fdout[1]); + else if (cmd->out > 1) + fhout = dup(cmd->out); if (cmd->dir) die("chdir in start_command() not implemented"); @@ -181,7 +171,8 @@ fail_pipe: cmd->argv = prepare_git_cmd(cmd->argv); } - cmd->pid = mingw_spawnvpe(cmd->argv[0], cmd->argv, env); + cmd->pid = mingw_spawnvpe(cmd->argv[0], cmd->argv, env, + fhin, fhout, fherr); failed_errno = errno; if (cmd->pid < 0 && (!cmd->silent_exec_failure || errno != ENOENT)) error("cannot spawn %s: %s", cmd->argv[0], strerror(errno)); @@ -192,12 +183,12 @@ fail_pipe: free(cmd->argv); cmd->argv = sargv; - if (s0 >= 0) - dup2(s0, 0), close(s0); - if (s1 >= 0) - dup2(s1, 1), close(s1); - if (s2 >= 0) - dup2(s2, 2), close(s2); + if (fhin != 0) + close(fhin); + if (fhout != 1) + close(fhout); + if (fherr != 2) + close(fherr); } #endif -- cgit v1.2.1