Re: [PATCH] general style: replaces memcmp() with proper starts_with()
- From
René Scharfe <l.s.r@web.de>
- Date
- Mar 12, 2014, 21:45 UTC
- Message-ID
- <5320D56B.8060101@web.de>
- In-Reply-To
- <87fvmni0de.fsf@fencepost.gnu.org>
Am 12.03.2014 22:16, schrieb David Kastrup:
Show 28 quoted lines
> 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é