Subject: Re: [PATCH 01/10] reftable/merged: expose functions to initialize iterators

From: Patrick Steinhardt <[email-masked]>
Date: Wed, 14 Aug 2024 14:56:48 +0200

On Tue, Aug 13, 2024 at 05:36:27AM -0400, karthik nayak wrote:
> Patrick Steinhardt <[email-masked]> writes:
> 
> > We do not expose any functions via our public headers that would allow a
> > caller to initialize a reftable iterator from a merged table. Instead,
> > they are expected to go via the generic `reftable_table` interface,
> > which is somewhat roundabout.
> >
> > Implement two new functions to initialize iterators for ref and log
> > records to plug this gap.
> >
> > Signed-off-by: Patrick Steinhardt <[email-masked]>
> > ---
> >  reftable/merged.c          | 12 ++++++++++++
> >  reftable/reftable-merged.h |  8 ++++++++
> >  2 files changed, 20 insertions(+)
> >
> > diff --git a/reftable/merged.c b/reftable/merged.c
> > index 6adce44f4b..8d78b3da71 100644
> > --- a/reftable/merged.c
> > +++ b/reftable/merged.c
> > @@ -254,6 +254,18 @@ void merged_table_init_iter(struct reftable_merged_table *mt,
> >  	iterator_from_merged_iter(it, mi);
> >  }
> >
> > +void reftable_merged_table_init_ref_iterator(struct reftable_merged_table *mt,
> > +					     struct reftable_iterator *it)
> > +{
> > +	merged_table_init_iter(mt, it, BLOCK_TYPE_REF);
> > +}
> > +
> > +void reftable_merged_table_init_log_iterator(struct reftable_merged_table *mt,
> > +					     struct reftable_iterator *it)
> > +{
> > +	merged_table_init_iter(mt, it, BLOCK_TYPE_LOG);
> > +}
> > +
> 
> These too look similar to `static void
> reftable_merged_table_init_iter_void` defined a little below, I wonder
> if we could have simply exposed that? Perhaps we remove it in the
> upcoming patches.

Yup, `reftable_merged_table_init_iter_void()` is about to go away. But
regardless of that, even if it didn't it would make sense to have both
functions. The `iter_void()` one takes an arbitrary block type as input,
which we never expose via our public interfaces. So it has to stay an
implementation detail. The new ones added here do not expose the block
type.

Patrick