Cleanup: Remove constant comparisons reported by CodeQL

CodeQL's cpp/constant-comparison query from its security-and-extended
query set reported 4 instances where we have comparions that always
evaluate the same way.

In `draid_config_by_type()`, we have an early `if (nparity == 0)` check
that returns `EINVAL`, making a later `if (nparity == 0 || nparity >
VDEV_DRAID_MAXPARITY)` partially redundant. The later check prints an
error message when parity is 0, but the early check does not. This is
not useful feedback, so we move the later check to the place where the
early check runs to replace the early check.

In `perform_thread_merge()`, we return when `num_threads == 0`. After
that block, we do `if (num_threads > 0) {`, which will always be true.
We remove the `if` statement.

In `sa_modify_attrs()`, we have a loop condition that is `k != 2`, but
at the end of the loop, we have `if (k == 0 && hdl->sa_spill)` followed
by an else that does a break. The result is that k != 2 will never be
evaluated when it is false. We drop the comparison.

In `zap_leaf_array_read()`, we have a for loop condition that is `i <
ZAP_LEAF_ARRAY_BYTES && len > 0`. However, that loop itself is in a loop
that is `while (len > 0)` and while the value of len is decremented
inside the loop, when `len == 0`, it will return, such that `len > 0`
inside the loop condition will always be true. We drop that part of the
condition.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14575
This commit is contained in:
Richard Yao 2023-03-04 17:57:01 -05:00 committed by Brian Behlendorf
parent 5dd0f019cd
commit 17443e0b20
4 changed files with 10 additions and 15 deletions

View File

@ -1329,8 +1329,13 @@ draid_config_by_type(nvlist_t *nv, const char *type, uint64_t children)
return (EINVAL); return (EINVAL);
nparity = (uint64_t)get_parity(type); nparity = (uint64_t)get_parity(type);
if (nparity == 0) if (nparity == 0 || nparity > VDEV_DRAID_MAXPARITY) {
fprintf(stderr,
gettext("invalid dRAID parity level %llu; must be "
"between 1 and %d\n"), (u_longlong_t)nparity,
VDEV_DRAID_MAXPARITY);
return (EINVAL); return (EINVAL);
}
char *p = (char *)type; char *p = (char *)type;
while ((p = strchr(p, ':')) != NULL) { while ((p = strchr(p, ':')) != NULL) {
@ -1401,14 +1406,6 @@ draid_config_by_type(nvlist_t *nv, const char *type, uint64_t children)
return (EINVAL); return (EINVAL);
} }
if (nparity == 0 || nparity > VDEV_DRAID_MAXPARITY) {
fprintf(stderr,
gettext("invalid dRAID parity level %llu; must be "
"between 1 and %d\n"), (u_longlong_t)nparity,
VDEV_DRAID_MAXPARITY);
return (EINVAL);
}
/* /*
* Verify the requested number of spares can be satisfied. * Verify the requested number of spares can be satisfied.
* An arbitrary limit of 100 distributed spares is applied. * An arbitrary limit of 100 distributed spares is applied.

View File

@ -746,10 +746,8 @@ perform_thread_merge(bqueue_t *q, uint32_t num_threads,
bqueue_enqueue(q, record, sizeof (*record)); bqueue_enqueue(q, record, sizeof (*record));
return (0); return (0);
} }
if (num_threads > 0) { redact_nodes = kmem_zalloc(num_threads *
redact_nodes = kmem_zalloc(num_threads * sizeof (*redact_nodes), KM_SLEEP);
sizeof (*redact_nodes), KM_SLEEP);
}
avl_create(&start_tree, redact_node_compare_start, avl_create(&start_tree, redact_node_compare_start,
sizeof (struct redact_node), sizeof (struct redact_node),

View File

@ -1918,7 +1918,7 @@ sa_modify_attrs(sa_handle_t *hdl, sa_attr_type_t newattr,
count = bonus_attr_count; count = bonus_attr_count;
hdr = SA_GET_HDR(hdl, SA_BONUS); hdr = SA_GET_HDR(hdl, SA_BONUS);
idx_tab = SA_IDX_TAB_GET(hdl, SA_BONUS); idx_tab = SA_IDX_TAB_GET(hdl, SA_BONUS);
for (; k != 2; k++) { for (; ; k++) {
/* /*
* Iterate over each attribute in layout. Fetch the * Iterate over each attribute in layout. Fetch the
* size of variable-length attributes needing rewrite * size of variable-length attributes needing rewrite

View File

@ -315,7 +315,7 @@ zap_leaf_array_read(zap_leaf_t *l, uint16_t chunk,
struct zap_leaf_array *la = &ZAP_LEAF_CHUNK(l, chunk).l_array; struct zap_leaf_array *la = &ZAP_LEAF_CHUNK(l, chunk).l_array;
ASSERT3U(chunk, <, ZAP_LEAF_NUMCHUNKS(l)); ASSERT3U(chunk, <, ZAP_LEAF_NUMCHUNKS(l));
for (int i = 0; i < ZAP_LEAF_ARRAY_BYTES && len > 0; i++) { for (int i = 0; i < ZAP_LEAF_ARRAY_BYTES; i++) {
value = (value << 8) | la->la_array[i]; value = (value << 8) | la->la_array[i];
byten++; byten++;
if (byten == array_int_len) { if (byten == array_int_len) {