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.
