On Sat, Sep 05, 2015 at 09:56:27PM -0700, Junio C Hamano wrote:
> +static void am_signoff(struct strbuf *sb)
> +{
> + char *cp;
> + struct strbuf mine = STRBUF_INIT;
> +
> + /* Does it end with our own sign-off? */
> + strbuf_addf(&mine, "\n%s%s\n",
> + sign_off_header,
> + fmt_name(getenv("GIT_COMMITTER_NAME"),
> + getenv("GIT_COMMITTER_EMAIL")));
> + if (mine.len < sb->len &&
> + !strcmp(mine.buf, sb->buf + sb->len - mine.len))
> + goto exit; /* no need to duplicate */
Here you insert the "\n" directly at the start of "mine", so the test
"does it contain S-o-b at the beginning of a line" does not count the
first line. Probably fine, as somebody putting a S-o-b in their subject
deserves whatever they get.
But...
> + /* Does it have any Signed-off-by: in the text */
> + for (cp = sb->buf;
> + cp && *cp && (cp = strstr(cp, sign_off_header)) != NULL;
> + cp = strchr(cp, '\n')) {
> + if (sb->buf == cp || cp[-1] == '\n')
> + break;
> + }
Here you are more careful about finding S-o-b at sb->buf.
I don't think it really matters in practice, but it's an interesting
inconsistency.
Other than that (and I do not think it is worth re-rolling for this;
just an interesting observation), the patch looks OK to me.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html