Re: [RFC PATCH 1/2] libfrog: make xfrog_defragrange return a positive valued

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Jan 21, 2026 at 09:57:51AM +0100, Carlos Maiolino wrote:
> On Tue, Jan 20, 2026 at 09:20:39AM -0800, Darrick J. Wong wrote:
> > On Mon, Jan 19, 2026 at 03:26:50PM +0100, cem@kernel.org wrote:
> > > From: Carlos Maiolino <cem@kernel.org>
> > >
> > > Currently, the only user for xfrog_defragrange is xfs_fsr's packfile(),
> > > which expects error to be a positive value.
> > >
> > > Whenever xfrog_defragrange fails, the switch case always falls into the
> > > default clausule, making the error message pointless.
> > >
> > > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > > ---
> > >  libfrog/file_exchange.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/libfrog/file_exchange.c b/libfrog/file_exchange.c
> > > index e6c3f486b0ff..31bbc6da60c3 100644
> > > --- a/libfrog/file_exchange.c
> > > +++ b/libfrog/file_exchange.c
> > > @@ -232,7 +232,7 @@ xfrog_defragrange(
> > >  	if (ret) {
> > >  		if (errno == EOPNOTSUPP || errno != ENOTTY)
> > >  			goto legacy_fallback;
> > > -		return -errno;
> > > +		return errno;
> > 
> > Hrmm.  If you're going to change the polarity of the error numbers (e.g.
> > negative to positive) then please update the comments.
> 
> Sorry, I did it quickly and didn't even pay attention there were
> comments :)
> 
> > 
> > That said, I'd prefer to keep the errno polarity the same at least
> > within a .c file ... even though libfrog is a mess of different error
> > number return strategies.  What if the callsite changed to:
> > 
> > 	/* Swap the extents */
> > 	error = -xfrog_defragrange(...);
> 
> This looks to just add more confusion to it IMHO, it's not 'easy' for me
> at least to notice the minus sign in front of it.
> 
> What about just changing the switch case to catch for -ERROR instead of
> their positive counterparts? At least we stop playing the error sign
> change and just catch what we already have.

Sure, do whatever you think is best.  This whole part of C is an
unfixable mess as far as I'm concerned.

--D

> 
> > 
> > and
> > 
> > 	/* Snapshot file_fd before we start copying data... */
> > 	error = -xfrog_defragrange_prep(...);
> > 
> > (and I guess io/exchrange.c also needs a fix)
> > 
> > 	/* Snapshot the original file metadata in anticipation... */
> > 	ret = -xfrog_commitrange_prep(...);
> > 
> > Hrm?
> > 
> > --D
> > 
> > >  	}
> > >
> > >  	return 0;
> > > @@ -240,7 +240,7 @@ xfrog_defragrange(
> > >  legacy_fallback:
> > >  	ret = xfrog_ioc_swapext(file2_fd, xdf);
> > >  	if (ret)
> > > -		return -errno;
> > > +		return errno;
> > >
> > >  	return 0;
> > >  }
> > > --
> > > 2.52.0
> > >
> > >
> > 
>