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

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

>> Blindly replacing starts_with() with !memcmp() in the above part is
>> a readability regression otherwise.
>
> I actually think the right solution is:
>
>   static inline int standard_header_field(const char *field, size_t len)
>   {
>           return mem_equals(field, len, "tree ") ||
>                  mem_equals(field, len, "parent ") ||
>                  ...;
>   }
>
> and the caller should tell us it's OK to look at field[len]:
>
>   standard_header_field(line, eof - line + 1)
>
> We could also omit the space from the standard_header_field.

Yes, that was what I had in mind.  The only reason why the callee
(over-)optimizes the "SP must follow these know keywords" part by
using the extra "len" parameter is because the caller has to do a
single strchr() to skip an arbitrary field name anyway so computing
"len" is essentially free.

> The caller
> just ran strchr() looking for the space, so we know that either it is
> there, or we are at the end of the line/buffer. Arguably a string like
> "parent\n" should be "a parent header with no data" (but right now it is
> not matched by this function). I'm not aware of an implementation that
> writes such a thing, but it seems to fall in the "be liberal in what you
> accept" category.

It is _not_ a standard header field, so it will be read by the logic
in the caller as a non-standard header field without getting lost.