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

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 1362 bytes --]

On Wed, Mar 20, 2024 at 02:50:43PM -0700, Karthik Nayak wrote:
> Patrick Steinhardt <ps@pks.im> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]