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:38 UTC
- Message-ID
- <20140827193857.GD7561@peff.net>
- In-Reply-To
- <xmqqsikhd98f.fsf@gitster.dls.corp.google.com>
On Wed, Aug 27, 2014 at 12:26:56PM -0700, Junio C Hamano wrote:
Show 13 quoted lines
> 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