Re: [PATCH v3 1/4] builtin/verify-tag.c: Ignore SIGPIPE on gpg-interface
- From
Santiago Torres <santiago@nyu.edu>
- Date
- Apr 3, 2016, 21:46 UTC
- Message-ID
- <20160403214606.GB28933@LykOS>
- In-Reply-To
- <5700BD33.6090602@kdbg.org>
On Sun, Apr 03, 2016 at 08:50:27AM +0200, Johannes Sixt wrote:
Show 55 quoted lines
> Am 03.04.2016 um 01:16 schrieb santiago@nyu.edu:
> > From: Santiago Torres <santiago@nyu.edu>
> >
> > The verify_signed_buffer comand might cause a SIGPIPE signal when the
> > gpg child process terminates early (due to a bad keyid, for example) and
> > git tries to write to it afterwards. Previously, ignoring SIGPIPE was
> > done on the builtin/gpg-verify.c command to avoid this issue. However,
> > any other caller who wanted to use the verify_signed_buffer command
> > would have to include this signal call.
> >
> > Instead, we use sigchain_push(SIGPIPE, SIG_IGN) on the
> > verify_signed_buffer call (pretty much like in sign_buffer()) so
> > that any caller is not required to perform this task. This will avoid
> > possible mistakes by further developers using verify_signed_buffer.
> >
> > Signed-off-by: Santiago Torres <santiago@nyu.edu>
> > ---
> > Notes:
> > I dropped the multiline comment altogheter.
> >
> > builtin/verify-tag.c | 3 ---
> > gpg-interface.c | 3 +++
> > 2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
> > index 00663f6..77f070a 100644
> > --- a/builtin/verify-tag.c
> > +++ b/builtin/verify-tag.c
> > @@ -95,9 +95,6 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)
> > if (verbose)
> > flags |= GPG_VERIFY_VERBOSE;
> >
> > - /* sometimes the program was terminated because this signal
> > - * was received in the process of writing the gpg input: */
> > - signal(SIGPIPE, SIG_IGN);
> > while (i < argc)
> > if (verify_tag(argv[i++], flags))
> > had_error = 1;
> > diff --git a/gpg-interface.c b/gpg-interface.c
> > index 3dc2fe3..c1f6b2d 100644
> > --- a/gpg-interface.c
> > +++ b/gpg-interface.c
> > @@ -232,6 +232,8 @@ int verify_signed_buffer(const char *payload, size_t payload_size,
> > if (gpg_output)
> > gpg.err = -1;
> > args_gpg[3] = path;
> > +
> > + sigchain_push(SIGPIPE, SIG_IGN);
> > if (start_command(&gpg)) {
> > unlink(path);
> > return error(_("could not run gpg."));
>
> But no sigchain_pop() in the error path that we see here?
>
> Perhaps you can even defer the sigchain_push() until after start_command()?Yes, after reviewing where the sigchain_push() call is made on sign_buffer(), I should put the push call after starting the command in the same way sign_buffer() does.
Thanks! -Santiago.