On Fri, May 10, 2024 at 02:18:16PM -0500, Justin Tobler wrote:
> On 24/05/08 01:04PM, Patrick Steinhardt wrote:
> > To initialize a `struct merged_iter`, we need to seek all subiterators
> > to the wanted record and then add their results to the priority queue
> > used to sort the records. This logic is split up across two functions,
> > `merged_table_seek_record()` and `merged_table_iter()`. The scope of
>
> Did we mean `merged_iter_init` instead of `merged_table_iter()` here?
Indeed, good catch.
> > @@ -246,32 +230,33 @@ static int merged_table_seek_record(struct reftable_merged_table *mt,
> > struct reftable_iterator *it,
> > struct reftable_record *rec)
> > {
> > - struct merged_iter merged = {
> > - .typ = reftable_record_type(rec),
> > - .hash_id = mt->hash_id,
> > - .suppress_deletions = mt->suppress_deletions,
> > - .advance_index = -1,
> > - };
> > - struct merged_iter *p;
> > + struct merged_iter merged, *p;
> > int err;
> >
> > - REFTABLE_CALLOC_ARRAY(merged.subiters, mt->stack_len);
> > + merged_iter_init(&merged, mt);
> > +
> > for (size_t i = 0; i < mt->stack_len; i++) {
> > + reftable_record_init(&merged.subiters[merged.stack_len].rec,
> > + reftable_record_type(rec));
>
> I find it somewhat confusing how we increment the subiterator index
> here. I assume this is because when a record is not found we don't want
> to add it to the index. Might be nice to add a comment here explaining
> this.
>
> Edit: Looks like you address this in the next commit so I guess we are
> good :)
Seconded. And yes, as you noticed, this is one of the reasons why I
change this in the next patch.
Patrick
Attachment:
signature.asc
Description: PGP signature
