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

[PATCH 06/13] fsck: stop using object_info->type_name strbuf

From
Jeff King <peff@peff.net>
Date
May 16, 2025, 04:49 UTC
Message-ID
<20250516044953.GF22242@coredump.intra.peff.net>
In-Reply-To
<20250516044916.GA21985@coredump.intra.peff.net>

When fsck-ing a loose object, we use object_info's type_name strbuf to record the parsed object type as a string. For most objects this is redundant with the object_type enum, but it does let us report the string when we encounter an object with an unknown type (for which there is no matching enum value).

There are a few downsides, though:
  1. The code to report these cases is not actually robust. Since we did
     not pass a strbuf to unpack_loose_header(), we only retrieved types
     from headers up to 32 bytes. In longer cases, we'd simply say
     "object corrupt or missing".
  2. This is the last caller that uses object_info's type_name strbuf
     support. It would be nice to refactor it so that we can simplify
     that code.
  3. Likewise, we'll check the hash of the object using its unknown type
     (again, as long as that type is short enough). That depends on the
     hash_object_file_literally() code, which we'd eventually like to
     get rid of.

So we can simplify things by bailing immediately in read_loose_object() when we encounter an unknown type. This has a few user-visible effects:

  a. Instead of producing a single line of error output like this:
       error: 26ed13ce3564fbbb44e35bde42c7da717ea004a6: object is of unknown type 'bogus': .git/objects/26/ed13ce3564fbbb44e35bde42c7da717ea004a6
     we'll now issue two lines (the first from read_loose_object() when
     we see the unparsable header, and the second from the fsck code,
     since we couldn't read the object):
       error: unable to parse type from header 'bogus 4' of .git/objects/26/ed13ce3564fbbb44e35bde42c7da717ea004a6
       error: 26ed13ce3564fbbb44e35bde42c7da717ea004a6: object corrupt or missing: .git/objects/26/ed13ce3564fbbb44e35bde42c7da717ea004a6
     This is a little more verbose, but this sort of error should be
     rare (such objects are almost impossible to work with, and cannot
     be transferred between repositories as they are not representable
     in packfiles). And as a bonus, reporting the broken header in full
     could help with debugging other cases (e.g., a header like "blob
     xyzzy\0" would fail in parsing the size, but previously we'd not
     have showed the offending bytes).
  b. An object with an unknown type will be reported as corrupt, without
     actually doing a hash check. Again, I think this is unlikely to
     matter in practice since such objects are totally unusable.

We'll update one fsck test to match the new error strings. And we can remove another test that covered the case of an object with an unknown type _and_ a hash corruption. Since we'll skip the hash check now in this case, the test is no longer interesting.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/fsck.c  | 13 ++-----------
 object-file.c   | 12 +++++++++---
 t/t1450-fsck.sh | 29 +++--------------------------
 3 files changed, 14 insertions(+), 40 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 6cac28356c..e7d96a9c8e 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -614,12 +614,11 @@ static void get_default_heads(void)
 struct for_each_loose_cb
 {
 	struct progress *progress;
-	struct strbuf obj_type;
 };
 
-static int fsck_loose(const struct object_id *oid, const char *path, void *data)
+static int fsck_loose(const struct object_id *oid, const char *path,
+		      void *data UNUSED)
 {
-	struct for_each_loose_cb *cb_data = data;
 	struct object *obj;
 	enum object_type type = OBJ_NONE;
 	unsigned long size;
@@ -629,8 +628,6 @@ static int fsck_loose(const struct object_id *oid, const char *path, void *data)
 	struct object_id real_oid = *null_oid(the_hash_algo);
 	int err = 0;
 
-	strbuf_reset(&cb_data->obj_type);
-	oi.type_name = &cb_data->obj_type;
 	oi.sizep = &size;
 	oi.typep = &type;
 
@@ -642,10 +639,6 @@ static int fsck_loose(const struct object_id *oid, const char *path, void *data)
 			err = error(_("%s: object corrupt or missing: %s"),
 				    oid_to_hex(oid), path);
 	}
-	if (type != OBJ_NONE && type < 0)
-		err = error(_("%s: object is of unknown type '%s': %s"),
-			    oid_to_hex(&real_oid), cb_data->obj_type.buf,
-			    path);
 	if (err < 0) {
 		errors_found |= ERROR_OBJECT;
 		free(contents);
@@ -697,7 +690,6 @@ static void fsck_object_dir(const char *path)
 {
 	struct progress *progress = NULL;
 	struct for_each_loose_cb cb_data = {
-		.obj_type = STRBUF_INIT,
 		.progress = progress,
 	};
 
@@ -712,7 +704,6 @@ static void fsck_object_dir(const char *path)
 				      &cb_data);
 	display_progress(progress, 256);
 	stop_progress(&progress);
-	strbuf_release(&cb_data.obj_type);
 }
 
 static int fsck_head_link(const char *head_ref_name,
diff --git a/object-file.c b/object-file.c
index 1127e154f6..7a35bde96e 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1662,6 +1662,12 @@ int read_loose_object(const char *path,
 		goto out_inflate;
 	}
 
+	if (*oi->typep < 0) {
+		error(_("unable to parse type from header '%s' of %s"),
+		      hdr, path);
+		goto out_inflate;
+	}
+
 	if (*oi->typep == OBJ_BLOB &&
 	    *size > repo_settings_get_big_file_threshold(the_repository)) {
 		if (check_stream_oid(&stream, hdr, *size, path, expected_oid) < 0)
@@ -1672,9 +1678,9 @@ int read_loose_object(const char *path,
 			error(_("unable to unpack contents of %s"), path);
 			goto out_inflate;
 		}
-		hash_object_file_literally(the_repository->hash_algo,
-					   *contents, *size,
-					   oi->type_name->buf, real_oid);
+		hash_object_file(the_repository->hash_algo,
+				 *contents, *size,
+				 *oi->typep, real_oid);
 		if (!oideq(expected_oid, real_oid))
 			goto out_inflate;
 	}
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 0105045376..3f52dd5abc 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -71,30 +71,6 @@ test_expect_success 'object with hash mismatch' '
 	)
 '
 
