xfs: refactor cmp_two_keys routines to take advantage of cmp_int()

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>

diff --git a/fs/xfs/libxfs/xfs_alloc_btree.c b/fs/xfs/libxfs/xfs_alloc_btree.c
index d01807e0c4d4fdcabefda6c188ef13ddbfc4b3bc..f371f1b32cfbe9ebe558dfb3c08d4ed071fdbeee 100644 (file)
--- a/fs/xfs/libxfs/xfs_alloc_btree.c
+++ b/fs/xfs/libxfs/xfs_alloc_btree.c
@@ -213,7 +213,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,
@@ -222,29 +222,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/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c
index 0c5dba21d94a1f2fb882236ea5a12c25b586a055..bfe67e5d4d112bf949a87a9f0775fbffd4aaa825 100644 (file)
--- a/fs/xfs/libxfs/xfs_bmap_btree.c
+++ b/fs/xfs/libxfs/xfs_bmap_btree.c
@@ -378,29 +378,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/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h
index e72a10ba7ee6da169507fd95e411fea13021687b..fecd9f0b93984acc41c3273c3ccd5d1a7e55b329 100644 (file)
--- a/fs/xfs/libxfs/xfs_btree.h
+++ b/fs/xfs/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/fs/xfs/libxfs/xfs_ialloc_btree.c b/fs/xfs/libxfs/xfs_ialloc_btree.c
index 0d0c6534259acd2d9e6c745d57530d183e4a2f91..ab9fce20b083876dd5dda9948fe60967d5f20537 100644 (file)
--- a/fs/xfs/libxfs/xfs_ialloc_btree.c
+++ b/fs/xfs/libxfs/xfs_ialloc_btree.c
@@ -274,7 +274,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,
@@ -283,8 +283,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/fs/xfs/libxfs/xfs_refcount_btree.c b/fs/xfs/libxfs/xfs_refcount_btree.c
index 885fc3a0a304e1cff0942fedce2b221907e2041e..1c3996b11563d3eb545b78f01594fb8cfa39817e 100644 (file)
--- a/fs/xfs/libxfs/xfs_refcount_btree.c
+++ b/fs/xfs/libxfs/xfs_refcount_btree.c
@@ -188,7 +188,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,
@@ -197,8 +197,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/fs/xfs/libxfs/xfs_rmap_btree.c b/fs/xfs/libxfs/xfs_rmap_btree.c
index 74288d311b6807f673bc454afe4fd13aa99055ec..3cccdb0d04182a3a2a4ae3c9369f9c84f68e17b1 100644 (file)
--- a/fs/xfs/libxfs/xfs_rmap_btree.c
+++ b/fs/xfs/libxfs/xfs_rmap_btree.c
@@ -273,7 +273,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,
@@ -282,36 +282,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/fs/xfs/libxfs/xfs_rtrefcount_btree.c b/fs/xfs/libxfs/xfs_rtrefcount_btree.c
index 864c3aa664d76e2b13c995fa91d6ba0be0e13467..d9f79ae579c6950b8f2a48f19f9233f4a7655017 100644 (file)
--- a/fs/xfs/libxfs/xfs_rtrefcount_btree.c
+++ b/fs/xfs/libxfs/xfs_rtrefcount_btree.c
@@ -170,7 +170,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,
@@ -179,8 +179,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/fs/xfs/libxfs/xfs_rtrmap_btree.c b/fs/xfs/libxfs/xfs_rtrmap_btree.c
index b48336086ca7e8d6b9d39a3e84979077f29a6905..231a189ea2fe6889883ef083b97a2e26cf782a00 100644 (file)
--- a/fs/xfs/libxfs/xfs_rtrmap_btree.c
+++ b/fs/xfs/libxfs/xfs_rtrmap_btree.c
@@ -215,7 +215,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,
@@ -224,36 +224,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/fs/xfs/scrub/rcbag_btree.c b/fs/xfs/scrub/rcbag_btree.c
index 523ffd0da77a39f1f0289fdd231b5f0c2e4f6756..46598817b239185710b66b6033d546162f04d7f9 100644 (file)
--- a/fs/xfs/scrub/rcbag_btree.c
+++ b/fs/xfs/scrub/rcbag_btree.c
@@ -68,7 +68,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,
@@ -80,17 +80,8 @@ rcbagbt_cmp_two_keys(
 
        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;
-
-       return 0;
+       return cmp_int(kp1->rbg_startblock, kp2->rbg_startblock) ?:
+              cmp_int(kp1->rbg_blockcount, kp2->rbg_blockcount);
 }
 
 STATIC int