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

From: Junio C Hamano <[email-masked]>
Date: Wed, 27 Aug 2014 12:26:56 -0700

Jeff King <[email-masked]> writes:

> 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.

Fair enough.

> 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.

True.

> 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.

I agree that we do not want to worry too much about the minority
multi-line callers.

I don't mind returning -1 in out_len and have the callers check.
That way will allow callers to easily diagnose this

	tree $T
        author $GIT_AUTHOR_IDENT
	committer $GIT_COMMITTER_IDENT
	encoding encoding
         unexpected continuation line

	log message

as an error; they would just make sure that out_len is not the "this
is continued; you need to parse the rest yourself" value.

Otherwise, the callers need to check value[out_len+1] to see if it
is an SP (value[out_len] must be '\n') to catch the error, I would
think.