git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 1/6] log_tree_diff: die when we fail to parse a commit

From
Jeff King <peff@peff.net>
Date
Oct 24, 2013, 08:52 UTC
Message-ID
<20131024085236.GA1346@sigill.intra.peff.net>
In-Reply-To
<20131024085213.GA1267@sigill.intra.peff.net>

We currently call parse_commit and then assume we can dereference the resulting "tree" struct field. If parsing failed, however, that field is NULL and we end up segfaulting.

Instead of a segfault, let's print an error message and die a little more gracefully.

Note that this should never happen in practice, but may happen in a corrupt repository.

Signed-off-by: Jeff King <peff@peff.net>
---
 commit.c   | 7 +++++++
 commit.h   | 1 +
 log-tree.c | 6 +++---
 3 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/commit.c b/commit.c
index de16a3c..51a9bbc 100644
--- a/commit.c
+++ b/commit.c
@@ -341,6 +341,13 @@ int parse_commit(struct commit *item)
 	return ret;
 }
 
+void parse_commit_or_die(struct commit *item)
+{
+	if (parse_commit(item))
+		die("unable to parse commit %s",
+		    item ? sha1_to_hex(item->object.sha1) : "(null)");
+}
+
 int find_commit_subject(const char *commit_buffer, const char **subject)
 {
 	const char *eol;
diff --git a/commit.h b/commit.h
index bd841f4..934af88 100644
--- a/commit.h
+++ b/commit.h
@@ -49,6 +49,7 @@ struct commit *lookup_commit_or_die(const unsigned char *sha1, const char *ref_n
 
 int parse_commit_buffer(struct commit *item, const void *buffer, unsigned long size);
 int parse_commit(struct commit *item);
+void parse_commit_or_die(struct commit *item);
 
 /* Find beginning and length of commit subject. */
 int find_commit_subject(const char *commit_buffer, const char **subject);
diff --git a/log-tree.c b/log-tree.c
index 8534d91..e958d07 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -734,7 +734,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
 	if (!opt->diff && !DIFF_OPT_TST(&opt->diffopt, EXIT_WITH_STATUS))
 		return 0;
 
-	parse_commit(commit);
+	parse_commit_or_die(commit);
 	sha1 = commit->tree->object.sha1;
 
 	/* Root commit? */
@@ -759,7 +759,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
 			 * parent, showing summary diff of the others
 			 * we merged _in_.
 			 */
-			parse_commit(parents->item);
+			parse_commit_or_die(parents->item);
 			diff_tree_sha1(parents->item->tree->object.sha1,
 				       sha1, "", &opt->diffopt);
 			log_tree_diff_flush(opt);
@@ -774,7 +774,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
 	for (;;) {
 		struct commit *parent = parents->item;
 
-		parse_commit(parent);
+		parse_commit_or_die(parent);
 		diff_tree_sha1(parent->tree->object.sha1,
 			       sha1, "", &opt->diffopt);
 		log_tree_diff_flush(opt);
-- 
1.8.4.1.898.g8bf8a41.dirty
Previous: Jeff KingNext: Jeff King
Message 2 of 7 in “fix some parse_commit segfaults”
  1. 0/6 fix some parse_commit segfaultsJeff King, Oct 24, 2013
  2. 1/6 log_tree_diff: die when we fail to parse a commitJeff King, Oct 24, 2013
  3. 2/6 assume parse_commit checks commit->object.parsedJeff King, Oct 24, 2013
  4. 3/6 assume parse_commit checks for NULL commitJeff King, Oct 24, 2013
  5. 4/6 use parse_commit_or_die instead of segfaultingJeff King, Oct 24, 2013
  6. 5/6 use parse_commit_or_die instead of custom messageJeff King, Oct 24, 2013
  7. 6/6 checkout: do not die when leaving broken detached HEADJeff King, Oct 24, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.