git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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é
Previous: David KastrupNext: David Kastrup
Message 17 of 22 in “general style: replaces memcmp() with proper starts_with()”
  1. general style: replaces memcmp() with proper starts_with()Quint Guvernator, Mar 12, 2014
  2. Jeff KingMar 12, 2014
  3. Junio C HamanoMar 12, 2014
  4. Jeff KingMar 12, 2014
  5. Junio C HamanoMar 12, 2014
  6. Jeff KingMar 12, 2014
  7. Jeff KingMar 12, 2014
  8. Jeff KingMar 12, 2014
  9. Junio C HamanoMar 12, 2014
  10. Quint GuvernatorMar 13, 2014
  11. Junio C HamanoMar 13, 2014
  12. Jeff KingMar 14, 2014
  13. Quint GuvernatorMar 14, 2014
  14. Junio C HamanoMar 14, 2014
  15. René ScharfeMar 12, 2014
  16. David KastrupMar 12, 2014
  17. René ScharfeMar 12, 2014
  18. David KastrupMar 12, 2014
  19. Junio C HamanoMar 12, 2014
  20. David KastrupMar 13, 2014
  21. Junio C HamanoMar 13, 2014
  22. Jeff KingMar 13, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.