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

From: Fabian Stelzer <fs@gigacodes.de>
Date: Aug 4, 2021, 09:01 UTC
Message-ID: <fb7c504f-90f7-9a79-1903-e3942f43e808@gigacodes.de>
In-Reply-To: <xmqqzgtyrszl.fsf@gitster.g>

On 04.08.21 01:47, Junio C Hamano wrote:

Show 23 quoted lines

> "Fabian Stelzer via GitGitGadget" <gitgitgadget@gmail.com> 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.