From: René Scharfe <l.s.r@web.de>
Date: Wed, 12 Mar 2014 21:45:15 GMT
Subject: Re: [PATCH] general style: replaces memcmp() with proper starts_with()
Message-ID: <5320D56B.8060101@web.de>
In-Reply-To: <87fvmni0de.fsf@fencepost.gnu.org>

Am 12.03.2014 22:16, schrieb David Kastrup:
> René Scharfe <l.s.r@web.de> writes:
>
>> Am 12.03.2014 20:39, schrieb Junio C Hamano:
>>> Jeff King <peff@peff.net> 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.

I'm not sure we can rely on that property.  But anyway -- if field 
points to, say, a one-byte buffer containing the letter t, then memcmp 
would overrun that buffer.  If field was guaranteed to be NUL-terminated 
then at least starts_with would stop at the end, but the signature of 
standard_header_field() suggests that it can be used with arbitrary 
buffers, not just C strings.

René