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

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

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.

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(...);

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
> 
>