[PATCH 2/9] ref-filter: avoid extra copies of payload/signature

When we know we're going to show the subject or body of a tag or commit, we call find_subpos(), which returns pointers and lengths for the three parts: subject, body, signature.

Oddly, the function finds the signature twice: once by calling parse_signature() at the start, which copies the signature into a separate strbuf, and then again by calling parse_signed_buffer() after we've parsed past the subject.

This is due to 482c119186 (gpg-interface: improve interface for parsing tags, 2021-02-11) and 88bce0e24c (ref-filter: hoist signature parsing, 2021-02-11). The idea is that in a multi-hash world, tag signatures may appear in the header, rather than at the end of the body, in which case we need to extract them into a separate buffer.

But parse_signature() would never find such a buffer! It only looks for signature lines (like "-----BEGIN PGP") at the start of each line, without any header keyword. So this code will never find anything except the usual in-body signature.

And the extra code has two downsides:

  1. We spend time copying the payload and signature into strbufs. That
     might even be useful if we ended up with a NUL-terminated copy of
     the payload data, but we throw it away immediately. And the
     signature, since it comes at the end of the message, is already its
     own NUL-terminated buffer.

     The overhead isn't huge, but I measured a pretty consistent 1-2%
     speedup running "git for-each-ref --format='%(subject)'" with this
     patch on a clone of linux.git.

  2. The output of find_subpos() is a set of three ptr/len combinations,
     but only two of them point into the original buffer. This makes the
     interface confusing: you can't do pointer comparisons between them,
     and you have to remember to free the signature buffer. Since
     there's only one caller, it's not too bad in practice, but it did
     bite me while working on the next patch (and simplifying it will
     pave the way for that).

In the long run we might have to go back to something like this approach, if we do have multi-hash header signatures. But I would argue that the extra buffer should kick in only for a header signature, and be passed out of find_subpos() separately.

Signed-off-by: Jeff King <peff@peff.net>
---
This should produce no behavior change.

It does seem funny to me that we do this signature parsing for commits as well as tags, even through the former would have an in-header signature. So arguably we are wrongly cutting in-body signatures from commits, and not showing their signatures with %(contents:signature). (But note that %(signature) is its own thing and does handle commits correctly).

So I don't know if we'd want to fix that or not, but I left it as-is in this series. And as I said above, if we did want to go that way, I think we'd still want to build it on top of this, so that find_subpos() returns the in-header and in-body signatures separately.

 ref-filter.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)