Am 09.02.24 um 23:11 schrieb Junio C Hamano:
> René Scharfe <l.s.r@xxxxxx> 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
