[PATCH 2/4] avoid double close of descriptors handed to run_command
- From
Jeff King <peff@peff.net>
- Date
- Jun 24, 2014, 09:45 UTC
- Message-ID
- <20140624094545.GB14514@sigill.intra.peff.net>
- In-Reply-To
- <20140624094217.GA14216@sigill.intra.peff.net>
When a file descriptor is given to run_command via the "in", "out", or "err" parameters, run_command takes ownership. The descriptor will be closed in the parent process whether the process is spawned successfully or not, and closing it again is wrong.
In practice this has not caused problems, because we usually close() right after start_command returns, meaning no other code has opened a descriptor in the meantime. So we just get EBADF and ignore it (rather than accidentally closing somebody else's descriptor!).
Signed-off-by: Jeff King <peff@peff.net> --- I noticed the one in replace, and grepped around for other instances. I think this is all of them. The only possible cases I didn't investigate carefully were in send-pack/receive-pack. The logic there is quite complicated, and I remember looking at and fixing close() issues there not too long ago (e.g., 49ecfa1), so I assumed it was OK.
builtin/replace.c | 2 -- daemon.c | 1 - 2 files changed, 3 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c index 8507835..eb1d2ec 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -207,8 +207,6 @@ static void export_object(const unsigned char *sha1, const char *filename) if (run_command(&cmd)) die("cat-file reported failure"); - - close(fd); } /* diff --git a/daemon.c b/daemon.c index f9c63e9..e1e424d 100644 --- a/daemon.c +++ b/daemon.c @@ -775,7 +775,6 @@ static void handle(int incoming, struct sockaddr *addr, socklen_t addrlen) logerror("unable to fork"); else add_child(&cld, addr, addrlen); - close(incoming); } static void child_handler(int signo)
-- 2.0.0.566.gfe3e6b2