All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Subject: [PATCH 2/4] avoid double close of descriptors handed to run_command
Date: Tue, 24 Jun 2014 05:45:46 -0400	[thread overview]
Message-ID: <20140624094545.GB14514@sigill.intra.peff.net> (raw)
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

  parent reply	other threads:[~2014-06-24  9:45 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-24  9:42 [PATCH 0/4] git replace --edit --raw Jeff King
2014-06-24  9:43 ` [PATCH 1/4] replace: replace spaces with tabs in indentation Jeff King
2014-06-24  9:45 ` Jeff King [this message]
2014-06-24  9:46 ` [PATCH 3/4] replace: use argv_array in export_object Jeff King
2014-06-24  9:46 ` [PATCH 4/4] replace: add a --raw mode for --edit Jeff King
2014-06-25  1:40   ` Eric Sunshine
2014-06-25 10:24     ` Jeff King
2014-06-25 22:33   ` Junio C Hamano
2014-06-26 15:55     ` Jeff King

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=20140624094545.GB14514@sigill.intra.peff.net \
    --to=peff@peff.net \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.