Re: git cherry(pick) dumps core

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Apr 29, 2010, 19:49 UTC
Message-ID: <20100429194936.GA31941@progeny.tock>
In-Reply-To: <20100429191150.GA10526@inner.home.ulmdo.de>

Hi Andreas,

Andreas Krey wrote:

Show 18 quoted lines

> I just have a case of git cherry-pick dieing with a core dump.
> The directly offending lines are get_message() in buildin/revert.c:
> 
> 	if ((out->reencoded_message = reencode_string(raw_message,
> 					git_commit_encoding, encoding)))
> 		out->message = out->reencoded_message;
> 
> 	abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
> 	abbrev_len = strlen(abbrev);
> 
> 	/* Find beginning and end of commit subject. */
> 	p = out->message;
> 	while (*p && (*p != '\n' || p[1] != '\n'))
> 
> and out->message is null at that point.
> 
> It looks like reencode_string is returning NULL,
> and get_message can't quite cope with that.

Thanks for the report. What encoding were you using? (You can check with ‘git cat-file commit <revision you were trying to cherry-pick>’.)

The code you mention was just wrong, sorry. If the local iconv doesn’t understand some recording, the correct behavior is to pass the message through, not to segfault.

With this change, cherry-pick would treat the old message as UTF-8 (or whatever the current [i18n] commitencoding setting specifies, if present). If the message happens to contain an illegal byte sequence, the cherry-pick will not segfault but it will still fail.

In other words, this patch should fix the segfault, but it does not fix the underlying problem which was there before.

diff --git a/builtin/revert.c b/builtin/revert.c
index bbaa937..9f8ceab 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -114,6 +114,8 @@ static int get_message(const char *raw_message, struct commit_message *out)
 	if ((out->reencoded_message = reencode_string(raw_message,
 					git_commit_encoding, encoding)))
 		out->message = out->reencoded_message;
+	else
+		out->message = raw_message;
 
 	abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
 	abbrev_len = strlen(abbrev);