Jeff King wrote:
> 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,
Not so much "default" as "insane inherited", as in the example
of old versions of Python's subprocess.Popen.
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.
[...]
> --- 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);
How about
die("BUG: another thread changed SIGPIPE handling behind my back!");
to make it easier to find and fix such problems?
Thanks,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html