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

From: Patrick Steinhardt <ps@pks.im>
Date: Sep 10, 2024, 06:09 UTC
Message-ID: <Zt_ikkhuMrqVcFOx@pks.im>
In-Reply-To: <20240909231228.GB921834@coredump.intra.peff.net>

On Mon, Sep 09, 2024 at 07:12:28PM -0400, Jeff King wrote:

Show 19 quoted lines

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

Okay. So in other words the intent was to parse in-header signatures, but the code failed to do so correctly and thus this never worked in the first place?

In any case, `parse_signature()` is only a glorified wrapper around `parse_signed_buffer()` in the first place, so in the end they would both parse the buffer in the same way.

Nice cleanup, even though it leaves one wondering why the in-header signatures have only been wired up partially.

Patrick