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 <djwong@kernel.org>
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

diff --git a/include/platform_defs.h b/include/platform_defs.h
index 74a00583ebd6cb4996d486ed17d79396f5380e52..fa66551d99ff02de53497782ab66cc7dff7636a8 100644 (file)
--- 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 6e7af0020bfea2d0335bf9257e8d8e97aa7ec08b..c3c69a9de398db48d297743cab010249bb4a1dde 100644 (file)
--- 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 3fc23444f3f2491547c193610db013442ea76163..19eab66fad26f8e2a6e11baf9ae4b47f10294c82 100644 (file)
--- 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 e72a10ba7ee6da169507fd95e411fea13021687b..fecd9f0b93984acc41c3273c3ccd5d1a7e55b329 100644 (file)
--- 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 d56876c5be0e45d628ef654820181d7f44549994..973ae62d39cf9eabe16de125846ce977b9dcb172 100644 (file)
--- 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 0924ab7eb7dd39e5e4d4b3dd7114ea456d1a19d9..668c788dca796b2ea28f2fff0de93d4034ef5646 100644 (file)
--- 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 ea946616bfb5553ce5f47a3ce78196cc0e1a1ae1..ab207b9cc2d0b695b06bccae15e6a1124019b2fe 100644 (file)
--- 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 7a4eec49ca869348d79ee53db046c0b5cab2ca0f..7fbbc6387c6e9e0e854bcf46b63ce7b66ad10c1f 100644 (file)
--- 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 59a99bb42c2c9b8894d7f366751542dc30ac9017..0492cd55d5034f9a16481c6cedba9e0f72450c1a 100644 (file)
--- 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 704e66e9fbcd93fd7142757c50f16f0200871687..8a50bd03a55360e725490802b6c2f9a5345b0231 100644 (file)
--- 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 2f3c87be79f78325ba0b458214f6ecce462fcad0..4ed7cd99635bdab7bae1ec63381249f6248711f7 100644 (file)
--- 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(