From: Karthik Nayak Date: Wed, 19 Feb 2025 20:10:54 GMT Subject: Re: [PATCH 09/14] refs/iterator: implement seeking for merged iterators Message-ID: In-Reply-To: <20250217-pks-update-ref-optimization-v1-9-a2b6d87a24af@pks.im> Patrick Steinhardt writes: > Implement seeking on merged iterators. The implementation is rather > straight forward, with the only exception that we must not deallocate > the underlying iterators once they have been exhausted. > > Signed-off-by: Patrick Steinhardt > --- > refs/iterator.c | 38 +++++++++++++++++++++++++++++--------- > 1 file changed, 29 insertions(+), 9 deletions(-) > > diff --git a/refs/iterator.c b/refs/iterator.c > index 757b105261a..63608ef9907 100644 > --- a/refs/iterator.c > +++ b/refs/iterator.c > @@ -96,7 +96,8 @@ int is_empty_ref_iterator(struct ref_iterator *ref_iterator) > struct merge_ref_iterator { > struct ref_iterator base; > > - struct ref_iterator *iter0, *iter1; > + struct ref_iterator *iter0, *iter0_owned; > + struct ref_iterator *iter1, *iter1_owned; > > ref_iterator_select_fn *select; > void *cb_data; > @@ -160,13 +161,11 @@ static int merge_ref_iterator_advance(struct ref_iterator *ref_iterator) > if (!iter->current) { > /* Initialize: advance both iterators to their first entries */ > if ((ok = ref_iterator_advance(iter->iter0)) != ITER_OK) { > - ref_iterator_free(iter->iter0); > iter->iter0 = NULL; Okay so if advancing the iterator fails, we set the current iterator to NULL but the underlying pointer `iter0_owned` still holds. Okay makes sense. Now it's just a matter of ensuring that we reuse the original iterator when needed and seems like that' what we do in this patch. Looks good! [snip]