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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to