Re: [PATCH] general style: replaces memcmp() with proper starts_with()
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 12, 2014, 20:08 UTC
- Message-ID
- <xmqq61njkwnw.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <20140312194943.GA2912@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 17 quoted lines
>> 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.
Show 7 quoted lines
> 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.