From: Derrick Stolee <stolee@gmail.com>
To: Junio C Hamano <gitster@pobox.com>,
Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] sparse-index: avoid crash on intent-to-add entry outside the cone
Date: Fri, 31 Jul 2026 09:23:41 -0400 [thread overview]
Message-ID: <e4cce4e2-4287-4e1a-8833-d37ee48ff7d6@gmail.com> (raw)
In-Reply-To: <xmqq33x1o465.fsf@gitster.g>
On 7/29/2026 5:51 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> From: Derrick Stolee <stolee@gmail.com>
>>
>> When collapsing a full index to a sparse index, the recursive
>> convert_to_sparse_rec() walks the cache tree to determine if any
>> of the cache tree entries can be used to represent a sparse directory.
>>
>> As it goes, the method tracks how many cache entries are being represented
>> by the cache tree entry. The cache tree node's 'entry_count' represents how
>> many cache entries are covered by the node.
>>
>> However, this value can be negative, representing that a node is invalid,
>> and is no longer reflecting the number of cache entries fit within. This can
>> happen when the user uses 'git add --intent-to-add' to mark an untracked
>> file with the intent-to-add bit to avoid committing without finishing the
>> add.
>
> Yes. If the code were not anticipating this, I can understand how a
> bug can arise ;-)
>
>> Theoretically, we could scan to find the range of entries that match this
>> directory and determine if they truly do have an intent-to-add bit and then
>> collapse as many child trees as possible (the ones with valid cache tree
>> nodes). That would be a non-trivial change for performance-only benefit.
>> Since this combination of the intent-to-add and sparse index features has so
>> far gone undetected by real users, this scenario is unlikely to be worth
>> such a change.
>
> I tend to agree. That does sound nasty.
>> + span = ct->down[pos]->cache_tree->entry_count;
>> + if (span < 0) {
>> + /* cache-tree entry is invalidated, cannot collapse. */
>> + istate->cache[num_converted++] = ce;
>> + i++;
>> + continue;
>> + }
>
> OK. That is an easy and safe cop-out that is much better than segfaulting.
>
> Shall we mark the topic for 'next'?
Thanks for taking a look. yes, this should be a pretty safe change that
can merge.
Thanks,
-Stolee
next prev parent reply other threads:[~2026-07-31 13:23 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-06 13:50 [PATCH] sparse-index: avoid crash on intent-to-add entry outside the cone Derrick Stolee via GitGitGadget
2026-07-29 21:51 ` Junio C Hamano
2026-07-31 13:23 ` Derrick Stolee [this message]
2026-07-31 15:56 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e4cce4e2-4287-4e1a-8833-d37ee48ff7d6@gmail.com \
--to=stolee@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.