Re: [PATCH v2 2/2] xfs: clean up attr intent recovery error paths

[Darrick J. Wong]

On Wed, Jun 10, 2026 at 10:20:28AM +0800, Yingjie Gao wrote:
> xfs_attr_recover_work() still uses a backward goto from out_cancel to
> out_unlock.

What's so bad about backwards gotos?

--D

> Restructure the cleanup labels into the same linear fallthrough style
> used by the neighboring bmap and exchmaps recovery helpers.  Initialize
> the local error variable to zero as those helpers do as well, which
> makes the shared return path easier to follow.
> 
> No functional change intended.
> 
> Signed-off-by: Yingjie Gao <gaoyingjie@xxxxxxxxxxxxx>
> ---
>  fs/xfs/xfs_attr_item.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
> index 841838bc1d0f..f8aa9dd80bb9 100644
> --- a/fs/xfs/xfs_attr_item.c
> +++ b/fs/xfs/xfs_attr_item.c
> @@ -739,7 +739,7 @@ xfs_attr_recover_work(
>  	struct xfs_trans_res		resv;
>  	struct xfs_attri_log_format	*attrp;
>  	struct xfs_attri_log_nameval	*nv = attrip->attri_nameval;
> -	int				error;
> +	int				error = 0;
>  	unsigned int			total = 0;
>  
>  	/*
> @@ -789,14 +789,20 @@ xfs_attr_recover_work(
>  		goto out_cancel;
>  
>  	error = xfs_defer_ops_capture_and_commit(tp, capture_list);
> +	if (error)
> +		goto out_unlock;
> +
> +	xfs_iunlock(ip, XFS_ILOCK_EXCL);
> +	xfs_irele(ip);
> +	return 0;
> +
> +out_cancel:
> +	xfs_trans_cancel(tp);
>  out_unlock:
>  	xfs_iunlock(ip, XFS_ILOCK_EXCL);
>  out_rele:
>  	xfs_irele(ip);
>  	return error;
> -out_cancel:
> -	xfs_trans_cancel(tp);
> -	goto out_unlock;
>  }
>  
>  /* Re-log an intent item to push the log tail forward. */
> -- 
> 2.20.1
> 
>