From: Fabian Stelzer Date: Wed, 04 Aug 2021 09:01:53 GMT Subject: Re: [PATCH v7 6/9] ssh signing: verify signatures using ssh-keygen Message-ID: In-Reply-To: On 04.08.21 01:47, Junio C Hamano wrote: > "Fabian Stelzer via GitGitGadget" writes: > >> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c >> index 2d1f97e1ca7..05dc8e160f8 100644 >> --- a/builtin/receive-pack.c >> +++ b/builtin/receive-pack.c >> @@ -131,6 +131,10 @@ static int receive_pack_config(const char *var, const char *value, void *cb) >> { >> int status = parse_hide_refs_config(var, value, "receive"); >> >> + if (status) >> + return status; >> + >> + status = git_gpg_config(var, value, NULL); >> if (status) >> return status; > > Hmph, it feels a bit odd for a misconfigured "transfer.hiderefs" to > prevent GPG related configuration from getting read, but is this > because a failure from receive_pack_config() will immediately kill > the process without doing any harm to the system? If so, the code > is good as written. > I think i misunderstood the comment from Jonathan about this. He wrote: "Check the return value of git_gpg_config() to see if that config was processed by that function - if yes, we can return early." Looking at git_gpg_config i don't think i can actually determine by its return code if a value was successfully processed (it will also return 0 when nothing happened). The return in case parse_hide_refs fails was already in place before my change and returning on git_gpg_config failure is done in most of the other commands calling it. builtin/send-pack.c is the exception but i have no idea why. Generally i think a broken config should die() early as it does in this case with the return.