Re: [PATCH v5 1/6] builtin/verify-tag.c: Ignore SIGPIPE on gpg-interface

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 6, 2016, 16:43 UTC
Message-ID: <xmqqpou2y9fy.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1459872449-7537-2-git-send-email-santiago@nyu.edu>

santiago@nyu.edu writes:

> Subject: [PATCH v5 1/6] builtin/verify-tag.c: Ignore SIGPIPE on gpg-interface

s/Ignore/ignore/

Show 8 quoted lines

> 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/verify-tag.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.

s/comand/command/ but more importantly, it is not a command ;-)

> 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.

I'd rephrase like this if I were doing this patch:

    verify-tag: ignore SIGPIPE in verify_signed_buffer()

    The verify_signed_buffer() function may trigger a SIGPIPE when
    the GPG child process terminates early (due to a bad keyid, for
    example) and we try to write to it afterwards.  Ignoring SIGPIPE
    is done in builtin/verify-tag.c to avoid dying from it, but any
    other caller who wants to call verify_signed_buffer() would have
    to do the same.

    Use sigchain_push(SIGPIPE, SIG_IGN) in verify_signed_buffer(),
    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().

The patch text (as I already said in the response to the cover letter) looked fine.