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

[PATCH 05/23] fsck: stop checking commit->tree value

From
Jeff King <peff@peff.net>
Date
Oct 18, 2019, 04:48 UTC
Message-ID
<20191018044820.GE17879@sigill.intra.peff.net>
In-Reply-To
<20191018044103.GA17625@sigill.intra.peff.net>

We check in fsck_commit_buffer() that commit->tree isn't NULL, which in turn generally comes from a previous parse by parse_commit(). But this isn't really accomplishing anything. The two things we might care about are:

  - was there a syntactically valid "tree <oid>" line in the object? But
    we've just done our own parse in fsck_commit_buffer() to check this.
  - does it point to a valid tree object? But checking the "tree"
    pointer here doesn't actually accomplish that; it just shows that
    lookup_tree() didn't return NULL, which only means that we haven't
    yet seen that oid as a non-tree in this process.
    A real connectivity check would exhaustively walk all graph links,
    and we do that already in a separate function.

So this code isn't helping anything. And it makes the fsck code slightly more confusing and rigid (e.g., it requires that any commit structs have already been parsed). Let's drop it.

As a bit of history, the presence of this code looks like a leftover from early fsck code (which did rely on parse_commit() to do most of the parsing). The check comes from ff5ebe39b0 (Port fsck-cache to use parsing functions, 2005-04-18), but we later added an explicit walk in 355885d531 (add generic, type aware object chain walker, 2008-02-25).

Signed-off-by: Jeff King <peff@peff.net>
---
 fsck.c | 5 -----
 1 file changed, 5 deletions(-)
diff --git a/fsck.c b/fsck.c
index cdb7d8db03..6dfc533fb0 100644
--- a/fsck.c
+++ b/fsck.c
@@ -800,11 +800,6 @@ static int fsck_commit_buffer(struct commit *commit, const char *buffer,
 	err = fsck_ident(&buffer, &commit->object, options);
 	if (err)
 		return err;
-	if (!get_commit_tree(commit)) {
-		err = report(options, &commit->object, FSCK_MSG_BAD_TREE, "could not load commit's tree %s", oid_to_hex(&tree_oid));
-		if (err)
-			return err;
-	}
 	if (memchr(buffer_begin, '\0', size)) {
 		err = report(options, &commit->object, FSCK_MSG_NUL_IN_COMMIT,
 			     "NUL byte in the commit object body");
-- 
2.23.0.1228.gee29b05929
Previous: Jeff KingNext: Junio C Hamano
Message 7 of 39 in “parsing and fsck cleanups”
  1. 0/23 parsing and fsck cleanupsJeff King, Oct 18, 2019
  2. 08/23 fsck: require an actual buffer for non-blobsJeff King, Oct 18, 2019
  3. 04/23 remember commit/tag parse failuresJeff King, Oct 18, 2019
  4. Junio C HamanoOct 24, 2019
  5. Jonathan TanOct 24, 2019
  6. Jeff KingOct 24, 2019
  7. 05/23 fsck: stop checking commit->tree valueJeff King, Oct 18, 2019
  8. Junio C HamanoOct 24, 2019
  9. 06/23 fsck: stop checking commit->parent countsJeff King, Oct 18, 2019
  10. 09/23 fsck: unify object-name codeJeff King, Oct 18, 2019
  11. Junio C HamanoOct 24, 2019
  12. Jeff KingOct 24, 2019
  13. Junio C HamanoOct 25, 2019
  14. Jeff KingOct 25, 2019
  15. 10/23 fsck_describe_object(): build on our get_object_name() primitiveJeff King, Oct 18, 2019
  16. Junio C HamanoOct 24, 2019
  17. 11/23 fsck: use oids rather than objects for object_name APIJeff King, Oct 18, 2019
  18. 07/23 fsck: stop checking tag->taggedJeff King, Oct 18, 2019
  19. 12/23 fsck: don't require object structs for display functionsJeff King, Oct 18, 2019
  20. 13/23 fsck: only provide oid/type in fsck_error callbackJeff King, Oct 18, 2019
  21. 14/23 fsck: only require an oid for skiplist functionsJeff King, Oct 18, 2019
  22. 15/23 fsck: don't require an object struct for report()Jeff King, Oct 18, 2019
  23. 16/23 fsck: accept an oid instead of a "struct blob" for fsck_blob()Jeff King, Oct 18, 2019
  24. 17/23 fsck: drop blob struct from fsck_finish()Jeff King, Oct 18, 2019
  25. 18/23 fsck: don't require an object struct for fsck_ident()Jeff King, Oct 18, 2019
  26. 19/23 fsck: don't require an object struct in verify_headers()Jeff King, Oct 18, 2019
  27. 20/23 fsck: rename vague "oid" local variablesJeff King, Oct 18, 2019
  28. 21/23 fsck: accept an oid instead of a "struct tag" for fsck_tag()Jeff King, Oct 18, 2019
  29. 22/23 fsck: accept an oid instead of a "struct commit" for fsck_commit()Jeff King, Oct 18, 2019
  30. 23/23 fsck: accept an oid instead of a "struct tree" for fsck_tree()Jeff King, Oct 18, 2019
  31. 01/23 parse_commit_buffer(): treat lookup_commit() failure as parse errorJeff King, Oct 18, 2019
  32. Junio C HamanoOct 24, 2019
  33. Jeff KingOct 24, 2019
  34. 02/23 parse_commit_buffer(): treat lookup_tree() failure as parse errorJeff King, Oct 18, 2019
  35. Jonathan TanOct 24, 2019
  36. Jeff KingOct 24, 2019
  37. 03/23 parse_tag_buffer(): treat NULL tag pointer as parse errorJeff King, Oct 18, 2019
  38. Jonathan TanOct 24, 2019
  39. Junio C HamanoOct 25, 2019

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.