Subject: Re: [PATCH] general style: replaces memcmp() with proper starts_with()

From: David Kastrup <[email-masked]>
Date: Wed, 12 Mar 2014 22:16:13 +0100

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

> Am 12.03.2014 20:39, schrieb Junio C Hamano:
>> Jeff King <[email-masked]> writes:
>>
>>>>   static inline int standard_header_field(const char *field, size_t len)
>>>>   {
>>>> -	return ((len == 4 && !memcmp(field, "tree ", 5)) ||
>>>> -		(len == 6 && !memcmp(field, "parent ", 7)) ||
>>>> -		(len == 6 && !memcmp(field, "author ", 7)) ||
>>>> -		(len == 9 && !memcmp(field, "committer ", 10)) ||
>>>> -		(len == 8 && !memcmp(field, "encoding ", 9)));
>>>> +	return ((len == 4 && starts_with(field, "tree ")) ||
>>>> +		(len == 6 && starts_with(field, "parent ")) ||
>>>> +		(len == 6 && starts_with(field, "author ")) ||
>>>> +		(len == 9 && starts_with(field, "committer ")) ||
>>>> +		(len == 8 && starts_with(field, "encoding ")));
>>>
>>> These extra "len" checks are interesting.  They look like an attempt to
>>> optimize lookup, since the caller will already have scanned forward to
>>> the space.
>
> I wonder what the performance impact might be.  The length checks are
> also needed for correctness, however, to avoid running over the end of
> the buffer.

Depends on whether memcmp is guaranteed to stop immediately on mismatch.
Then memcmp(field, "tree ", 5) cannot walk across a NUL byte in field.

-- 
David Kastrup