Re: [PATCH v7 6/9] ssh signing: verify signatures using ssh-keygen

On 04.08.21 01:47, Junio C Hamano wrote:

"Fabian Stelzer via GitGitGadget" [off-list ref] writes:

quoted

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.