Re: [PATCH] ref-iterator-seek: correctly initialize the prefix_state for a new level
On Wed, Jul 23, 2025 at 2:51 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 14 quoted lines
>
> When cache_ref_iterator_seek() "jumps" to a middle of the sorted ref
> list, it forgets to set the .prefix_state member of the new
> (i.e. deeper) level it just initialized. This later causes
> cache_ref_iterator_advance() to look at this uninitialized member
> to base its decision on what to do next.
>
> Kyle Lippincott [*] and Jeff King noticed this with MSAN and
> Valgrind, and Karthik Nayak as the original author located exactly
> where the missing initialization is.
>
> [*] <CAO_smVg9TDakUnubepjPGmLyOzW6n8Z=MDbnZKvkwN2=kN2RRw@mail.gmail.com>
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Looks good to me, thanks!
Show 28 quoted lines
> ---
> refs/ref-cache.c | 1 +
> 1 file changed, 1 insertion(+)
>
> * I had this as "fixup!" on top of your topic for quite a while and
> forgot to ask you to send in an official fix. As Kyle's
> discovery was after the topic hit 'next' (understandable, as
> their internal edition of Git is based on 'next'), we need a
> separate fix on top.
>
> To prepare for merging down the whole thing to 'master', I wrote
> the proposed log message to help expedite the process. Comments?
>
> diff --git a/refs/ref-cache.c b/refs/ref-cache.c
> index 1d95b56d40..ceef3a2008 100644
> --- a/refs/ref-cache.c
> +++ b/refs/ref-cache.c
> @@ -527,6 +527,7 @@ static int cache_ref_iterator_seek(struct ref_iterator *ref_iterator,
> level = &iter->levels[iter->levels_nr++];
> level->dir = dir;
> level->index = -1;
> + level->prefix_state = PREFIX_CONTAINS_DIR;
> } else {
> /* reduce the index so the leaf node is iterated over */
> if (cmp <= 0 && !slash)
> --
> 2.50.1-521-gf11ee0bd80
>