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

[Santiago Torres <santiago@nyu.edu>]

> > 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 see, yes. This looks like the way forward.
> 
> > 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.
> 

Now I get it  I think this should be easy to achieve by moving
verify_tag() to tag.c, along with the static run_gpg_verify functions. I
could move the SIGPIPE call inside the verify-tag command and patch up
everything accordingly. Does this sound ok?

Thanks,
-Santiago.