Skip to content

Commit

Permalink
unpack-trees: fix path search bug in verify_absent
Browse files Browse the repository at this point in the history
Commit 0cf7375 (unpack-trees.c: assume submodules are clean during
check-out) changed an argument to verify_absent from 'path' to 'ce',
which is however shadowed by a local variable of the same name.

The bug triggers if verify_absent is used on a tree entry, for which
the index contains one or more subsequent directories of the same
length. The affected subdirectories are removed from the index. The
testcase included in this commit bisects to 5521883 (checkout: do not
lose staged removal), which reveals the bug in this case, but is
otherwise unrelated.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Clemens Buchacher authored and Junio C Hamano committed Jan 5, 2009
1 parent 6b9315d commit 837e5fe
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 10 deletions.
27 changes: 27 additions & 0 deletions t/t1001-read-tree-m-2way.sh
Original file line number Diff line number Diff line change
Expand Up @@ -365,4 +365,31 @@ test_expect_success \
git ls-files --stage &&
test -f a/b'

test_expect_success \
'a/b vs a, plus c/d case setup.' \
'rm -f .git/index &&
rm -fr a &&
: >a &&
mkdir c &&
: >c/d &&
git update-index --add a c/d &&
treeM=`git write-tree` &&
echo treeM $treeM &&
git ls-tree $treeM &&
git ls-files --stage >treeM.out &&
rm -f a &&
mkdir a
: >a/b &&
git update-index --add --remove a a/b &&
treeH=`git write-tree` &&
echo treeH $treeH &&
git ls-tree $treeH'

test_expect_success \
'a/b vs a, plus c/d case test.' \
'git read-tree -u -m "$treeH" "$treeM" &&
git ls-files --stage | tee >treeMcheck.out &&
test_cmp treeM.out treeMcheck.out'

test_done
20 changes: 10 additions & 10 deletions unpack-trees.c
Original file line number Diff line number Diff line change
Expand Up @@ -516,22 +516,22 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action,
namelen = strlen(ce->name);
pos = index_name_pos(o->src_index, ce->name, namelen);
if (0 <= pos)
return cnt; /* we have it as nondirectory */
return 0; /* we have it as nondirectory */
pos = -pos - 1;
for (i = pos; i < o->src_index->cache_nr; i++) {
struct cache_entry *ce = o->src_index->cache[i];
int len = ce_namelen(ce);
struct cache_entry *ce2 = o->src_index->cache[i];
int len = ce_namelen(ce2);
if (len < namelen ||
strncmp(ce->name, ce->name, namelen) ||
ce->name[namelen] != '/')
strncmp(ce->name, ce2->name, namelen) ||
ce2->name[namelen] != '/')
break;
/*
* ce->name is an entry in the subdirectory.
* ce2->name is an entry in the subdirectory.
*/
if (!ce_stage(ce)) {
if (verify_uptodate(ce, o))
if (!ce_stage(ce2)) {
if (verify_uptodate(ce2, o))
return -1;
add_entry(o, ce, CE_REMOVE, 0);
add_entry(o, ce2, CE_REMOVE, 0);
}
cnt++;
}
Expand Down Expand Up @@ -623,7 +623,7 @@ static int verify_absent(struct cache_entry *ce, const char *action,
* If this removed entries from the index,
* what that means is:
*
* (1) the caller unpack_trees_rec() saw path/foo
* (1) the caller unpack_callback() saw path/foo
* in the index, and it has not removed it because
* it thinks it is handling 'path' as blob with
* D/F conflict;
Expand Down

0 comments on commit 837e5fe

Please sign in to comment.