Subject: Re: [PATCH 07/14] refs/iterator: separate lifecycle from iteration

From: Patrick Steinhardt <[email-masked]>
Date: Wed, 19 Feb 2025 14:17:07 +0100

On Wed, Feb 19, 2025 at 09:06:58PM +0800, shejialuo wrote:
> On Wed, Feb 19, 2025 at 01:59:13PM +0100, Patrick Steinhardt wrote:
> > Regarding the question why to even rename `ref_iterator_abort()` itself:
> > this is done to avoid confusion going forward. Previously it really only
> > had to be called when you actually wanted to abort an ongoing iteration
> > over its yielded references. This is not the case anymore, and now you
> > have to call it unconditionally after you're done with the iterator. So
> > while the naming previously made sense, now it doesn't anymore.
> > 
> 
> Good point, I didn't realise this part. Thanks for the detailed
> explanation. I will continue to review the later patches. However, I
> won't touch the oid part, because I am not familiar with this. By the
> way, I think we miss out one thing in this patch:
> 
> We forget to free the dir iterator defined in the
> "files-backend.c::files_fsck_refs_dir". I have just remembered that I
> use dir iterator when checking the ref consistency.

Hm, good point. Why doesn't CI complain about this leak...? I'll
investigate, thanks for the hint!

Patrick