Re: [PATCH/RFC] builtin/tag.c: move PGP verification inside builtin.

[Jeff King <peff@peff.net>]

On Thu, Mar 24, 2016 at 05:51:05PM -0400, Santiago Torres wrote:

> Sorry for the delay with this, I got caught up with coursework.

No problem. The project moves forward as contributor time permits.

> This is my first stab at this, in the dumbest/simplest way imaginable. I
> don't like that there is no code reuse (the run_gpg_verify function is
> repeated here and in the plumbing command). I would appreciate pointers
> on what would be the best way to avoid this.

It looks to me like you could factor the repeated code into a common
verify_tag(), but maybe I am missing something.

> I also spent quite some time figuring out what you meant with
> 
> > Do note the trickery with SIGPIPE in verify-tag, though. We probably
> > need to do the same here (in fact, I wonder if that should be pushed
> > down into the code that calls gpg).
> I don't see any explicit SIGPIPE trickery here. Any pointers?

There is a call to ignore SIGPIPE in builtin/verify-tag.c, line 100.
Do we need to do be doing the same thing here?

There's some discussion in the thread starting at:

  http://thread.gmane.org/gmane.comp.version-control.git/53878/focus=53904

The claim there is that we get SIGPIPE and die early if we feed gpg a
tag which isn't signed. We _should_ be catching that case already via
parse_signature(), though I wonder if it can be fooled (e.g., something
that looks like a signature, but when gpg parses it, it turns out to be
bogus). So we should probably continue ignoring SIGPIPE to be on the
safe side.

But I notice that we already handle SIGPIPE explicitly in sign_buffer()
for similar reasons.  What I was wondering earlier was whether we should
teach other functions that call gpg (like verify_signed_buffer()) to
ignore SIGPIPE, too, so that we can return a reasonable error value
rather than just killing the whole program.

-Peff