All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Patrick Reynolds <patrick.reynolds@github.com>,
	git@vger.kernel.org, Chris Packham <judge.packham@gmail.com>,
	Johannes Sixt <j6t@kdbg.org>
Subject: Re: [PATCH] unblock and unignore SIGPIPE
Date: Wed, 17 Sep 2014 04:11:49 -0400	[thread overview]
Message-ID: <20140917081148.GB16200@peff.net> (raw)
In-Reply-To: <xmqqd2av1bsg.fsf@gitster.dls.corp.google.com>

On Tue, Sep 16, 2014 at 02:43:43PM -0700, Junio C Hamano wrote:

> > +/* un-ignore and un-block SIGPIPE */
> > +void sanitize_signals(void)
> > +{
> > +	sigset_t unblock;
> > +
> > +	sigemptyset(&unblock);
> > +	sigaddset(&unblock, SIGPIPE);
> > +	sigprocmask(SIG_UNBLOCK, &unblock, NULL);
> > +	signal(SIGPIPE, SIG_DFL);
> 
> With the only caller in git.c, there is not a good reason that we
> would want to have this as a global in a different file (I think the
> patch merely follows the pattern of sanitize-fds, but that one has
> to be called from many places).

Would we want to call it from external C commands, too? For the most
part, git.c is the entry point for running git commands, and any
sanitizing it does will be inherited by sub-commands. But it _is_ still
legal to call dashed commands individually, and even required in some
cases (e.g., git-upload-pack for ssh clients).

> > diff --git a/t/t0012-sigpipe.sh b/t/t0012-sigpipe.sh
> > new file mode 100755
> > index 0000000..213cde3
> > --- /dev/null
> > +++ b/t/t0012-sigpipe.sh
> > @@ -0,0 +1,27 @@
> > +#!/bin/sh
> 
> Hmmm, do we really need to allocate a new test number just for these
> two tests, instead of folding it into an existing one?

I see in your proposed patch below you put them into t0000. I wonder if
t0005 would be a more obvious place.

-Peff

  reply	other threads:[~2014-09-17  8:11 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-15  5:29 [PATCH] unblock and unignore SIGPIPE Patrick Reynolds
2014-08-18  1:14 ` Eric Wong
2014-08-22  5:40   ` Patrick Reynolds
2014-09-16 21:43 ` Junio C Hamano
2014-09-17  8:11   ` Jeff King [this message]
2014-09-17 23:02     ` Junio C Hamano
2014-09-18 14:35     ` Patrick Reynolds
2014-09-18 15:04       ` Junio C Hamano

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=20140917081148.GB16200@peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=judge.packham@gmail.com \
    --cc=patrick.reynolds@github.com \
    /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.