Re: [PATCH] commit: allow {--amend|-c foo} when {HEAD|foo} has empty message

Jeff King [off-list ref] writes:

On Tue, Feb 28, 2012 at 04:05:40AM -0500, Jeff King wrote:

quoted

quoted

 	} 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:

quoted

 		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"));

Interesting.  After I got your mail, I started poking around, and it
turns out we're in a funny situation here.  I did this:

  $ git cat-file commit HEAD
  tree 205f6b799e7d5c2524468ca006a0131aa57ecce7
  parent c4938d8e6d23e3a8fe10e6466ecd827662c14846
  author Thomas Rast [off-list ref] 1330417798 +0100
  committer Thomas Rast [off-list ref] 1330417798 +0100

  $ git cat-file commit HEAD | grep -v '^$' | git hash-object -w -t commit --stdin
  f68fcc1996173a9e04bd45d42abbb7c85c79546d
  $ git reset --hard f68fcc1996173a9e04bd45d42abbb7c85c79546d

So now I'm at a commit which does not have that \n\n.  And poking around
gives a very confusing picture:

  $ git fsck
  Checking object directories: 100% (256/256), done.
  $ git show
  Segmentation fault
  $ git log
  Segmentation fault
  $ git format-patch -1
  0001-.txt
  $ cat 0001-.txt  # no output!
  $ git bundle create test.bdl HEAD
  error: rev-list died of signal 11
  error: rev-list died
  $ git rev-list HEAD
  f68fcc1996173a9e04bd45d42abbb7c85c79546d
  c4938d8e6d23e3a8fe10e6466ecd827662c14846
  $ git rev-list --pretty=oneline HEAD
  Segmentation fault

So either there's a lot to be fixed, or fsck needs to catch this.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch