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

> 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.