Subject: Re: [PATCH 01/15] reftable/stack: fix error handling in `reftable_stack_init_addition()`

From: Patrick Steinhardt <[email-masked]>
Date: Mon, 25 Mar 2024 10:10:26 +0100

On Wed, Mar 20, 2024 at 02:50:43PM -0700, Karthik Nayak wrote:
> Patrick Steinhardt <[email-masked]> writes:
> 
> > diff --git a/reftable/stack.c b/reftable/stack.c
> > index 1ecf1b9751..92d9a7facb 100644
> > --- a/reftable/stack.c
> > +++ b/reftable/stack.c
> > @@ -590,8 +590,7 @@ static int reftable_stack_init_addition(struct reftable_addition *add,
> >  	err = stack_uptodate(st);
> >  	if (err < 0)
> >  		goto done;
> > -
> > -	if (err > 1) {
> > +	if (err > 0) {
> >  		err = REFTABLE_LOCK_ERROR;
> >  		goto done;
> >  	}
> > @@ -713,10 +712,6 @@ static int stack_try_add(struct reftable_stack *st,
> >  	int err = reftable_stack_init_addition(&add, st);
> >  	if (err < 0)
> >  		goto done;
> > -	if (err > 0) {
> > -		err = REFTABLE_LOCK_ERROR;
> > -		goto done;
> > -	}
> 
> This changes the behavior though, since now we skip the `goto done`. It
> would be best to change the previous line to `if (err)`, which is what
> the other function (`reftable_stack_new_addition`) does.

It actually doesn't because `reftable_stack_init_addition()` does not
return a positive value anymore. And as it returns `REFTABLE_LOCK_ERROR`
in the case where it previously did return a positive value the
behaviour is exactly the same because we go into thee `if (err < 0)`
branch.

I'll amend the commit message to clariyf.

Patrick