From: Karthik Nayak Date: Mon, 29 Sep 2025 14:47:50 GMT Subject: Re: [PATCH v2] refs/ref-cache: fix SEGFAULT when seeking in empty directories Message-ID: In-Reply-To: Junio C Hamano writes: > Karthik Nayak 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. Yeah, I think something like this would be much better, I'll amend the commit and send in a new version soon. Thanks! Karthik