On Tue, Feb 28, 2012 at 04:05:40AM -0500, Jeff King wrote:
> > } else if (use_message) {
> > buffer = strstr(use_message_buffer, "\n\n");
> > - if (!buffer || buffer[2] == '\0')
> > + if (!amend && !edit_message && (!buffer || buffer[2] == '\0'))
> > die(_("commit has empty message"));
>
> Hmm. So "buffer" used to never be NULL (because we would die if it is),
> and now we might not die if we are doing an amend, no? And the next line
> is:
>
> > strbuf_add(&sb, buffer + 2, strlen(buffer + 2));
>
> Doesn't this need to handle the case of NULL buffer (i.e., when it does
> not already have "\n\n" in it)?
I wrote that after looking at just your patch. Looking at
builtin/commit.c, I think use_message_buffer will always be a re-encoded
commit object. So that strstr should _never_ fail unless the commit
object is corrupt. So the right thing is probably:
buffer = strstr(use_message_buffer, "\n\n");
if (!buffer)
die(_("commit object has invalid format"));
if (!amend && !edit_message && buffer[2] == '\0))
die(_("commit has empty message"));
-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