{"thread":{"id":"35070","subject":"[PATCH] log_tree_diff: die when we fail to parse a commit","startedAt":"2013-10-06T04:48:57Z","lastAt":"2013-10-06T04:48:57Z","messageCount":1,"participants":["Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"228576","messageId":"20131006044856.GA12301@sigill.intra.peff.net","threadId":"35070","inReplyTo":null,"subject":"[PATCH] log_tree_diff: die when we fail to parse a commit","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-06T04:48:57Z","receivedAt":"2013-10-06T04:48:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We currently call parse_commit and then assume we can\ndereference the resulting \"tree\" struct field. If parsing\nfailed, however, that field is NULL and we end up\nsegfaulting.\n\nInstead of a segfault, let's print an error message and die\na little more gracefully.\n\nNote that this should never happen in practice, but may\nhappen in a corrupt repository (or when accessing objects\nnot connected to the reachable history, whose parents may be\nsubject to pruning).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nNot a huge deal, since we are terminating the program either way. There\nare other places in the code with a bare parse_commit that could\nprobably use the same treatment. I didn't investigate them, but they\ncould easily build on the parse_commit_or_die here if somebody wants to\nfollow up.\n\n commit.c   | 7 +++++++\n commit.h   | 1 +\n log-tree.c | 6 +++---\n 3 files changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex de16a3c..51a9bbc 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -341,6 +341,13 @@ int parse_commit(struct commit *item)\n \treturn ret;\n }\n \n+void parse_commit_or_die(struct commit *item)\n+{\n+\tif (parse_commit(item))\n+\t\tdie(\"unable to parse commit %s\",\n+\t\t    item ? sha1_to_hex(item->object.sha1) : \"(null)\");\n+}\n+\n int find_commit_subject(const char *commit_buffer, const char **subject)\n {\n \tconst char *eol;\ndiff --git a/commit.h b/commit.h\nindex bd841f4..934af88 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -49,6 +49,7 @@ int parse_commit(struct commit *item);\n \n int parse_commit_buffer(struct commit *item, const void *buffer, unsigned long size);\n int parse_commit(struct commit *item);\n+void parse_commit_or_die(struct commit *item);\n \n /* Find beginning and length of commit subject. */\n int find_commit_subject(const char *commit_buffer, const char **subject);\ndiff --git a/log-tree.c b/log-tree.c\nindex 8534d91..e958d07 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -734,7 +734,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log\n \tif (!opt->diff && !DIFF_OPT_TST(&opt->diffopt, EXIT_WITH_STATUS))\n \t\treturn 0;\n \n-\tparse_commit(commit);\n+\tparse_commit_or_die(commit);\n \tsha1 = commit->tree->object.sha1;\n \n \t/* Root commit? */\n@@ -759,7 +759,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log\n \t\t\t * parent, showing summary diff of the others\n \t\t\t * we merged _in_.\n \t\t\t */\n-\t\t\tparse_commit(parents->item);\n+\t\t\tparse_commit_or_die(parents->item);\n \t\t\tdiff_tree_sha1(parents->item->tree->object.sha1,\n \t\t\t\t       sha1, \"\", &opt->diffopt);\n \t\t\tlog_tree_diff_flush(opt);\n@@ -774,7 +774,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log\n \tfor (;;) {\n \t\tstruct commit *parent = parents->item;\n \n-\t\tparse_commit(parent);\n+\t\tparse_commit_or_die(parent);\n \t\tdiff_tree_sha1(parent->tree->object.sha1,\n \t\t\t       sha1, \"\", &opt->diffopt);\n \t\tlog_tree_diff_flush(opt);\n-- \n1.8.4.1.4.gf327177\n"}]}