Skip to content

Commit 82a2479

Browse files
committed
More consistent use of TREE_* macros in AVL comparators
Where is it appropriate and obvious, use TREE_CMP(), TREE_ISIGN() and TREE_PCMP() instead or direct comparisons. It can make the code a lot smaller, less error prone, and easier to read. Sponsored-by: TrueNAS Signed-off-by: Rob Norris <rob.norris@truenas.com>
1 parent f8457fb commit 82a2479

File tree

19 files changed

+89
-199
lines changed

19 files changed

+89
-199
lines changed

cmd/zed/zed_exec.c

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -45,21 +45,10 @@ struct launched_process_node {
4545
static int
4646
_launched_process_node_compare(const void *x1, const void *x2)
4747
{
48-
pid_t p1;
49-
pid_t p2;
48+
const struct launched_process_node *node1 = x1;
49+
const struct launched_process_node *node2 = x2;
5050

51-
assert(x1 != NULL);
52-
assert(x2 != NULL);
53-
54-
p1 = ((const struct launched_process_node *) x1)->pid;
55-
p2 = ((const struct launched_process_node *) x2)->pid;
56-
57-
if (p1 < p2)
58-
return (-1);
59-
else if (p1 == p2)
60-
return (0);
61-
else
62-
return (1);
51+
return (TREE_CMP(node1->pid, node2->pid));
6352
}
6453

6554
static pthread_t _reap_children_tid = (pthread_t)-1;

cmd/zed/zed_strings.c

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -42,26 +42,10 @@ typedef struct zed_strings_node zed_strings_node_t;
4242
static int
4343
_zed_strings_node_compare(const void *x1, const void *x2)
4444
{
45-
const char *s1;
46-
const char *s2;
47-
int rv;
45+
const zed_strings_node_t *n1 = x1;
46+
const zed_strings_node_t *n2 = x2;
4847

49-
assert(x1 != NULL);
50-
assert(x2 != NULL);
51-
52-
s1 = ((const zed_strings_node_t *) x1)->key;
53-
assert(s1 != NULL);
54-
s2 = ((const zed_strings_node_t *) x2)->key;
55-
assert(s2 != NULL);
56-
rv = strcmp(s1, s2);
57-
58-
if (rv < 0)
59-
return (-1);
60-
61-
if (rv > 0)
62-
return (1);
63-
64-
return (0);
48+
return (TREE_ISIGN(strcmp(n1->key, n2->key)));
6549
}
6650

6751
/*

cmd/zfs/zfs_iter.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -413,14 +413,12 @@ zfs_sort(const void *larg, const void *rarg)
413413

414414
if (lstr)
415415
ret = TREE_ISIGN(strcmp(lstr, rstr));
416-
else if (lnum < rnum)
417-
ret = -1;
418-
else if (lnum > rnum)
419-
ret = 1;
416+
else
417+
ret = TREE_CMP(lnum, rnum);
420418

421419
if (ret != 0) {
422420
if (psc->sc_reverse == B_TRUE)
423-
ret = (ret < 0) ? 1 : -1;
421+
ret = -ret;
424422
return (ret);
425423
}
426424
}

cmd/zfs/zfs_main.c

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2905,15 +2905,13 @@ us_compare(const void *larg, const void *rarg)
29052905
uint64_t rv64 = 0;
29062906
zfs_prop_t prop = sortcol->sc_prop;
29072907
const char *propname = NULL;
2908-
boolean_t reverse = sortcol->sc_reverse;
29092908

29102909
switch (prop) {
29112910
case ZFS_PROP_TYPE:
29122911
propname = "type";
29132912
(void) nvlist_lookup_uint32(lnvl, propname, &lv32);
29142913
(void) nvlist_lookup_uint32(rnvl, propname, &rv32);
2915-
if (rv32 != lv32)
2916-
rc = (rv32 < lv32) ? 1 : -1;
2914+
rc = TREE_CMP(lv32, rv32);
29172915
break;
29182916
case ZFS_PROP_NAME:
29192917
propname = "name";
@@ -2923,16 +2921,15 @@ us_compare(const void *larg, const void *rarg)
29232921
&lv64);
29242922
(void) nvlist_lookup_uint64(rnvl, propname,
29252923
&rv64);
2926-
if (rv64 != lv64)
2927-
rc = (rv64 < lv64) ? 1 : -1;
2924+
rc = TREE_CMP(lv64, rv64);
29282925
} else {
29292926
if ((nvlist_lookup_string(lnvl, propname,
29302927
&lvstr) == ENOENT) ||
29312928
(nvlist_lookup_string(rnvl, propname,
29322929
&rvstr) == ENOENT)) {
29332930
goto compare_nums;
29342931
}
2935-
rc = strcmp(lvstr, rvstr);
2932+
rc = TREE_ISIGN(strcmp(lvstr, rvstr));
29362933
}
29372934
break;
29382935
case ZFS_PROP_USED:
@@ -2945,19 +2942,17 @@ us_compare(const void *larg, const void *rarg)
29452942
propname = "quota";
29462943
(void) nvlist_lookup_uint64(lnvl, propname, &lv64);
29472944
(void) nvlist_lookup_uint64(rnvl, propname, &rv64);
2948-
if (rv64 != lv64)
2949-
rc = (rv64 < lv64) ? 1 : -1;
2945+
rc = TREE_CMP(lv64, rv64);
29502946
break;
29512947

29522948
default:
29532949
break;
29542950
}
29552951

29562952
if (rc != 0) {
2957-
if (rc < 0)
2958-
return (reverse ? 1 : -1);
2959-
else
2960-
return (reverse ? -1 : 1);
2953+
if (sortcol->sc_reverse)
2954+
return (-rc);
2955+
return (rc);
29612956
}
29622957
}
29632958

@@ -2969,7 +2964,7 @@ us_compare(const void *larg, const void *rarg)
29692964
if (nvlist_lookup_boolean_value(lnvl, "smbentity", &lvb) == 0 &&
29702965
nvlist_lookup_boolean_value(rnvl, "smbentity", &rvb) == 0 &&
29712966
lvb != rvb)
2972-
return (lvb < rvb ? -1 : 1);
2967+
return (TREE_CMP(lvb, rvb));
29732968

29742969
return (0);
29752970
}
@@ -5497,11 +5492,11 @@ who_perm_compare(const void *larg, const void *rarg)
54975492
zfs_deleg_who_type_t rtype = r->who_perm.who_type;
54985493
int lweight = who_type2weight(ltype);
54995494
int rweight = who_type2weight(rtype);
5500-
int res = lweight - rweight;
5495+
int res = TREE_CMP(lweight, rweight);
55015496
if (res == 0)
5502-
res = strncmp(l->who_perm.who_name, r->who_perm.who_name,
5503-
ZFS_MAX_DELEG_NAME-1);
5504-
return (TREE_ISIGN(res));
5497+
res = TREE_ISIGN(strncmp(l->who_perm.who_name,
5498+
r->who_perm.who_name, ZFS_MAX_DELEG_NAME-1));
5499+
return (res);
55055500
}
55065501

55075502
static int

lib/libzdb/libzdb.c

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -68,35 +68,23 @@ livelist_compare(const void *larg, const void *rarg)
6868
{
6969
const blkptr_t *l = larg;
7070
const blkptr_t *r = rarg;
71+
int cmp = 0;
7172

7273
/* Sort them according to dva[0] */
73-
uint64_t l_dva0_vdev, r_dva0_vdev;
74-
l_dva0_vdev = DVA_GET_VDEV(&l->blk_dva[0]);
75-
r_dva0_vdev = DVA_GET_VDEV(&r->blk_dva[0]);
76-
if (l_dva0_vdev < r_dva0_vdev)
77-
return (-1);
78-
else if (l_dva0_vdev > r_dva0_vdev)
79-
return (+1);
74+
cmp = TREE_CMP(DVA_GET_VDEV(&l->blk_dva[0]),
75+
DVA_GET_VDEV(&r->blk_dva[0]));
76+
if (cmp != 0)
77+
return (cmp);
8078

8179
/* if vdevs are equal, sort by offsets. */
82-
uint64_t l_dva0_offset;
83-
uint64_t r_dva0_offset;
84-
l_dva0_offset = DVA_GET_OFFSET(&l->blk_dva[0]);
85-
r_dva0_offset = DVA_GET_OFFSET(&r->blk_dva[0]);
86-
if (l_dva0_offset < r_dva0_offset) {
87-
return (-1);
88-
} else if (l_dva0_offset > r_dva0_offset) {
89-
return (+1);
90-
}
80+
cmp = TREE_CMP(DVA_GET_OFFSET(&l->blk_dva[0]),
81+
DVA_GET_OFFSET(&r->blk_dva[0]));
82+
if (cmp != 0)
83+
return (cmp);
9184

9285
/*
9386
* Since we're storing blkptrs without cancelling FREE/ALLOC pairs,
9487
* it's possible the offsets are equal. In that case, sort by txg
9588
*/
96-
if (BP_GET_BIRTH(l) < BP_GET_BIRTH(r)) {
97-
return (-1);
98-
} else if (BP_GET_BIRTH(l) > BP_GET_BIRTH(r)) {
99-
return (+1);
100-
}
101-
return (0);
89+
return (TREE_CMP(BP_GET_BIRTH(l), BP_GET_BIRTH(r)));
10290
}

