From: Paolo Bonzini <bonzini@gnu•org>
To: Johannes Sixt <j.sixt@viscovery•net>
Cc: Karl Chen <quarl@cs•berkeley.edu>,
Git mailing list <git@vger•kernel.org>,
Junio C Hamano <gitster@pobox•com>
Subject: [PATCH v2] fix start_command() bug when stdin is closed
Date: Mon, 25 Aug 2008 14:00:35 +0200 [thread overview]
Message-ID: <E1KXawS-0001gg-Ty@fencepost.gnu.org> (raw)
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
next prev parent reply other threads:[~2008-08-25 12:15 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-08-25 8:28 [PATCH] Fix start_command() pipe bug when stdin is closed Karl Chen
2008-08-25 10:44 ` Johannes Sixt
2008-08-25 11:49 ` Paolo Bonzini
2008-08-25 12:00 ` Paolo Bonzini [this message]
2008-08-25 13:12 ` [PATCH v2] fix start_command() " Johannes Sixt
2008-08-25 13:37 ` [PATCH v2 properly indented] " Paolo Bonzini
2008-08-25 16:00 ` Karl Chen
2008-08-26 0:06 ` Junio C Hamano
2008-08-26 6:09 ` Junio C Hamano
2008-08-26 6:33 ` Johannes Sixt
2008-08-26 6:45 ` Paolo Bonzini
2008-08-26 6:48 ` [PATCH] be paranoid about closed stdin/stdout/stderr Paolo Bonzini
2008-08-26 6:57 ` Johannes Sixt
2008-08-26 7:40 ` Stephen R. van den Berg
2008-08-27 5:01 ` Avery Pennarun
2008-08-27 9:18 ` Stephen R. van den Berg
2008-08-27 12:36 ` Paolo Bonzini
2008-08-27 15:20 ` [PATCH v4] make git-shell " Paolo Bonzini
2008-08-27 17:22 ` Stephen R. van den Berg
2008-08-27 17:27 ` [PATCH] be " Junio C Hamano
2008-08-28 13:17 ` Paolo Bonzini
2008-08-28 13:58 ` Stephen R. van den Berg
2008-08-27 18:22 ` Avery Pennarun
2008-08-28 12:21 ` Nick Andrew
2008-08-28 12:52 ` Stephen R. van den Berg
2008-08-26 17:38 ` Junio C Hamano
2008-08-26 18:33 ` Paolo Bonzini
2008-08-26 22:42 ` Junio C Hamano
2008-08-26 23:04 ` Junio C Hamano
2008-08-26 23:10 ` Stephen R. van den Berg
2008-08-27 3:05 ` Karl Chen
2008-08-27 4:38 ` Paolo Bonzini
2008-08-27 9:04 ` Stephen R. van den Berg
2008-08-27 6:35 ` Johannes Sixt
2008-08-27 8:20 ` Paolo Bonzini
2008-08-27 2:04 ` Nick Andrew
2008-08-25 15:56 ` [PATCH] Fix start_command() pipe bug when stdin is closed Karl Chen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=E1KXawS-0001gg-Ty@fencepost.gnu.org \
--to=bonzini@gnu$(echo .)org \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=j.sixt@viscovery$(echo .)net \
--cc=quarl@cs$(echo .)berkeley.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox