git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Johannes Sixt <j.sixt@viscovery.net>
Cc: Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org, Jeff King <peff@peff.net>
Subject: [PATCH 2/2] run-command: handle short writes and EINTR in die_child
Date: Wed, 20 Apr 2011 05:40:05 -0500	[thread overview]
Message-ID: <20110420104005.GC6027@elie> (raw)
In-Reply-To: <20110420103319.GA6027@elie>

If start_command fails after forking and before exec finishes, there
is not much use in noticing an I/O error on top of that.
finish_command will notice that the child exited with nonzero status
anyway.  So as noted in v1.7.0.3~20^2 (run-command.c: fix build
warnings on Ubuntu, 2010-01-30) and v1.7.5-rc0~29^2 (2011-03-16), it
is safe to ignore errors from write in this codepath.

Even so, the result from write contains useful information: it tells
us if the write was cancelled by a signal (EINTR) or was only
partially completed (e.g., when writing to an almost-full pipe).
Let's use write_in_full to loop until the desired number of bytes have
been written (still ignoring errors if that fails).

As a happy side effect, the assignment to a dummy variable to appease
gcc -D_FORTIFY_SOURCE is no longer needed.  xwrite and write_in_full
check the return value from write(2).

Noticed with gcc -Wunused-but-set-variable.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Changes from v1:

 - rewrite the commit message from the pov of a person who does not
   care about this patch's origin as a brown paper bag

 - drop the "if"s.  If the fussy compiler/library combination was
   right in some strange sense after all, then it does not make much
   sense to take the opportunity to make another token effort to
   appease it as a preventative step.

 run-command.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/run-command.c b/run-command.c
index f91e446..70e8a24 100644
--- a/run-command.c
+++ b/run-command.c
@@ -67,21 +67,24 @@ static int child_notifier = -1;
 
 static void notify_parent(void)
 {
-	ssize_t unused;
-	unused = write(child_notifier, "", 1);
+	/*
+	 * execvp failed.  If possible, we'd like to let start_command
+	 * know, so failures like ENOENT can be handled right away; but
+	 * otherwise, finish_command will still report the error.
+	 */
+	xwrite(child_notifier, "", 1);
 }
 
 static NORETURN void die_child(const char *err, va_list params)
 {
 	char msg[4096];
-	ssize_t unused;
 	int len = vsnprintf(msg, sizeof(msg), err, params);
 	if (len > sizeof(msg))
 		len = sizeof(msg);
 
-	unused = write(child_err, "fatal: ", 7);
-	unused = write(child_err, msg, len);
-	unused = write(child_err, "\n", 1);
+	write_in_full(child_err, "fatal: ", 7);
+	write_in_full(child_err, msg, len);
+	write_in_full(child_err, "\n", 1);
 	exit(128);
 }
 #endif
-- 
1.7.5.rc2

  parent reply	other threads:[~2011-04-20 10:40 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-18 20:54 [REGRESSION] git-wrapper to run-commands codepath regression Junio C Hamano
2011-04-18 21:11 ` Jeff King
2011-04-18 21:18   ` Jeff King
2011-04-18 21:40     ` Junio C Hamano
2011-04-18 21:43       ` Jeff King
2011-04-18 22:10         ` Junio C Hamano
2011-04-18 22:11       ` Andreas Schwab
2011-04-18 21:16 ` Junio C Hamano
2011-04-18 22:17   ` Jonathan Nieder
2011-04-19  7:05   ` [PATCH] run-command: write full error message in die_child Jonathan Nieder
2011-04-20  7:42     ` Johannes Sixt
2011-04-20 10:33       ` [PATCH v2 0/2] " Jonathan Nieder
2011-04-20 10:35         ` [PATCH 1/2] tests: check error message from run_command Jonathan Nieder
2011-04-20 10:40         ` Jonathan Nieder [this message]
2011-04-19  0:07 ` [REGRESSION] git-wrapper to run-commands codepath regression Junio C Hamano
2011-04-20  4:01   ` [PATCH] report which $PATH entry had trouble running execvp(3) Junio C Hamano
2011-04-20  5:51     ` Jeff King
2011-04-21  0:00       ` Junio C Hamano
2011-04-20  7:37     ` Johannes Sixt
2011-04-20 11:21     ` Jonathan Nieder

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=20110420104005.GC6027@elie \
    --to=jrnieder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j.sixt@viscovery.net \
    --cc=peff@peff.net \
    /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).