-test_expect_success 'object with hash and type mismatch' '
-	git init --bare hash-type-mismatch &&
-	(
-		cd hash-type-mismatch &&
-
-		oid=$(echo blob | git hash-object -w --stdin -t garbage --literally) &&
-		oldoid=$oid &&
-		old=$(test_oid_to_path "$oid") &&
-		new=$(dirname $old)/$(test_oid ff_2) &&
-		oid="$(dirname $new)$(basename $new)" &&
-
-		mv objects/$old objects/$new &&
-		git update-index --add --cacheinfo 100644 $oid foo &&
-		tree=$(git write-tree) &&
-		cmt=$(echo bogus | git commit-tree $tree) &&
-		git update-ref refs/heads/bogus $cmt &&
-
-
-		test_must_fail git fsck 2>out &&
-		grep "^error: $oldoid: hash-path mismatch, found at: .*$new" out &&
-		grep "^error: $oldoid: object is of unknown type '"'"'garbage'"'"'" out
-	)
-'
-
 test_expect_success 'zlib corrupt loose object output ' '
 	git init --bare corrupt-loose-output &&
 	(
@@ -1001,8 +977,9 @@ test_expect_success 'fsck error and recovery on invalid object type' '
 
 		test_must_fail git fsck 2>err &&
 		grep -e "^error" -e "^fatal" err >errors &&
-		test_line_count = 1 errors &&
-		grep "$garbage_blob: object is of unknown type '"'"'garbage'"'"':" err
+		test_line_count = 2 errors &&
+		test_grep "unable to parse type from header .garbage" err &&
+		test_grep "$garbage_blob: object corrupt or missing:" err
 	)
 '
 

-- 
2.49.0.896.g93578ceaaf
Previous: Jeff KingNext: Patrick Steinhardt
Message 12 of 35 in “dropping support for non-standard object types”
  1. 0/13 dropping support for non-standard object typesJeff King, May 16, 2025
  2. 01/13 object-file.h: fix typo in variable declarationJeff King, May 16, 2025
  3. 02/13 cat-file: make --allow-unknown-type a noopJeff King, May 16, 2025
  4. Patrick SteinhardtMay 16, 2025
  5. Jeff KingMay 19, 2025
  6. Patrick SteinhardtMay 19, 2025
  7. Junio C HamanoMay 16, 2025
  8. 03/13 object-file: drop OBJECT_INFO_ALLOW_UNKNOWN_TYPE flagJeff King, May 16, 2025
  9. 04/13 cat-file: use type enum instead of buffer for -t optionJeff King, May 16, 2025
  10. Junio C HamanoMay 16, 2025
  11. 05/13 oid_object_info_convert(): stop using string for object typeJeff King, May 16, 2025
  12. 06/13 fsck: stop using object_info->type_name strbufJeff King, May 16, 2025
  13. Patrick SteinhardtMay 16, 2025
  14. Junio C HamanoMay 19, 2025
  15. Jeff KingMay 19, 2025
  16. 07/13 oid_object_info(): drop type_name strbufJeff King, May 16, 2025
  17. Junio C HamanoMay 19, 2025
  18. 08/13 t/helper: add zlib test-toolJeff King, May 16, 2025
  19. Junio C HamanoMay 19, 2025
  20. Jeff KingMay 19, 2025
  21. Junio C HamanoMay 21, 2025
  22. 09/13 t: add lib-loose.shJeff King, May 16, 2025
  23. Patrick SteinhardtMay 16, 2025
  24. Jeff KingMay 19, 2025
  25. Junio C HamanoMay 19, 2025
  26. 10/13 hash-object: stop allowing unknown typesJeff King, May 16, 2025
  27. Junio C HamanoMay 19, 2025
  28. 11/13 hash-object: merge HASH_* and INDEX_* flagsJeff King, May 16, 2025
  29. Patrick SteinhardtMay 16, 2025
  30. 12/13 hash-object: handle --literally with OPT_NEGBITJeff King, May 16, 2025
  31. Junio C HamanoMay 19, 2025
  32. 13/13 object-file: drop support for writing objects with unknown typesJeff King, May 16, 2025
  33. Patrick SteinhardtMay 16, 2025
  34. Junio C HamanoMay 19, 2025
  35. Junio C HamanoMay 16, 2025

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.