Re: [PATCH v3 08/19] write_or_die: raise SIGPIPE when we get EPIPE

[Jeff King <peff@peff.net>]

On Wed, Feb 20, 2013 at 01:51:11PM -0800, Jonathan Nieder wrote:

> > 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,
> 
> Not so much "default" as "insane inherited", as in the example
> of old versions of Python's subprocess.Popen.

It's possible that somebody could have a legitimate reason for doing so.
I just can't think of one. :)

> I suspect this used exit(0) instead of raise(SIGPIPE) in the first
> place to work around a bash bug (too much verbosity about SIGPIPE).
> If any programs still have that kind of bug, I'd rather put pressure
> on them to fix it by *not* working around it.  So the basic idea here
> looks good to me.

Yeah, if you look for old discussions on SIGPIPE in the git list, it is
mostly Linus complaining about the bash behavior, and this code does
date back to that era. The bash bug is long since fixed.

> > +	if (err == EPIPE) {
> > +		signal(SIGPIPE, SIG_DFL);
> > +		raise(SIGPIPE);
> > +		/* Should never happen, but just in case... */
> > +		exit(141);
> 
> How about
> 
> 		die("BUG: another thread changed SIGPIPE handling behind my back!");
> 
> to make it easier to find and fix such problems?

You mean for the "should never happen" bit, not the first part, right? I
actually wonder if we should simply exit(141) in the first place. That
is shell exit-code for SIGPIPE death already (so it's what our
run_command would show us, and what anybody running us through shell
would see).

-Peff