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

[PATCH 2/5] assume parse_commit checks for NULL commit

From
Jeff King <peff@peff.net>
Date
Oct 8, 2013, 13:56 UTC
Message-ID
<20131008135625.GB7650@sigill.intra.peff.net>
In-Reply-To
<20131008134843.GA7530@sigill.intra.peff.net>

The parse_commit function will check whether it was passed a NULL commit pointer, and if so, return an error. There is no need for callers to check this separately.

Signed-off-by: Jeff King <peff@peff.net>
---
There are a few cases left untouched, because they actually produce a
different result for the NULL case (e.g., printing a different error
message, as in read_tree_1).
Note also the case of:
  if (parent && parse_commit(parent) < 0)

in sequencer.c, which looks at first glance like it falls in this class, but doesn't (it is specifically avoiding the NULL check and reporting an error only when we actually have something to parse at all).

 builtin/branch.c | 2 +-
 builtin/commit.c | 4 ++--
 commit.c         | 2 +-
 notes-utils.c    | 2 +-
 sha1_name.c      | 2 --
 5 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index ad0f86d..491090f 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -496,7 +496,7 @@ static void add_verbose_info(struct strbuf *out, struct ref_item *item,
 	const char *sub = _(" **** invalid ref ****");
 	struct commit *commit = item->commit;
 
-	if (commit && !parse_commit(commit)) {
+	if (!parse_commit(commit)) {
 		pp_commit_easy(CMIT_FMT_ONELINE, commit, &subject);
 		sub = subject.buf;
 	}
diff --git a/builtin/commit.c b/builtin/commit.c
index 6ab4605..e89c519 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1338,7 +1338,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1,
 	commit = lookup_commit(sha1);
 	if (!commit)
 		die(_("couldn't look up newly created commit"));
-	if (!commit || parse_commit(commit))
+	if (parse_commit(commit))
 		die(_("could not parse newly created commit"));
 
 	strbuf_addstr(&format, "format:%h] %s");
@@ -1525,7 +1525,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		current_head = NULL;
 	else {
 		current_head = lookup_commit_or_die(sha1, "HEAD");
-		if (!current_head || parse_commit(current_head))
+		if (parse_commit(current_head))
 			die(_("could not parse HEAD commit"));
 	}
 	argc = parse_and_validate_options(argc, argv, builtin_commit_options,
diff --git a/commit.c b/commit.c
index 51a9bbc..11509ff 100644
--- a/commit.c
+++ b/commit.c
@@ -79,7 +79,7 @@ struct commit *lookup_commit_reference_by_name(const char *name)
 	if (get_sha1_committish(name, sha1))
 		return NULL;
 	commit = lookup_commit_reference(sha1);
-	if (!commit || parse_commit(commit))
+	if (parse_commit(commit))
 		return NULL;
 	return commit;
 }
diff --git a/notes-utils.c b/notes-utils.c
index 9107c37..7bb3473 100644
--- a/notes-utils.c
+++ b/notes-utils.c
@@ -18,7 +18,7 @@ void create_notes_commit(struct notes_tree *t, struct commit_list *parents,
 		unsigned char parent_sha1[20];
 		if (!read_ref(t->ref, parent_sha1)) {
 			struct commit *parent = lookup_commit(parent_sha1);
-			if (!parent || parse_commit(parent))
+			if (parse_commit(parent))
 				die("Failed to find/parse commit %s", t->ref);
 			commit_list_insert(parent, &parents);
 		}
diff --git a/sha1_name.c b/sha1_name.c
index 0e5fe7f..1dfc401 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -582,8 +582,6 @@ static int get_parent(const char *name, int len,
 	if (ret)
 		return ret;
 	commit = lookup_commit_reference(sha1);
-	if (!commit)
-		return -1;
 	if (parse_commit(commit))
 		return -1;
 	if (!idx) {
-- 
1.8.4.1.4.gf327177
Previous: Jeff KingNext: Jeff King
Message 3 of 6 in “parse_commit cleanups”
  1. 0/5 parse_commit cleanupsJeff King, Oct 8, 2013
  2. 1/5 assume parse_commit checks commit->object.parsedJeff King, Oct 8, 2013
  3. 2/5 assume parse_commit checks for NULL commitJeff King, Oct 8, 2013
  4. 3/5 use parse_commit_or_die instead of segfaultingJeff King, Oct 8, 2013
  5. 4/5 use parse_commit_or_die instead of custom messageJeff King, Oct 8, 2013
  6. 5/5 checkout: do not die when leaving broken detached HEADJeff King, Oct 8, 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.