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

[Jeff King <peff@peff.net>]

On Wed, Mar 12, 2014 at 05:14:15PM -0400, Jeff King wrote:

> I also think that "eof = next" (which I retained here) is off-by-one.
> "next" here is not the newline, but the start of the next line. And I'm
> guessing the code actually wanted the newline (otherwise "it->key" ends
> up with the newline in it). But we cannot just subtract one, because if
> we hit "eob", it really is in the right spot.

I started on a patch for this, but found another interesting corner
case. If we do not find a newline and hit end-of-buffer (which
_shouldn't_ happen, as that is a malformed commit object), we set "next"
to "eob". But then we copy the whole string, including *next into the
"value" of the header.

So we intend to capture the trailing newline in the value, and do in the
normal case. But in the end-of-buffer case, we capture an extra NUL. I
think that's OK, because the eventual fate of the buffer is to become a
C-string. But it does mean that the result sometimes has a
newline-terminator and sometimes doesn't, and the calling code needs to
handle this when printing, or risk lines running together.

Should this function append a newline if there is not already one? If
it's a mergetag header, we feed the result to gpg, etc, and expect to
get the data out verbatim. We would not want to mess that up. OTOH, the
commit object is bogus (and possibly truncated) in the first place, so
it probably doesn't really matter. And the GPG signature on tag objects
has its own delimiters, so a stray newline present or not at the end
should not matter.

-Peff