Patrick Steinhardt <ps@xxxxxx> writes:
>> + * When the REF_ITERATOR_SEEK_SET_PREFIX flag is set, the iterator's prefix is
>> + * updated to match the seek string, affecting all subsequent iterations. If
>> + * not, the iterator seeks to the specified reference and clears any previously
>> + * set prefix.
>> *
>> * Returns 0 on success, a negative error code otherwise.
>> */
>> -int ref_iterator_seek(struct ref_iterator *ref_iterator,
>> - const char *prefix);
>> +int ref_iterator_seek(struct ref_iterator *ref_iterator, const char *seek,
>> + unsigned int flags);
>
> Another tiny nit: instead of calling the variable `seek` we can just
> call it `refname`. That might give a bit more of a hint what you're
> actually seeking for.
>
> But other than that I'm happy with the new behaviour, where we are now
> consistently either setting or resetting the prefix depending on whether
> or not the caller set the flag.
I am not sure. The way the "prefix" is used, if I understand correctly, is
- it is set by iterator-begin, typically to the area to iterate
over (e.g. "refs/heads/" for iterating over branches) in the
for_each_ref_*() family of helpers, and internally we seek to
that area (skipping anything that come strictly before
"refs/heads/" for example).
- iterator-advance looks at it and decides we are done when the
iterator points beyond that prefix
So if you are iterating inside "refs/heads/" hierarchy and seek to
"refs/heads/m", don't you still want to stop when you step outside
"refs/heads/" by keeping the original prefix, instead of unsetting
the prefix to empty? A postimage of this patch for packed backend
(picked at random) reads like this:
static int packed_ref_iterator_seek(struct ref_iterator *ref_iterator,
const char *seek, unsigned int flags)
{
struct packed_ref_iterator *iter =
(struct packed_ref_iterator *)ref_iterator;
const char *start;
if (seek && *seek)
start = find_reference_location(iter->snapshot, seek, 0);
else
start = iter->snapshot->start;
/* Unset any previously set prefix */
FREE_AND_NULL(iter->prefix);
if (flags & REF_ITERATOR_SEEK_SET_PREFIX)
iter->prefix = xstrdup_or_null(seek);
so after (true) seeking that does not have the SET_PREFIX flag on,
wouldn't our iterator-advance run through the end since it no longer
is aware of where to stop?
Thanks.
