git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	Jonathan Nieder <jrnieder@gmail.com>,
	"Shawn O. Pearce" <spearce@spearce.org>
Subject: [PATCH v3 08/19] write_or_die: raise SIGPIPE when we get EPIPE
Date: Wed, 20 Feb 2013 15:01:36 -0500	[thread overview]
Message-ID: <20130220200136.GH25647@sigill.intra.peff.net> (raw)
In-Reply-To: <20130220195147.GA25332@sigill.intra.peff.net>

The write_or_die function will always die on an error,
including EPIPE. However, it currently treats EPIPE
specially by suppressing any error message, and by exiting
with exit code 0.

Suppressing the error message makes some sense; a pipe death
may just be a sign that the other side is not interested in
what we have to say. However, exiting with a successful
error code is not a good idea, as write_or_die is frequently
used in cases where we want to be careful about having
written all of the output, and we may need to signal to our
caller that we have done so (e.g., you would not want a push
whose other end has hung up to report success).

This distinction doesn't typically matter in git, because we
do not ignore SIGPIPE in the first place. Which means that
we will not get EPIPE, but instead will just die when we get
a SIGPIPE. But it's possible for a default handler to be set
by a parent process, or for us to add a callsite inside one
of our few SIGPIPE-ignoring blocks of code.

This patch converts write_or_die to actually raise SIGPIPE
when we see EPIPE, rather than exiting with zero. This
brings the behavior in line with the "normal" case that we
die from SIGPIPE (and any callers who want to check why we
died will see the same thing). We also give the same
treatment to other related functions, including
write_or_whine_pipe and maybe_flush_or_die.

Signed-off-by: Jeff King <peff@peff.net>
---
 write_or_die.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/write_or_die.c b/write_or_die.c
index 960f448..b50f99a 100644
--- a/write_or_die.c
+++ b/write_or_die.c
@@ -1,5 +1,15 @@
 #include "cache.h"
 
+static void check_pipe(int err)
+{
+	if (err == EPIPE) {
+		signal(SIGPIPE, SIG_DFL);
+		raise(SIGPIPE);
+		/* Should never happen, but just in case... */
+		exit(141);
+	}
+}
+
 /*
  * Some cases use stdio, but want to flush after the write
  * to get error handling (and to get better interactive
@@ -34,8 +44,7 @@ void maybe_flush_or_die(FILE *f, const char *desc)
 			return;
 	}
 	if (fflush(f)) {
-		if (errno == EPIPE)
-			exit(0);
+		check_pipe(errno);
 		die_errno("write failure on '%s'", desc);
 	}
 }
@@ -50,8 +59,7 @@ void write_or_die(int fd, const void *buf, size_t count)
 void write_or_die(int fd, const void *buf, size_t count)
 {
 	if (write_in_full(fd, buf, count) < 0) {
-		if (errno == EPIPE)
-			exit(0);
+		check_pipe(errno);
 		die_errno("write error");
 	}
 }
@@ -59,8 +67,7 @@ int write_or_whine_pipe(int fd, const void *buf, size_t count, const char *msg)
 int write_or_whine_pipe(int fd, const void *buf, size_t count, const char *msg)
 {
 	if (write_in_full(fd, buf, count) < 0) {
-		if (errno == EPIPE)
-			exit(0);
+		check_pipe(errno);
 		fprintf(stderr, "%s: write error (%s)\n",
 			msg, strerror(errno));
 		return 0;
-- 
1.8.2.rc0.9.g352092c

  parent reply	other threads:[~2013-02-20 20:02 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-20 19:51 [PATCHv3 0/19] pkt-line cleanups and fixes Jeff King
2013-02-20 19:53 ` [PATCH v3 01/19] upload-pack: use get_sha1_hex to parse "shallow" lines Jeff King
2013-02-20 19:54 ` [PATCH v3 02/19] upload-pack: do not add duplicate objects to shallow list Jeff King
2013-02-20 19:55 ` [PATCH v3 03/19] upload-pack: remove packet debugging harness Jeff King
2013-02-20 20:00 ` [PATCH v3 04/19] fetch-pack: fix out-of-bounds buffer offset in get_ack Jeff King
2013-02-20 20:00 ` [PATCH v3 05/19] send-pack: prefer prefixcmp over memcmp in receive_status Jeff King
2013-02-20 20:00 ` [PATCH v3 06/19] upload-archive: do not copy repo name Jeff King
2013-02-20 20:01 ` [PATCH v3 07/19] upload-archive: use argv_array to store client arguments Jeff King
2013-02-20 20:01 ` Jeff King [this message]
2013-02-20 21:51   ` [PATCH v3 08/19] write_or_die: raise SIGPIPE when we get EPIPE Jonathan Nieder
2013-02-20 21:58     ` Jeff King
2013-02-20 22:01       ` Jonathan Nieder
2013-02-20 22:03         ` Jeff King
2013-02-20 22:06           ` Jonathan Nieder
2013-02-20 22:12             ` Jeff King
2013-02-20 22:19               ` Junio C Hamano
2014-03-28  8:35   ` [BUG] MSVC: error box when interrupting `gitlog` by quitting less Marat Radchenko
2014-03-28  9:14     ` Marat Radchenko
2014-03-28  9:44       ` Jeff King
2014-03-28 10:07         ` Marat Radchenko
2014-03-28 10:19           ` Jeff King
2014-03-28 10:28           ` Johannes Sixt
2014-03-28 11:19             ` [PATCH] MSVC: link in invalidcontinue.obj for better POSIX compatibility Marat Radchenko
2014-03-28 18:27               ` Junio C Hamano
2014-03-28 18:46                 ` Marat Radchenko
2014-03-28 19:06                 ` Junio C Hamano
2014-03-28 20:08                   ` [PATCH v2] " Marat Radchenko
2014-03-28 20:35                     ` Junio C Hamano
2013-02-20 20:01 ` [PATCH v3 09/19] pkt-line: move a misplaced comment Jeff King
2013-02-20 20:01 ` [PATCH v3 10/19] pkt-line: drop safe_write function Jeff King
2013-02-20 20:02 ` [PATCH v3 11/19] pkt-line: provide a generic reading function with options Jeff King
2013-02-20 20:02 ` [PATCH v3 12/19] pkt-line: teach packet_read_line to chomp newlines Jeff King
2013-02-20 20:02 ` [PATCH v3 13/19] pkt-line: move LARGE_PACKET_MAX definition from sideband Jeff King
2013-02-20 20:02 ` [PATCH v3 14/19] pkt-line: provide a LARGE_PACKET_MAX static buffer Jeff King
2013-02-20 20:04 ` [PATCH v3 15/19] pkt-line: share buffer/descriptor reading implementation Jeff King
2013-02-22 11:22   ` Eric Sunshine
2013-02-20 20:06 ` [PATCH v3 16/19] teach get_remote_heads to read from a memory buffer Jeff King
2013-02-20 20:07 ` [PATCH v3 17/19] remote-curl: pass buffer straight to get_remote_heads Jeff King
2013-02-20 20:07 ` [PATCH v3 18/19] remote-curl: move ref-parsing code up in file Jeff King
2013-02-20 20:07 ` [PATCH v3 19/19] remote-curl: always parse incoming refs 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=20130220200136.GH25647@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=spearce@spearce.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).