Patrick Steinhardt <ps@xxxxxx> writes: > + if (it->entry_count + pos > istate->cache_nr) { > + ret = error(_("corrupted cache-tree has entries not present in index")); > + goto out; > + } Is it a safe assumption that the if() condition always indicates an error? When sparse-index is in effect, istate->cache_nr may be a number that is smaller than the true number of paths in the index (because all paths under a subdirectory we are not interested in are folded into a single tree-ish entry), and I am not sure how it should interact with it->entry_count (i.e. the number of paths under the current directory we are looking at, which obviously cannot be a sparsified entry) and pos (i.e. the index into active_cache[] that represend the first path under the current directory)? I guess as long as "it" is not folded, it does not matter how other paths from different directories in active_cache[] are sparsified or expanded, as long as "pos" keeps track of the current position correctly. > diff --git a/t/t4058-diff-duplicates.sh b/t/t4058-diff-duplicates.sh > index 2501c89c1c9..3f602adb055 100755 > --- a/t/t4058-diff-duplicates.sh > +++ b/t/t4058-diff-duplicates.sh > @@ -132,15 +132,15 @@ test_expect_success 'create a few commits' ' > rm commit_id up final > ' > > -test_expect_failure 'git read-tree does not segfault' ' > - test_when_finished rm .git/index.lock && > - test_might_fail git read-tree --reset base > +test_expect_success 'git read-tree does not segfault' ' > + test_must_fail git read-tree --reset base 2>err && > + test_grep "error: corrupted cache-tree has entries not present in index" err > ' Very good. test_might_fail is a sign of trouble, and this gives us a lot more predictability.