Re: [PATCH v2 1/6] commit: provide a function to find a header in a buffer

From: Jeff King <peff@peff.net>
Date: Aug 27, 2014, 19:14 UTC
Message-ID: <20140827191414.GA7561@peff.net>
In-Reply-To: <xmqqwq9tda8t.fsf@gitster.dls.corp.google.com>

On Wed, Aug 27, 2014 at 12:05:06PM -0700, Junio C Hamano wrote:

Show 15 quoted lines

> > If you mean including continuation lines in the output, I don't think
> > that's a good idea here. It would mean the function would have to copy
> > the value out (to get rid of the continuation whitespace) rather than
> > point directly into the msg buffer.
> 
> I meant the counting of out_len.  You do not copy the contents for
> the caller for a single line case either, so I wouldn't expect it.
> 
> You locate where the stuff begins to make it easier for the caller
> to read or copy, and the caller may choose to stop reading from the
> location up to out_len bytes, or it may choose to ignore out_len and
> stop at the first newline.  For the latter kind of caller, it does
> not matter if out_len does not point at the end of the "field"
> (i.e. for a continued-line case), but for the former, wouldn't it be
> more useful if out_len told where the end of the "field" is?

I think they are a direct tradeoff. If you include only the first line, then callers who want multiple lines have to keep parsing. If you include multiple lines, then callers who care only about the first line will have to re-find the newline rather than just using "out_len" directly.

I suppose you could argue that people who are only expecting one line (e.g., "encoding") should just assume that out_len ends at the first line. For correctly-formatted commits, that works the same under either scheme. For a broken commit where "encoding" _is_ multi-line, one case would ignore the continued bits and the other case would return an unexpected encoding value with newlines in it. The choice probably doesn't matter much in practice.

Mostly I just punted on it with a comment since I did not plan to add any multi-line callers, and I figured we could sort it out then. If you feel strongly, it should be pretty easy to check for continuation and extend out_len if necessary.

-Peff