Re: [PATCH 6/6] xfs: refactor xfs_btree_diff_two_ptrs() to take advantage of cmp_int()

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

On Thu, Jun 12, 2025 at 01:24:50PM +0300, Fedor Pchelkin wrote:
> Use cmp_int() to yield the result of a three-way-comparison instead of
> performing subtractions with extra casts.
> 
> Found by Linux Verification Center (linuxtesting.org).
> 
> Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
> ---
>  fs/xfs/libxfs/xfs_btree.c | 6 +++---
>  fs/xfs/libxfs/xfs_btree.h | 6 +++---
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
> index d3591728998e..9a227b6b3296 100644
> --- a/fs/xfs/libxfs/xfs_btree.c
> +++ b/fs/xfs/libxfs/xfs_btree.c
> @@ -5353,15 +5353,15 @@ xfs_btree_count_blocks(
>  }
>  
>  /* Compare two btree pointers. */
> -int64_t
> +int
>  xfs_btree_diff_two_ptrs(

I'm surprised you didn't rename the diff.*ptr functions too -- there's
no inherent meaning in the difference between two pointers, but it is
useful to compare them.  Renaming would make the sole callsite much
clearer in purpose:

	if (xfs_btree_cmp_two_ptrs(cur, pp, sibling) != 0)
		xchk_btree_set_corrupt(bs->sc, cur, level);

Other than that, the logic changes look correct to me.

--D

>  	struct xfs_btree_cur		*cur,
>  	const union xfs_btree_ptr	*a,
>  	const union xfs_btree_ptr	*b)
>  {
>  	if (cur->bc_ops->ptr_len == XFS_BTREE_LONG_PTR_LEN)
> -		return (int64_t)be64_to_cpu(a->l) - be64_to_cpu(b->l);
> -	return (int64_t)be32_to_cpu(a->s) - be32_to_cpu(b->s);
> +		return cmp_int(be64_to_cpu(a->l), be64_to_cpu(b->l));
> +	return cmp_int(be32_to_cpu(a->s), be32_to_cpu(b->s));
>  }
>  
>  struct xfs_btree_has_records {
> diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h
> index 1bf20d509ac9..23598f287af5 100644
> --- a/fs/xfs/libxfs/xfs_btree.h
> +++ b/fs/xfs/libxfs/xfs_btree.h
> @@ -519,9 +519,9 @@ struct xfs_btree_block *xfs_btree_get_block(struct xfs_btree_cur *cur,
>  		int level, struct xfs_buf **bpp);
>  bool xfs_btree_ptr_is_null(struct xfs_btree_cur *cur,
>  		const union xfs_btree_ptr *ptr);
> -int64_t xfs_btree_diff_two_ptrs(struct xfs_btree_cur *cur,
> -				const union xfs_btree_ptr *a,
> -				const union xfs_btree_ptr *b);
> +int xfs_btree_diff_two_ptrs(struct xfs_btree_cur *cur,
> +			    const union xfs_btree_ptr *a,
> +			    const union xfs_btree_ptr *b);
>  void xfs_btree_get_sibling(struct xfs_btree_cur *cur,
>  			   struct xfs_btree_block *block,
>  			   union xfs_btree_ptr *ptr, int lr);
> -- 
> 2.49.0
> 
>