From: Karl Chen <quarl@cs.berkeley.edu>
To: Git mailing list <git@vger.kernel.org>
Subject: [PATCH] Fix start_command() pipe bug when stdin is closed.
Date: Mon, 25 Aug 2008 01:28:19 -0700 [thread overview]
Message-ID: <quack.20080825T0128.lthr68djy70@roar.cs.berkeley.edu> (raw)
I ran into what I think is a bug:
sh$ git fetch 0<&-
(i.e. run git-fetch with stdin closed.)
It aborts with:
fatal: read error (Bad file descriptor)
I think the problem arises from the use of dup2+close in
start_command(). It wants to rename a pipe file descriptor to 0,
so it does
dup2(from, to);
close(from);
... but in this case from == to == 0, so
dup2(0, 0);
close(0);
just ends up closing the pipe.
The patch below fixes the problem for me.
>From 78446c82131a5ca7f22f92bc32d7f3036bba9629 Mon Sep 17 00:00:00 2001
From: Karl Chen <quarl@quarl.org>
Date: Mon, 25 Aug 2008 01:09:08 -0700
Subject: [PATCH] Fix start_command() pipe bug when stdin is closed.
When intending to rename a fd to 0, if the fd is already 0, then do nothing,
instead of dup2(0,0); close(0);
The problematic behavior could be seen thus: git-fetch 0<&-
Signed-off-by: Karl Chen <quarl@quarl.org>
---
run-command.c | 29 +++++++++++++++++------------
1 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/run-command.c b/run-command.c
index caab374..b4bd80f 100644
--- a/run-command.c
+++ b/run-command.c
@@ -8,11 +8,18 @@ static inline void close_pair(int fd[2])
close(fd[1]);
}
+static inline void rename_fd(int from, int to)
+{
+ if (from != to) {
+ dup2(from, to);
+ close(from);
+ }
+}
+
static inline void dup_devnull(int to)
{
int fd = open("/dev/null", O_RDWR);
- dup2(fd, to);
- close(fd);
+ rename_fd(fd, to);
}
int start_command(struct child_process *cmd)
@@ -74,18 +81,17 @@ int start_command(struct child_process *cmd)
if (cmd->no_stdin)
dup_devnull(0);
else if (need_in) {
- dup2(fdin[0], 0);
- close_pair(fdin);
+ rename_fd(fdin[0], 0);
+ close(fdin[1]);
} else if (cmd->in) {
- dup2(cmd->in, 0);
- close(cmd->in);
+ rename_fd(cmd->in, 0);
}
if (cmd->no_stderr)
dup_devnull(2);
else if (need_err) {
- dup2(fderr[1], 2);
- close_pair(fderr);
+ rename_fd(fderr[1], 2);
+ close(fderr[0]);
}
if (cmd->no_stdout)
@@ -93,11 +99,10 @@ int start_command(struct child_process *cmd)
else if (cmd->stdout_to_stderr)
dup2(2, 1);
else if (need_out) {
- dup2(fdout[1], 1);
- close_pair(fdout);
+ rename_fd(fdout[1], 1);
+ close(fdout[0]);
} else if (cmd->out > 1) {
- dup2(cmd->out, 1);
- close(cmd->out);
+ rename_fd(cmd->out, 1);
}
if (cmd->dir && chdir(cmd->dir))
--
1.5.6.2
next reply other threads:[~2008-08-25 8:29 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-08-25 8:28 Karl Chen [this message]
2008-08-25 10:44 ` [PATCH] Fix start_command() pipe bug when stdin is closed Johannes Sixt
2008-08-25 11:49 ` Paolo Bonzini
2008-08-25 12:00 ` [PATCH v2] fix start_command() " Paolo Bonzini
2008-08-25 13:12 ` 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=quack.20080825T0128.lthr68djy70@roar.cs.berkeley.edu \
--to=quarl@cs.berkeley.edu \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).