Re: git cherry(pick) dumps core

[Jonathan Nieder <jrnieder@gmail.com>]

Hi Andreas,

Andreas Krey wrote:

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