Re: [PATCH v2 3/6] ssh signing: make verify-commit consider key lifetime

From: Fabian Stelzer <fs@gigacodes.de>
Date: Oct 28, 2021, 08:01 UTC
Message-ID: <6a8748aa-9ed9-8aec-860b-a7f9b0ce9504@gigacodes.de>
In-Reply-To: <xmqqsfwmus5g.fsf@gitster.g>

On 27.10.21 22:30, Junio C Hamano wrote:

Show 25 quoted lines

> Fabian Stelzer <fs@gigacodes.de> writes:
> 
>> +static int parse_payload_metadata(struct signature_check *sigc)
>> +{
>> +	const char *ident_line = NULL;
>> +	size_t ident_len;
>> +	struct ident_split ident;
>> +	const char *signer_header;
>> +
>> +	switch(sigc->payload_type) {
>> +		case SIGNATURE_PAYLOAD_COMMIT:
>> +			signer_header = "committer";
>> +			break;
>> +		case SIGNATURE_PAYLOAD_TAG:
>> +			signer_header = "tagger";
>> +			break;
>> +		default:
>> +			/* Ignore unknown payload types */
>> +			return 0;
>> +	}
> 
> The case arms should be indented to the same level as opening
> switch().  Have SP between keyword "switch" and the expression
> the statement switches on.
>

Thanks, will fix.

Show 7 quoted lines

> More importantly, can you explain why it is necessary to allow
> callers to call this function with a random value in payload_type
> and have it silently succeed?  Isn't it a programming error that
> deserves a call to BUG("...")?
> 
> Thanks.
>

Not really. I will add a BUG() to the default case and only ignore the known cases that we don't want to handle.

Thanks