Subject: Re: [PATCH 1/2] receive-pack: use find_commit_header() in check_cert_push_options()

From: Junio C Hamano <[email-masked]>
Date: Fri, 09 Feb 2024 14:11:35 -0800

René Scharfe <[email-masked]> writes:

> The string comparison becomes more complicated because we need to check
> for NUL explicitly after comparing the length-limited option, but on the
> flip side we don't need to clean up allocations or track the remaining
> buffer length.

Yeah, the strncmp() followed by the termination check indeed is
trickier but not having to worry about allocation is nice.

>  		if (options_seen > push_options->nr
> -		    || strcmp(option,
> -			      push_options->items[options_seen - 1].string)) {

We used to allocate option[] with NUL termination, but ...

> -			retval = 0;
> -			goto leave;
> -		}
> -		free(option);
> +		    || strncmp(push_options->items[options_seen - 1].string,
> +			       option, optionlen)
> +		    || push_options->items[options_seen - 1].string[optionlen])

... now option[] is a borrowed memory, option[optionlen] would have
been NUL if we were allocating.  So to see if the last-seen string[]
is different from option[], we have to see that they match up to
optionlen and the last-seen string[] ends there.  Trickier than
before, but is correct.

> +			return 0;
>  	}
>
>  	if (options_seen != push_options->nr)
>  		retval = 0;
>
> -leave:
> -	free(option);
>  	return retval;
>  }
>
> --
> 2.43.0