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

From: Jeff King <peff@peff.net>
Date: Mar 12, 2014, 21:39 UTC
Message-ID: <20140312213944.GA12818@sigill.intra.peff.net>
In-Reply-To: <20140312211415.GA10305@sigill.intra.peff.net>

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

Show 5 quoted lines

> 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