On Mon, Feb 24, 2025 at 09:37:36PM +0800, shejialuo wrote:
> 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
