Thanks for the report.
The patch did not preserve the exact behaviour of
the previous code. Rather than calling BUG() whenever a trust level is out
of the sigcheck_gpg_trust_level[] array, we can simply return an empty string
if (level < 0 || level >= ARRAY_SIZE(sigcheck_gpg_trust_level))
return "";
It will replicate the exact behaviour as the previous code. But as
Jeff pointed out,
Should this really be the defined behavior?
Let me know what you think. I will make the necessary changes.
Thanks,
Jaydeep.
On Tue, Apr 18, 2023 at 12:18 PM Jeff King <peff@xxxxxxxx> wrote:
>
> On Tue, Apr 18, 2023 at 08:12:03AM +0200, Rolf Eike Beer wrote:
>
> > When I now run "git log" in a repository that contains commits signed by
> > people not in my keyring (e.g. the Gentoo git) I get this backtrace:
> >
> > BUG: gpg-interface.c:915: invalid trust level requested -1
>
> Thanks for giving an example repo. After cloning:
>
> https://anongit.gentoo.org/git/repo/gentoo.git
>
> I can reproduce just by running "git log -1 --format=%GT". Bisecting
> turns up 803978da49 (gpg-interface: add function for converting trust
> level to string, 2022-07-11), which is not too surprising.
>
> Before that we returned an empty string. I don't know if the fix is a
> simple as:
>
> diff --git a/gpg-interface.c b/gpg-interface.c
> index aceeb08336..edb0da1bda 100644
> --- a/gpg-interface.c
> +++ b/gpg-interface.c
> @@ -934,7 +934,10 @@ const char *gpg_trust_level_to_str(enum signature_trust_level level)
> {
> struct sigcheck_gpg_trust_level *trust;
>
> - if (level < 0 || level >= ARRAY_SIZE(sigcheck_gpg_trust_level))
> + if (level < 0)
> + return "";
> +
> + if (level >= ARRAY_SIZE(sigcheck_gpg_trust_level))
> BUG("invalid trust level requested %d", level);
>
> trust = &sigcheck_gpg_trust_level[level];
>
> which restores the original behavior, or if the original was papering
> over another bug (e.g., should this be "undefined"?). Certainly the
> empty string matches other placeholders like %GS for this case (since we
> obviously don't know anything about the signer).
>
> +cc folks who worked on 803978da49.
>
> -Peff
