Re: [PATCH 1/2] receive-pack: use find_commit_header() in check_cert_push_options()
- From
René Scharfe <l.s.r@web.de>
- Date
- Feb 10, 2024, 07:42 UTC
- Message-ID
- <7b0a87f1-4693-480e-90d4-14675e9a5f01@web.de>
- In-Reply-To
- <xmqqsf21cmp4.fsf@gitster.g>
Am 09.02.24 um 23:11 schrieb Junio C Hamano:
Show 29 quoted lines
> René Scharfe <l.s.r@web.de> 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.I just discovered 14570dc67d (wrapper: add function to compare strings with different NUL termination, 2020-05-25). Perhaps squash this in to simplify?
--- builtin/receive-pack.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index dbee508775..db65607485 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -717,9 +717,8 @@ static int check_cert_push_options(const struct string_list *push_options) buf = option + optionlen + 1; options_seen++; if (options_seen > push_options->nr - || strncmp(push_options->items[options_seen - 1].string, - option, optionlen) - || push_options->items[options_seen - 1].string[optionlen]) + || xstrncmpz(push_options->items[options_seen - 1].string, + option, optionlen)) return 0; } -- 2.43.0