module/icp/core/kcf_mech_tabs.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,8 @@ static int
9090
kcf_mech_hash_compar(const void *lhs, const void *rhs)
9191
{
9292
const kcf_mech_entry_t *l = lhs, *r = rhs;
93-
int cmp = strncmp(l->me_name, r->me_name, CRYPTO_MAX_MECH_NAME);
94-
return ((0 < cmp) - (cmp < 0));
93+
return (TREE_ISIGN(strncmp(l->me_name, r->me_name,
94+
CRYPTO_MAX_MECH_NAME)));
9595
}
9696

9797
void

module/os/freebsd/spl/acl_common.c

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1213,15 +1213,7 @@ static int
12131213
acevals_compare(const void *va, const void *vb)
12141214
{
12151215
const acevals_t *a = va, *b = vb;
1216-
1217-
if (a->key == b->key)
1218-
return (0);
1219-
1220-
if (a->key > b->key)
1221-
return (1);
1222-
1223-
else
1224-
return (-1);
1216+
return (TREE_CMP(a->key, b->key));
12251217
}
12261218

12271219
/*

module/os/linux/zfs/zfs_ctldir.c

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -251,16 +251,7 @@ snapentry_compare_by_name(const void *a, const void *b)
251251
{
252252
const zfs_snapentry_t *se_a = a;
253253
const zfs_snapentry_t *se_b = b;
254-
int ret;
255-
256-
ret = strcmp(se_a->se_name, se_b->se_name);
257-
258-
if (ret < 0)
259-
return (-1);
260-
else if (ret > 0)
261-
return (1);
262-
else
263-
return (0);
254+
return (TREE_ISIGN(strcmp(se_a->se_name, se_b->se_name)));
264255
}
265256

266257
/*
@@ -272,15 +263,10 @@ snapentry_compare_by_objsetid(const void *a, const void *b)
272263
const zfs_snapentry_t *se_a = a;
273264
const zfs_snapentry_t *se_b = b;
274265

275-
if (se_a->se_spa != se_b->se_spa)
276-
return ((ulong_t)se_a->se_spa < (ulong_t)se_b->se_spa ? -1 : 1);
277-
278-
if (se_a->se_objsetid < se_b->se_objsetid)
279-
return (-1);
280-
else if (se_a->se_objsetid > se_b->se_objsetid)
281-
return (1);
282-
else
283-
return (0);
266+
int cmp = TREE_PCMP(se_a->se_spa, se_b->se_spa);
267+
if (cmp != 0)
268+
return (cmp);
269+
return (TREE_CMP(se_a->se_objsetid, se_b->se_objsetid));
284270
}
285271

286272
/*

module/zfs/dmu_redact.c

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -176,11 +176,7 @@ objnode_compare(const void *o1, const void *o2)
176176
{
177177
const struct objnode *obj1 = o1;
178178
const struct objnode *obj2 = o2;
179-
if (obj1->obj < obj2->obj)
180-
return (-1);
181-
if (obj1->obj > obj2->obj)
182-
return (1);
183-
return (0);
179+
return (TREE_CMP(obj1->obj, obj2->obj));
184180
}
185181

186182

@@ -425,11 +421,11 @@ redact_node_compare_start(const void *arg1, const void *arg2)
425421
if (rr2->eos_marker)
426422
return (-1);
427423

428-
int cmp = redact_range_compare(rr1->start_object, rr1->start_blkid,
429-
rr1->datablksz, rr2->start_object, rr2->start_blkid,
430-
rr2->datablksz);
424+
int cmp = TREE_ISIGN(redact_range_compare(
425+
rr1->start_object, rr1->start_blkid, rr1->datablksz,
426+
rr2->start_object, rr2->start_blkid, rr2->datablksz));
431427
if (cmp == 0)
432-
cmp = (rn1->thread_num < rn2->thread_num ? -1 : 1);
428+
cmp = TREE_CMP(rn1->thread_num, rn2->thread_num);
433429
return (cmp);
434430
}
435431

@@ -451,11 +447,11 @@ redact_node_compare_end(const void *arg1, const void *arg2)
451447
if (srr2->eos_marker)
452448
return (-1);
453449

454-
int cmp = redact_range_compare(srr1->end_object, srr1->end_blkid,
455-
srr1->datablksz, srr2->end_object, srr2->end_blkid,
456-
srr2->datablksz);
450+
int cmp = TREE_ISIGN(redact_range_compare(
451+
srr1->end_object, srr1->end_blkid, srr1->datablksz,
452+
srr2->end_object, srr2->end_blkid, srr2->datablksz));
457453
if (cmp == 0)
458-
cmp = (rn1->thread_num < rn2->thread_num ? -1 : 1);
454+
cmp = TREE_CMP(rn1->thread_num, rn2->thread_num);
459455
return (cmp);
460456
}
461457

module/zfs/dsl_bookmark.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -837,8 +837,7 @@ dsl_bookmark_compare(const void *l, const void *r)
837837
(rdbn->dbn_phys.zbm_flags & ZBM_FLAG_HAS_FBN));
838838
if (likely(cmp))
839839
return (cmp);
840-
cmp = strcmp(ldbn->dbn_name, rdbn->dbn_name);
841-
return (TREE_ISIGN(cmp));
840+
return (TREE_ISIGN(strcmp(ldbn->dbn_name, rdbn->dbn_name)));
842841
}
843842

844843
/*

0 commit comments

Comments
 (0)