Re: [PATCH v2 1/2] fsr: package function should check for negative errors

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

On Tue, Mar 17, 2026 at 05:07:39PM +0100, cem@kernel.org wrote:
> From: Carlos Maiolino <cem@kernel.org>
> 
> xfrog_defragrange as most other functions from libfrog return
> a negative error value, while xfs_fsr's packfile(), expects
> a positive error value.
> 
> Whenever xfrog_defragrange fails, the switch case always falls into the
> default clausule, making the error message pointless.
> 
> Update packfile() to switch between negative errors.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
> V2:
> 	Change packfile() to check for negative error values
> 	instead of changing the return sign from xfrog_defragrange().
> 
>  fsr/xfs_fsr.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
> index 8845ff172fcb..aa2a10839314 100644
> --- a/fsr/xfs_fsr.c
> +++ b/fsr/xfs_fsr.c
> @@ -1463,17 +1463,17 @@ packfile(
>  	switch (error) {
>  		case 0:
>  			break;
> -	case ENOTSUP:
> +	case -ENOTSUP:

I would have just made the call site invert the sign:

	error = -xfrog_defragrange(...);

because you wouldn't have to change the case statements, and...

>  		if (vflag || dflag)
>  			fsrprintf(_("%s: file type not supported\n"), fname);
>  		break;
> -	case EFAULT:
> +	case -EFAULT:
>  		/* The file has changed since we started the copy */
>  		if (vflag || dflag)
>  			fsrprintf(_("%s: file modified defrag aborted\n"),
>  					fname);
>  		break;
> -	case EBUSY:
> +	case -EBUSY:
>  		/* Timestamp has changed or mmap'ed file */
>  		if (vflag || dflag)
>  			fsrprintf(_("%s: file busy\n"), fname);

...the default case below is still broken due to the strerror() call:

	default:
		fsrprintf(_("XFS_IOC_SWAPEXT failed: %s: %s\n"),
			  fname, strerror(error));

https://lore.kernel.org/linux-xfs/20260120172039.GO15551@frogsfrogsfrogs/

--D