Re: [PATCH v2 12/16] refs/iterator: implement seeking for reftable iterators

[shejialuo <shejialuo@gmail.com>]

On Wed, Feb 19, 2025 at 02:23:39PM +0100, Patrick Steinhardt wrote:
> Implement seeking of reftable iterators. As the low-level reftable
> iterators already support seeking this change is straight-forward. Two
> notes though:
> 
>   - We do not support seeking on reflog iterators.
> 
>   - We start to check whether `reftable_stack_init_ref_iterator()` is
>     successful.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  refs/reftable-backend.c | 35 ++++++++++++++++++++++++++++++-----
>  1 file changed, 30 insertions(+), 5 deletions(-)
> 
> diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
> index 06543f79c64..b0c09f34433 100644
> --- a/refs/reftable-backend.c
> +++ b/refs/reftable-backend.c
> @@ -547,7 +547,7 @@ struct reftable_ref_iterator {
>  	struct reftable_ref_record ref;
>  	struct object_id oid;
>  
> -	const char *prefix;
> +	char *prefix;
>  	size_t prefix_len;
>  	char **exclude_patterns;
>  	size_t exclude_patterns_index;
> @@ -718,6 +718,20 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator)
>  	return ITER_OK;
>  }
>  
> +static int reftable_ref_iterator_seek(struct ref_iterator *ref_iterator,
> +				      const char *prefix)
> +{
> +	struct reftable_ref_iterator *iter =
> +		(struct reftable_ref_iterator *)ref_iterator;
> +
> +	free(iter->prefix);
> +	iter->prefix = xstrdup_or_null(prefix);
> +	iter->prefix_len = prefix ? strlen(prefix) : 0;
> +	iter->err = reftable_iterator_seek_ref(&iter->iter, prefix);

Should we rename this function `reftable_iterator_seek_ref` by the way?
It is a little strange that we have two functions which are so similar:

1. reftable_ref_iterator_seek
2. reftable_iterator_seek_ref

However, don't worth a reroll.

> +
> +	return iter->err;
> +}
> +
>  static int reftable_ref_iterator_peel(struct ref_iterator *ref_iterator,
>  				      struct object_id *peeled)
>  {
> @@ -744,10 +758,12 @@ static void reftable_ref_iterator_release(struct ref_iterator *ref_iterator)
>  			free(iter->exclude_patterns[i]);
>  		free(iter->exclude_patterns);
>  	}
> +	free(iter->prefix);
>  }
>  
>  static struct ref_iterator_vtable reftable_ref_iterator_vtable = {
>  	.advance = reftable_ref_iterator_advance,
> +	.seek = reftable_ref_iterator_seek,
>  	.peel = reftable_ref_iterator_peel,
>  	.release = reftable_ref_iterator_release,
>  };
> @@ -806,8 +822,6 @@ static struct reftable_ref_iterator *ref_iterator_for_stack(struct reftable_ref_
>  
>  	iter = xcalloc(1, sizeof(*iter));
>  	base_ref_iterator_init(&iter->base, &reftable_ref_iterator_vtable);
> -	iter->prefix = prefix;
> -	iter->prefix_len = prefix ? strlen(prefix) : 0;

We don't assign `iter->prefix` and `iter->prefix_len` here. This is
because we want to use the new defined function
`reftable_ref_iterator_seek`. In the fist glance, I am worried that
"iter->prefix" might not be `NULL`. However, because we use `xcalloc`,
"iter->prefix" would be `NULL` by default.

Thanks,
Jialuo