@matheusbsbs / matheus-bagliano / commits / 4ae0e9423c

fsck: stop using object_info->type_name strbuf

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed May 16, 2025 at 00:49 UTC 4ae0e9423c95c63c17f66fb2de255c46dc14c4e5

3 files changed +14 -40

builtin/fsck.c

+2 -11

@@ -614,12 +614,11 @@ static void get_default_heads(void)

614 struct for_each_loose_cb

615 {

616 struct progress *progress;

617 - struct strbuf obj_type;

617 };

618

620 -static int fsck_loose(const struct object_id *oid, const char *path, void *data)

619 +static int fsck_loose(const struct object_id *oid, const char *path,

620 + void *data UNUSED)

621 {

622 - struct for_each_loose_cb *cb_data = data;

622 struct object *obj;

623 enum object_type type = OBJ_NONE;

624 unsigned long size;

@@ -629,8 +628,6 @@ static int fsck_loose(const struct object_id *oid, const char *path, void *data)

628 struct object_id real_oid = *null_oid(the_hash_algo);

629 int err = 0;

630

632 - strbuf_reset(&cb_data->obj_type);

633 - oi.type_name = &cb_data->obj_type;

631 oi.sizep = &size;

632 oi.typep = &type;

633

@@ -642,10 +639,6 @@ static int fsck_loose(const struct object_id *oid, const char *path, void *data)

639 err = error(_("%s: object corrupt or missing: %s"),

640 oid_to_hex(oid), path);

641 }

645 - if (type != OBJ_NONE && type < 0)

646 - err = error(_("%s: object is of unknown type '%s': %s"),

647 - oid_to_hex(&real_oid), cb_data->obj_type.buf,

648 - path);

642 if (err < 0) {

643 errors_found |= ERROR_OBJECT;

644 free(contents);

@@ -697,7 +690,6 @@ static void fsck_object_dir(const char *path)

690 {

691 struct progress *progress = NULL;

692 struct for_each_loose_cb cb_data = {

700 - .obj_type = STRBUF_INIT,

693 .progress = progress,

694 };

695

@@ -712,7 +704,6 @@ static void fsck_object_dir(const char *path)

704 &cb_data);

705 display_progress(progress, 256);

706 stop_progress(&progress);

715 - strbuf_release(&cb_data.obj_type);

707 }

708

709 static int fsck_head_link(const char *head_ref_name,

object-file.c

+9 -3

@@ -1662,6 +1662,12 @@ int read_loose_object(const char *path,

1662 goto out_inflate;

1663 }

1664

1665 + if (*oi->typep < 0) {

1666 + error(_("unable to parse type from header '%s' of %s"),

1667 + hdr, path);

1668 + goto out_inflate;

1669 + }

1670 +

1671 if (*oi->typep == OBJ_BLOB &&

1672 *size > repo_settings_get_big_file_threshold(the_repository)) {

1673 if (check_stream_oid(&stream, hdr, *size, path, expected_oid) < 0)

@@ -1672,9 +1678,9 @@ int read_loose_object(const char *path,

1678 error(_("unable to unpack contents of %s"), path);

1679 goto out_inflate;

1680 }

1675 - hash_object_file_literally(the_repository->hash_algo,

1676 - *contents, *size,

1677 - oi->type_name->buf, real_oid);

1681 + hash_object_file(the_repository->hash_algo,

1682 + *contents, *size,

1683 + *oi->typep, real_oid);

1684 if (!oideq(expected_oid, real_oid))

1685 goto out_inflate;

1686 }

t/t1450-fsck.sh

+3 -26

@@ -71,30 +71,6 @@ test_expect_success 'object with hash mismatch' '

71 )

72 '

74 -test_expect_success 'object with hash and type mismatch' '

75 - git init --bare hash-type-mismatch &&

76 - (

77 - cd hash-type-mismatch &&

78 -

79 - oid=$(echo blob | git hash-object -w --stdin -t garbage --literally) &&

80 - oldoid=$oid &&

81 - old=$(test_oid_to_path "$oid") &&

82 - new=$(dirname $old)/$(test_oid ff_2) &&

83 - oid="$(dirname $new)$(basename $new)" &&

84 -

85 - mv objects/$old objects/$new &&

86 - git update-index --add --cacheinfo 100644 $oid foo &&

87 - tree=$(git write-tree) &&

88 - cmt=$(echo bogus | git commit-tree $tree) &&

89 - git update-ref refs/heads/bogus $cmt &&

90 -

91 -

92 - test_must_fail git fsck 2>out &&

93 - grep "^error: $oldoid: hash-path mismatch, found at: .*$new" out &&

94 - grep "^error: $oldoid: object is of unknown type '"'"'garbage'"'"'" out

95 - )

96 -'

97 -

74 test_expect_success 'zlib corrupt loose object output ' '

75 git init --bare corrupt-loose-output &&

76 (

@@ -1001,8 +977,9 @@ test_expect_success 'fsck error and recovery on invalid object type' '

977

978 test_must_fail git fsck 2>err &&

979 grep -e "^error" -e "^fatal" err >errors &&

1004 - test_line_count = 1 errors &&

1005 - grep "$garbage_blob: object is of unknown type '"'"'garbage'"'"':" err

980 + test_line_count = 2 errors &&

981 + test_grep "unable to parse type from header .garbage" err &&

982 + test_grep "$garbage_blob: object corrupt or missing:" err

983 )

984 '

985