Re: [PATCH v2] refs/ref-cache: fix SEGFAULT when seeking in empty directories

[Junio C Hamano <gitster@pobox.com>]

Karthik Nayak <karthik.188@gmail.com> writes:

> This wouldn't happen because before the loop starts, we set:
>
>    dir = get_ref_dir(iter->cache->root);
>
> So, `dir` will always have an entry (i.e. 'refs') in the first
> iteration, after that, the checks in the while section  of the 'do {}
> while (...)' loop will kick in.

Ah, OK.  So we know dir->nr != 0 when the loop is entered for the
first time.  We may descend into a new subdirectory and at that
point we might hit dir->nr == 0 and that is why we want to catch it
at the end of "do {} while ()".  Makes sense.

>> I wonder if we can leave at
>> the beginning of the outer loop, even before sort_ref_dir(dir), when
>> dir->nr is zero, or something?
>
> We could add it there too. I can't see the merit of one over the other.

No, I don't, either.  As long as we know dir->nr != 0 upon the entry
into the loop, the check at the tail end in while() is sufficient.

Thanks for clarifying.  I wonder if a bit of update to the log
message would help future readers of the code, though.

    When using the files-backend with packed-refs, it is possible that some
    of the refs directories are empty. For e.g. just after repacking, the
    'refs/heads' directory would be empty. The ref-cache seek mechanism,
    doesn't take this into consideration when descending into a subdirectory,
    and makes an out of bounds access, causing SEGFAULT as we try to
    access entries within the directory. Fix this by breaking out of the
    loop when we enter an empty directory.

or something?  I dunno.

Thanks.