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>
This commit is contained in:
Fedor Pchelkin
2025-07-02 12:39:30 +03:00
committed by Carlos Maiolino
parent 82b63ee160
commit 3b583adf55
9 changed files with 48 additions and 84 deletions

View File

@@ -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

View File

@@ -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

View File

@@ -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);

View File

@@ -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

View File

@@ -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

View File

@@ -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;

View File

@@ -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

View File

@@ -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;

View File

@@ -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