On Thu, Sep 11, 2025 at 6:55 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Patrick Steinhardt <ps@xxxxxx> writes:
>
> >> +/* Process signatures (up to 2: one "sha1" and one "sha256") */
> >
> > Hm. Does "up to 2" indicate that the commit may have two signatures at
> > once? If so...
> >
> >> +static void import_signature(struct signature_data *sig_sha1,
> >> + struct signature_data *sig_sha256,
> >> + const char *v)
> >> +{
> >> + struct signature_data sig = { NULL, NULL, STRBUF_INIT };
> >> +
> >> + parse_one_signature(&sig, v);
> >> +
> >> + if (!strcmp(sig.hash_algo, "sha1"))
> >> + store_signature(sig_sha1, &sig, "SHA-1");
> >> + else if (!strcmp(sig.hash_algo, "sha256"))
> >> + store_signature(sig_sha256, &sig, "SHA-256");
> >
> > ... then the code here seems to indicate otherwise as you only parse
> > either the "sha1" signature or the "sha256" signature, but never both.
>
> Correct and not quite. The caller can call you twice in its loop.
> But if the input was malformed and had two "sha1" (and no "sha256"),
> this will not barf (as the original, so it is not a new bug).
store_signature() should barf if it has already been called with the
same first argument:
static void store_signature(struct signature_data *stored_sig,
struct signature_data *new_sig,
const char *hash_type)
{
if (stored_sig->hash_algo) {
warning("multiple %s signatures found, "
"ignoring additional signature",
hash_type);
strbuf_release(&new_sig->data);
free(new_sig->hash_algo);
} else {
*stored_sig = *new_sig;
}
}
> In any case, I also found that "up to 2" comment somewhat strange.
> It was more understandable back when it was near the loop, but not
> here.
In V2 I just removed that comment, as I am not sure it really helps.
Thanks.
