All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Brooke Kuhlmann <brooke@alchemists.io>
Subject: Re: [PATCH 2/9] ref-filter: avoid extra copies of payload/signature
Date: Tue, 10 Sep 2024 08:09:22 +0200	[thread overview]
Message-ID: <Zt_ikkhuMrqVcFOx@pks.im> (raw)
In-Reply-To: <20240909231228.GB921834@coredump.intra.peff.net>

On Mon, Sep 09, 2024 at 07:12:28PM -0400, Jeff King wrote:
> 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

  reply	other threads:[~2024-09-10  6:09 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-09 23:07 [PATCH 0/9] ref-filter %(trailer) fixes Jeff King
2024-09-09 23:08 ` [PATCH 1/9] t6300: drop newline from wrapped test title Jeff King
2024-09-09 23:12 ` [PATCH 2/9] ref-filter: avoid extra copies of payload/signature Jeff King
2024-09-10  6:09   ` Patrick Steinhardt [this message]
2024-09-10  6:26     ` Jeff King
2024-09-09 23:14 ` [PATCH 3/9] ref-filter: strip signature when parsing tag trailers Jeff King
2024-09-10  6:08   ` Patrick Steinhardt
2024-09-10  6:28     ` Jeff King
2024-09-09 23:14 ` [PATCH 4/9] ref-filter: drop useless cast in trailers_atom_parser() Jeff King
2024-09-09 23:16 ` [PATCH 5/9] ref-filter: store ref_trailer_buf data per-atom Jeff King
2024-09-10  6:08   ` Patrick Steinhardt
2024-09-09 23:18 ` [PATCH 6/9] ref-filter: fix leak of %(trailers) "argbuf" Jeff King
2024-09-10  6:09   ` Patrick Steinhardt
2024-09-10  6:33     ` Jeff King
2024-09-09 23:19 ` [PATCH 7/9] ref-filter: fix leak with %(describe) arguments Jeff King
2024-09-09 23:19 ` [PATCH 8/9] ref-filter: fix leak when formatting %(push:remoteref) Jeff King
2024-09-10  6:09   ` Patrick Steinhardt
2024-09-09 23:21 ` [PATCH 9/9] ref-filter: add ref_format_clear() function Jeff King
2024-09-10  6:09   ` Patrick Steinhardt
2024-09-10  6:37     ` Jeff King
2024-09-10  6:57 ` [PATCH 10/9] ref-filter: fix leak with unterminated %(if) atoms Patrick Steinhardt
2024-09-10  7:12   ` Jeff King
2024-09-10 16:48   ` Junio C Hamano
2024-09-12 10:22     ` Patrick Steinhardt
2024-09-12 11:18       ` Jeff King
2024-09-12 11:32         ` Patrick Steinhardt
2024-09-12 20:24         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Zt_ikkhuMrqVcFOx@pks.im \
    --to=ps@pks.im \
    --cc=brooke@alchemists.io \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.