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

> On Wed, Feb 19, 2025 at 08:41:03PM +0800, shejialuo wrote:
> > But there is one thing I want to argue with. I don't think we need to
> > rename "abort" callback to "release" and also "ref_iterator_abort" to
> > "ref_iterator_free" for the following reasons:
> > 
> > 1. We never call "release" expect in the "ref_iterator_free" function.
> > For other exposed functions "ref_iterator_advance", "ref_iterator_peel"
> > and the original "ref_iterator_abort". We will just call the registered
> > callback "advance", "peel" or "abort" via virtual table. I somehow think
> > we should follow this pattern. But I don't know actually.
> > 2. When I read the patch yesterday, I really wonder what is the
> > difference between "release" and "free". Why do we only change the
> > "ref_iterator_abort" to "ref_iterator_free" but for the callback, we
> > rename "abort" to "release". I know that you want to distinguish to
> > emphasis that we won't free the iterator but only release its resource
> > for ref iterator. But could abort also mean this?
> 
> The difference between "release" and "free" is explicitly documented in
> our CodingGuidelines. Quoting the relevant parts:
> 
>     - `S_release()` releases a structure's contents without freeing the
>       structure.
> 
>     - `S_free()` releases a structure's contents and frees the
>       structure.
> 
> So following these coding guidelines, we have to call the underlying
> implementations that are specific to the iterators `release()` because
> they don't free the iterator itself. And because the generic part _does_
> free the iterator itself in addition to releasing its state, it has to
> be called `free()`.
> 

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