fsck_tree(): fix shadowed variable
Commit b2f2039c2b (fsck: accept an oid instead of a "struct tree" for fsck_tree(), 2019-10-18) introduced a new "oid" parameter to fsck_tree(), and we pass it to the report() function when we find problems. However, that is shadowed within the tree-walking loop by the existing "oid" variable which we use to store the oid of each tree entry. As a result, we may report the wrong oid for some problems we detect within the loop (the entry oid, instead of the tree oid). Our tests didn't catch this because they checked only that we found the expected fsck problem, not that it was attached to the correct object. Let's rename both variables in the function to avoid confusion. This makes the diff a little noisy (e.g., all of the report() calls outside the loop were already correct but need to be touched), but makes sure we catch all cases and will avoid similar confusion in the future. And we can update the test to be a bit more specific and catch this problem. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King committed
May 1, 2021 at 11:41 UTC
9e1947cb482c8fc7e1d0c8334f126ced5062b895
2 files changed
+25
-22
fsck.c
+22
-20
@@ -558,7 +558,7 @@ static int verify_ordered(unsigned mode1, const char *name1,
558
return c1 < c2 ? 0 : TREE_UNORDERED;
559
}
560
561
-static int fsck_tree(const struct object_id *oid,
561
+static int fsck_tree(const struct object_id *tree_oid,
562
const char *buffer, unsigned long size,
563
struct fsck_options *options)
564
{
@@ -579,7 +579,7 @@ static int fsck_tree(const struct object_id *oid,
579
struct name_stack df_dup_candidates = { NULL };
580
581
if (init_tree_desc_gently(&desc, buffer, size)) {
582
- retval += report(options, oid, OBJ_TREE, FSCK_MSG_BAD_TREE, "cannot be parsed as a tree");
582
+ retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_BAD_TREE, "cannot be parsed as a tree");
583
return retval;
584
}
585
@@ -589,11 +589,11 @@ static int fsck_tree(const struct object_id *oid,
589
while (desc.size) {
590
unsigned short mode;
591
const char *name, *backslash;
592
- const struct object_id *oid;
592
+ const struct object_id *entry_oid;
593
594
- oid = tree_entry_extract(&desc, &name, &mode);
594
+ entry_oid = tree_entry_extract(&desc, &name, &mode);
595
596
- has_null_sha1 |= is_null_oid(oid);
596
+ has_null_sha1 |= is_null_oid(entry_oid);
597
has_full_path |= !!strchr(name, '/');
598
has_empty_name |= !*name;
599
has_dot |= !strcmp(name, ".");
@@ -603,10 +603,11 @@ static int fsck_tree(const struct object_id *oid,
603
604
if (is_hfs_dotgitmodules(name) || is_ntfs_dotgitmodules(name)) {
605
if (!S_ISLNK(mode))
606
- oidset_insert(&options->gitmodules_found, oid);
606
+ oidset_insert(&options->gitmodules_found,
607
+ entry_oid);
608
else
609
retval += report(options,
609
- oid, OBJ_TREE,
610
+ tree_oid, OBJ_TREE,
611
FSCK_MSG_GITMODULES_SYMLINK,
612
".gitmodules is a symbolic link");
613
}
@@ -617,9 +618,10 @@ static int fsck_tree(const struct object_id *oid,
618
has_dotgit |= is_ntfs_dotgit(backslash);
619
if (is_ntfs_dotgitmodules(backslash)) {
620
if (!S_ISLNK(mode))
620
- oidset_insert(&options->gitmodules_found, oid);
621
+ oidset_insert(&options->gitmodules_found,
622
+ entry_oid);
623
else
622
- retval += report(options, oid, OBJ_TREE,
624
+ retval += report(options, tree_oid, OBJ_TREE,
625
FSCK_MSG_GITMODULES_SYMLINK,
626
".gitmodules is a symbolic link");
627
}
@@ -628,7 +630,7 @@ static int fsck_tree(const struct object_id *oid,
630
}
631
632
if (update_tree_entry_gently(&desc)) {
631
- retval += report(options, oid, OBJ_TREE, FSCK_MSG_BAD_TREE, "cannot be parsed as a tree");
633
+ retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_BAD_TREE, "cannot be parsed as a tree");
634
break;
635
}
636
@@ -676,25 +678,25 @@ static int fsck_tree(const struct object_id *oid,
678
name_stack_clear(&df_dup_candidates);
679
680
if (has_null_sha1)
679
- retval += report(options, oid, OBJ_TREE, FSCK_MSG_NULL_SHA1, "contains entries pointing to null sha1");
681
+ retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_NULL_SHA1, "contains entries pointing to null sha1");
682
if (has_full_path)
681
- retval += report(options, oid, OBJ_TREE, FSCK_MSG_FULL_PATHNAME, "contains full pathnames");
683
+ retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_FULL_PATHNAME, "contains full pathnames");
684
if (has_empty_name)
683
- retval += report(options, oid, OBJ_TREE, FSCK_MSG_EMPTY_NAME, "contains empty pathname");
685
+ retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_EMPTY_NAME, "contains empty pathname");
686
if (has_dot)
685
- retval += report(options, oid, OBJ_TREE, FSCK_MSG_HAS_DOT, "contains '.'");
687
+ retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_HAS_DOT, "contains '.'");
688
if (has_dotdot)
687
- retval += report(options, oid, OBJ_TREE, FSCK_MSG_HAS_DOTDOT, "contains '..'");
689
+ retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_HAS_DOTDOT, "contains '..'");
690
if (has_dotgit)
689
- retval += report(options, oid, OBJ_TREE, FSCK_MSG_HAS_DOTGIT, "contains '.git'");
691
+ retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_HAS_DOTGIT, "contains '.git'");
692
if (has_zero_pad)
691
- retval += report(options, oid, OBJ_TREE, FSCK_MSG_ZERO_PADDED_FILEMODE, "contains zero-padded file modes");
693
+ retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_ZERO_PADDED_FILEMODE, "contains zero-padded file modes");
694
if (has_bad_modes)
693
- retval += report(options, oid, OBJ_TREE, FSCK_MSG_BAD_FILEMODE, "contains bad file modes");
695
+ retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_BAD_FILEMODE, "contains bad file modes");
696
if (has_dup_entries)
695
- retval += report(options, oid, OBJ_TREE, FSCK_MSG_DUPLICATE_ENTRIES, "contains duplicate file entries");
697
+ retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_DUPLICATE_ENTRIES, "contains duplicate file entries");
698
if (not_properly_sorted)
697
- retval += report(options, oid, OBJ_TREE, FSCK_MSG_TREE_NOT_SORTED, "not properly sorted");
699
+ retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_TREE_NOT_SORTED, "not properly sorted");
700
return retval;
701
}
702
t/t7415-submodule-names.sh
+3
-2
@@ -148,12 +148,13 @@ test_expect_success 'fsck detects symlinked .gitmodules file' '
148
{
149
printf "100644 blob $content\t$tricky\n" &&
150
printf "120000 blob $target\t.gitmodules\n"
151
- } | git mktree &&
151
+ } >bad-tree &&
152
+ tree=$(git mktree <bad-tree) &&
153
154
# Check not only that we fail, but that it is due to the
155
# symlink detector
156
test_must_fail git fsck 2>output &&
156
- grep gitmodulesSymlink output
157
+ grep "tree $tree: gitmodulesSymlink" output
158
)
159
'
160