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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 27, 2014, 19:26 UTC
Message-ID: <xmqqsikhd98f.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20140827191414.GA7561@peff.net>

Jeff King <peff@peff.net> writes:

Show 5 quoted lines

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

Show 7 quoted lines

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