Re: [PATCH v2 11/16] refs/iterator: implement seeking for merged iterators
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Feb 25, 2025, 07:39 UTC
- Message-ID
- <Z71zwGNcKaX7mGAl@pks.im>
- In-Reply-To
- <Z7x2IEdRP4fzdXMo@ArchLinux>
On Mon, Feb 24, 2025 at 09:37:36PM +0800, shejialuo wrote:
Show 28 quoted lines
> On Wed, Feb 19, 2025 at 02:23:38PM +0100, Patrick Steinhardt wrote:
> > 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)
> > +static int merge_ref_iterator_seek(struct ref_iterator *ref_iterator,
> > + const char *prefix)
> > +{
> > + struct merge_ref_iterator *iter =
> > + (struct merge_ref_iterator *)ref_iterator;
> > + int ret;
> > +
> > + iter->current = NULL;
> > + iter->iter0 = iter->iter0_owned;
> > + iter->iter1 = iter->iter1_owned;
> > +
> > + ret = ref_iterator_seek(iter->iter0, prefix);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = ref_iterator_seek(iter->iter1, prefix);
> > + if (ret < 0)
> > + return ret;
>
> We could simply use a single `if` statement to handle this. Is the
> reason why we design this is that we want to return the exact error code
> for each case?Yup, I don't want to loose the error code. We could write this as:
if ((ret = ref_iterator_seek(iter->iter0, prefix)) < 0 ||
(ret = ref_iterator_seek(iter->iter0, prefix)) < 0)
return ret;But assigning to variables in conditions is not something we typically do in the Git codebase.
Patrick