git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Santiago Torres <santiago@nyu.edu>
Date
Mar 24, 2016, 22:32 UTC
Message-ID
<20160324223257.GE8830@LykOS>
In-Reply-To
<20160324221457.GB17805@sigill.intra.peff.net>
Show 7 quoted lines
> > 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.
Show 28 quoted lines
> 
> > 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.

Previous: Jeff KingNext: Jeff King
Message 4 of 7 in “builtin/tag.c: move PGP verification inside builtin.”
  1. builtin/tag.c: move PGP verification inside builtin.santiago@nyu.edu, Mar 24, 2016
  2. Santiago TorresMar 24, 2016
  3. Jeff KingMar 24, 2016
  4. Santiago TorresMar 24, 2016
  5. Jeff KingMar 24, 2016
  6. Jeff KingMar 24, 2016
  7. Santiago TorresMar 24, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.