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

[Jeff King <peff@peff.net>]

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

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

Hmph. I was hoping that callers could just ignore this and fallback to
the "it's supposed to be one line, so assume that it is, and ignore
other lines" behavior. I guess that is the flip side of the "just use
the whole broken value" alternative that I mentioned earlier.

What I didn't want to do is deal with it in each callsite, like:

diff --git a/builtin/commit.c b/builtin/commit.c
index 7f9f071..10417bb 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -564,6 +564,8 @@ static void determine_author_info(struct strbuf *author_ident)
 		a = find_commit_header(author_message_buffer, "author", &len);
 		if (!a)
 			die(_("commit '%s' lacks author header"), author_message);
+		if (len == -1)
+			die(_("commit '%s' has a multi-line author"), author_message);
 		if (split_ident_line(&ident, a, len) < 0)
 			die(_("commit '%s' has malformed author line"), author_message);
 
diff --git a/commit.c b/commit.c
index 9416d84..6fec0c2 100644
--- a/commit.c
+++ b/commit.c
@@ -594,6 +594,8 @@ static void record_author_date(struct author_date_slab *author_date,
 	ident_line = find_commit_header(buffer, "author", &ident_len);
 	if (!ident_line)
 		goto fail_exit; /* no author line */
+	if (ident_len == -1)
+		goto fail_exit; /* multi-line author line */
 	if (split_ident_line(&ident, ident_line, ident_len) ||
 	    !ident.date_begin || !ident.date_end)
 		goto fail_exit; /* malformed "author" line */
@@ -1669,7 +1671,10 @@ const char *find_commit_header(const char *msg, const char *key, size_t *out_len
 		if (eol - line > key_len &&
 		    !strncmp(line, key, key_len) &&
 		    line[key_len] == ' ') {
-			*out_len = eol - line - key_len - 1;
+			if (eol[0] && eol[1] == ' ')
+				*out_len = -1;
+			else
+				*out_len = eol - line - key_len - 1;
 			return line + key_len + 1;
 		}
 		line = *eol ? eol + 1 : NULL;
diff --git a/pretty.c b/pretty.c
index 3a70557..ff2bf4a 100644
--- a/pretty.c
+++ b/pretty.c
@@ -551,7 +551,7 @@ static char *get_header(const char *msg, const char *key)
 {
 	size_t len;
 	const char *v = find_commit_header(msg, key, &len);
-	return v ? xmemdupz(v, len) : NULL;
+	return v && len != -1 ? xmemdupz(v, len) : NULL;
 }
 
 static char *replace_encoding_header(char *buf, const char *encoding)


which does not really add any value, IMHO.

-Peff