From: Fedor Pchelkin Date: Mon, 6 Oct 2025 12:40:16 +0000 (+0200) Subject: xfs: refactor cmp_two_keys routines to take advantage of cmp_int() X-Git-Tag: v6.17.0~10 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=4a902e04d98ef28daab8bffc8c78ac59e426dd5a;p=thirdparty%2Fxfsprogs-dev.git xfs: refactor cmp_two_keys routines to take advantage of cmp_int() Source kernel commit: 3b583adf55c649d5ba37bcd1ca87644b0bc10b86 The net value of these functions is to determine the result of a three-way-comparison between operands of the same type. Simplify the code using cmp_int() to eliminate potential errors with opencoded casts and subtractions. This also means we can change the return value type of cmp_two_keys routines from int64_t to int and make the interface a bit clearer. Found by Linux Verification Center (linuxtesting.org). Suggested-by: Darrick J. Wong Signed-off-by: Fedor Pchelkin Reviewed-by: Darrick J. Wong Signed-off-by: Carlos Maiolino Signed-off-by: Andrey Albershteyn Reviewed-by: "Darrick J. Wong" --- diff --git a/include/platform_defs.h b/include/platform_defs.h index 74a00583..fa66551d 100644 --- a/include/platform_defs.h +++ b/include/platform_defs.h @@ -294,4 +294,6 @@ static inline bool __must_check __must_check_overflow(bool overflow) __a > __b ? (__a - __b) : (__b - __a); \ }) +#define cmp_int(l, r) ((l > r) - (l < r)) + #endif /* __XFS_PLATFORM_DEFS_H__ */ diff --git a/libxfs/xfs_alloc_btree.c b/libxfs/xfs_alloc_btree.c index 6e7af002..c3c69a9d 100644 --- a/libxfs/xfs_alloc_btree.c +++ b/libxfs/xfs_alloc_btree.c @@ -211,7 +211,7 @@ xfs_cntbt_cmp_key_with_cur( return (int64_t)be32_to_cpu(kp->ar_startblock) - rec->ar_startblock; } -STATIC int64_t +STATIC int xfs_bnobt_cmp_two_keys( struct xfs_btree_cur *cur, const union xfs_btree_key *k1, @@ -220,29 +220,24 @@ xfs_bnobt_cmp_two_keys( { ASSERT(!mask || mask->alloc.ar_startblock); - return (int64_t)be32_to_cpu(k1->alloc.ar_startblock) - - be32_to_cpu(k2->alloc.ar_startblock); + return cmp_int(be32_to_cpu(k1->alloc.ar_startblock), + be32_to_cpu(k2->alloc.ar_startblock)); } -STATIC int64_t +STATIC int xfs_cntbt_cmp_two_keys( struct xfs_btree_cur *cur, const union xfs_btree_key *k1, const union xfs_btree_key *k2, const union xfs_btree_key *mask) { - int64_t diff; - ASSERT(!mask || (mask->alloc.ar_blockcount && mask->alloc.ar_startblock)); - diff = be32_to_cpu(k1->alloc.ar_blockcount) - - be32_to_cpu(k2->alloc.ar_blockcount); - if (diff) - return diff; - - return be32_to_cpu(k1->alloc.ar_startblock) - - be32_to_cpu(k2->alloc.ar_startblock); + return cmp_int(be32_to_cpu(k1->alloc.ar_blockcount), + be32_to_cpu(k2->alloc.ar_blockcount)) ?: + cmp_int(be32_to_cpu(k1->alloc.ar_startblock), + be32_to_cpu(k2->alloc.ar_startblock)); } static xfs_failaddr_t diff --git a/libxfs/xfs_bmap_btree.c b/libxfs/xfs_bmap_btree.c index 3fc23444..19eab66f 100644 --- a/libxfs/xfs_bmap_btree.c +++ b/libxfs/xfs_bmap_btree.c @@ -377,29 +377,17 @@ xfs_bmbt_cmp_key_with_cur( cur->bc_rec.b.br_startoff; } -STATIC int64_t +STATIC int xfs_bmbt_cmp_two_keys( struct xfs_btree_cur *cur, const union xfs_btree_key *k1, const union xfs_btree_key *k2, const union xfs_btree_key *mask) { - uint64_t a = be64_to_cpu(k1->bmbt.br_startoff); - uint64_t b = be64_to_cpu(k2->bmbt.br_startoff); - ASSERT(!mask || mask->bmbt.br_startoff); - /* - * Note: This routine previously casted a and b to int64 and subtracted - * them to generate a result. This lead to problems if b was the - * "maximum" key value (all ones) being signed incorrectly, hence this - * somewhat less efficient version. - */ - if (a > b) - return 1; - if (b > a) - return -1; - return 0; + return cmp_int(be64_to_cpu(k1->bmbt.br_startoff), + be64_to_cpu(k2->bmbt.br_startoff)); } static xfs_failaddr_t diff --git a/libxfs/xfs_btree.h b/libxfs/xfs_btree.h index e72a10ba..fecd9f0b 100644 --- a/libxfs/xfs_btree.h +++ b/libxfs/xfs_btree.h @@ -184,7 +184,7 @@ struct xfs_btree_ops { * each key field to be used in the comparison must contain a nonzero * value. */ - int64_t (*cmp_two_keys)(struct xfs_btree_cur *cur, + int (*cmp_two_keys)(struct xfs_btree_cur *cur, const union xfs_btree_key *key1, const union xfs_btree_key *key2, const union xfs_btree_key *mask); diff --git a/libxfs/xfs_ialloc_btree.c b/libxfs/xfs_ialloc_btree.c index d56876c5..973ae62d 100644 --- a/libxfs/xfs_ialloc_btree.c +++ b/libxfs/xfs_ialloc_btree.c @@ -273,7 +273,7 @@ xfs_inobt_cmp_key_with_cur( cur->bc_rec.i.ir_startino; } -STATIC int64_t +STATIC int xfs_inobt_cmp_two_keys( struct xfs_btree_cur *cur, const union xfs_btree_key *k1, @@ -282,8 +282,8 @@ xfs_inobt_cmp_two_keys( { ASSERT(!mask || mask->inobt.ir_startino); - return (int64_t)be32_to_cpu(k1->inobt.ir_startino) - - be32_to_cpu(k2->inobt.ir_startino); + return cmp_int(be32_to_cpu(k1->inobt.ir_startino), + be32_to_cpu(k2->inobt.ir_startino)); } static xfs_failaddr_t diff --git a/libxfs/xfs_refcount_btree.c b/libxfs/xfs_refcount_btree.c index 0924ab7e..668c788d 100644 --- a/libxfs/xfs_refcount_btree.c +++ b/libxfs/xfs_refcount_btree.c @@ -187,7 +187,7 @@ xfs_refcountbt_cmp_key_with_cur( return (int64_t)be32_to_cpu(kp->rc_startblock) - start; } -STATIC int64_t +STATIC int xfs_refcountbt_cmp_two_keys( struct xfs_btree_cur *cur, const union xfs_btree_key *k1, @@ -196,8 +196,8 @@ xfs_refcountbt_cmp_two_keys( { ASSERT(!mask || mask->refc.rc_startblock); - return (int64_t)be32_to_cpu(k1->refc.rc_startblock) - - be32_to_cpu(k2->refc.rc_startblock); + return cmp_int(be32_to_cpu(k1->refc.rc_startblock), + be32_to_cpu(k2->refc.rc_startblock)); } STATIC xfs_failaddr_t diff --git a/libxfs/xfs_rmap_btree.c b/libxfs/xfs_rmap_btree.c index ea946616..ab207b9c 100644 --- a/libxfs/xfs_rmap_btree.c +++ b/libxfs/xfs_rmap_btree.c @@ -272,7 +272,7 @@ xfs_rmapbt_cmp_key_with_cur( return 0; } -STATIC int64_t +STATIC int xfs_rmapbt_cmp_two_keys( struct xfs_btree_cur *cur, const union xfs_btree_key *k1, @@ -281,36 +281,31 @@ xfs_rmapbt_cmp_two_keys( { const struct xfs_rmap_key *kp1 = &k1->rmap; const struct xfs_rmap_key *kp2 = &k2->rmap; - int64_t d; - __u64 x, y; + int d; /* Doesn't make sense to mask off the physical space part */ ASSERT(!mask || mask->rmap.rm_startblock); - d = (int64_t)be32_to_cpu(kp1->rm_startblock) - - be32_to_cpu(kp2->rm_startblock); + d = cmp_int(be32_to_cpu(kp1->rm_startblock), + be32_to_cpu(kp2->rm_startblock)); if (d) return d; if (!mask || mask->rmap.rm_owner) { - x = be64_to_cpu(kp1->rm_owner); - y = be64_to_cpu(kp2->rm_owner); - if (x > y) - return 1; - else if (y > x) - return -1; + d = cmp_int(be64_to_cpu(kp1->rm_owner), + be64_to_cpu(kp2->rm_owner)); + if (d) + return d; } if (!mask || mask->rmap.rm_offset) { /* Doesn't make sense to allow offset but not owner */ ASSERT(!mask || mask->rmap.rm_owner); - x = offset_keymask(be64_to_cpu(kp1->rm_offset)); - y = offset_keymask(be64_to_cpu(kp2->rm_offset)); - if (x > y) - return 1; - else if (y > x) - return -1; + d = cmp_int(offset_keymask(be64_to_cpu(kp1->rm_offset)), + offset_keymask(be64_to_cpu(kp2->rm_offset))); + if (d) + return d; } return 0; diff --git a/libxfs/xfs_rtrefcount_btree.c b/libxfs/xfs_rtrefcount_btree.c index 7a4eec49..7fbbc638 100644 --- a/libxfs/xfs_rtrefcount_btree.c +++ b/libxfs/xfs_rtrefcount_btree.c @@ -168,7 +168,7 @@ xfs_rtrefcountbt_cmp_key_with_cur( return (int64_t)be32_to_cpu(kp->rc_startblock) - start; } -STATIC int64_t +STATIC int xfs_rtrefcountbt_cmp_two_keys( struct xfs_btree_cur *cur, const union xfs_btree_key *k1, @@ -177,8 +177,8 @@ xfs_rtrefcountbt_cmp_two_keys( { ASSERT(!mask || mask->refc.rc_startblock); - return (int64_t)be32_to_cpu(k1->refc.rc_startblock) - - be32_to_cpu(k2->refc.rc_startblock); + return cmp_int(be32_to_cpu(k1->refc.rc_startblock), + be32_to_cpu(k2->refc.rc_startblock)); } static xfs_failaddr_t diff --git a/libxfs/xfs_rtrmap_btree.c b/libxfs/xfs_rtrmap_btree.c index 59a99bb4..0492cd55 100644 --- a/libxfs/xfs_rtrmap_btree.c +++ b/libxfs/xfs_rtrmap_btree.c @@ -214,7 +214,7 @@ xfs_rtrmapbt_cmp_key_with_cur( return 0; } -STATIC int64_t +STATIC int xfs_rtrmapbt_cmp_two_keys( struct xfs_btree_cur *cur, const union xfs_btree_key *k1, @@ -223,36 +223,31 @@ xfs_rtrmapbt_cmp_two_keys( { const struct xfs_rmap_key *kp1 = &k1->rmap; const struct xfs_rmap_key *kp2 = &k2->rmap; - int64_t d; - __u64 x, y; + int d; /* Doesn't make sense to mask off the physical space part */ ASSERT(!mask || mask->rmap.rm_startblock); - d = (int64_t)be32_to_cpu(kp1->rm_startblock) - - be32_to_cpu(kp2->rm_startblock); + d = cmp_int(be32_to_cpu(kp1->rm_startblock), + be32_to_cpu(kp2->rm_startblock)); if (d) return d; if (!mask || mask->rmap.rm_owner) { - x = be64_to_cpu(kp1->rm_owner); - y = be64_to_cpu(kp2->rm_owner); - if (x > y) - return 1; - else if (y > x) - return -1; + d = cmp_int(be64_to_cpu(kp1->rm_owner), + be64_to_cpu(kp2->rm_owner)); + if (d) + return d; } if (!mask || mask->rmap.rm_offset) { /* Doesn't make sense to allow offset but not owner */ ASSERT(!mask || mask->rmap.rm_owner); - x = offset_keymask(be64_to_cpu(kp1->rm_offset)); - y = offset_keymask(be64_to_cpu(kp2->rm_offset)); - if (x > y) - return 1; - else if (y > x) - return -1; + d = cmp_int(offset_keymask(be64_to_cpu(kp1->rm_offset)), + offset_keymask(be64_to_cpu(kp2->rm_offset))); + if (d) + return d; } return 0; diff --git a/repair/rcbag_btree.c b/repair/rcbag_btree.c index 704e66e9..8a50bd03 100644 --- a/repair/rcbag_btree.c +++ b/repair/rcbag_btree.c @@ -72,7 +72,7 @@ rcbagbt_cmp_key_with_cur( return 0; } -STATIC int64_t +STATIC int rcbagbt_cmp_two_keys( struct xfs_btree_cur *cur, const union xfs_btree_key *k1, @@ -81,25 +81,19 @@ rcbagbt_cmp_two_keys( { const struct rcbag_key *kp1 = (const struct rcbag_key *)k1; const struct rcbag_key *kp2 = (const struct rcbag_key *)k2; + int d; ASSERT(mask == NULL); - if (kp1->rbg_startblock > kp2->rbg_startblock) - return 1; - if (kp1->rbg_startblock < kp2->rbg_startblock) - return -1; - - if (kp1->rbg_blockcount > kp2->rbg_blockcount) - return 1; - if (kp1->rbg_blockcount < kp2->rbg_blockcount) - return -1; + d = cmp_int(kp1->rbg_startblock, kp2->rbg_startblock); + if (d) + return d; - if (kp1->rbg_ino > kp2->rbg_ino) - return 1; - if (kp1->rbg_ino < kp2->rbg_ino) - return -1; + d = cmp_int(kp1->rbg_blockcount, kp2->rbg_blockcount); + if (d) + return d; - return 0; + return cmp_int(kp1->rbg_ino, kp2->rbg_ino); } STATIC int diff --git a/scrub/inodes.c b/scrub/inodes.c index 2f3c87be..4ed7cd99 100644 --- a/scrub/inodes.c +++ b/scrub/inodes.c @@ -197,8 +197,6 @@ bulkstat_the_rest( return seen_mask; } -#define cmp_int(l, r) ((l > r) - (l < r)) - /* Compare two bulkstat records by inumber. */ static int compare_bstat(