Skip to content

Commit 6a42939

Browse files
authored
Cleanup: Address Clang's static analyzer's unused code complaints
These were categorized as the following: * Dead assignment 23 * Dead increment 4 * Dead initialization 6 * Dead nested assignment 18 Most of these are harmless, but since actual issues can hide among them, we correct them. That said, there were a few return values that were being ignored that appeared to merit some correction: * `destroy_callback()` in `cmd/zfs/zfs_main.c` ignored the error from `destroy_batched()`. We handle it by returning -1 if there is an error. * `zfs_do_upgrade()` in `cmd/zfs/zfs_main.c` ignored the error from `zfs_for_each()`. We handle it by doing a binary OR of the error value from the subsequent `zfs_for_each()` call to the existing value. This is how errors are mostly handled inside `zfs_for_each()`. The error value here is passed to exit from the zfs command, so doing a binary or on it is better than what we did previously. * `get_zap_prop()` in `module/zfs/zcp_get.c` ignored the error from `dsl_prop_get_ds()` when the property is not of type string. We return an error when it does. There is a small concern that the `zfs_get_temporary_prop()` call would handle things, but in the case that it does not, we would be pushing an uninitialized numval onto the lua stack. It is expected that `dsl_prop_get_ds()` will succeed anytime that `zfs_get_temporary_prop()` does, so that not giving it a chance to fix things is not a problem. * `draid_merge_impl()` in `tests/zfs-tests/cmd/draid.c` used `nvlist_add_nvlist()` twice in ways in which errors are expected to be impossible, so we switch to `fnvlist_add_nvlist()`. A few notable ones did not merit use of the return value, so we suppressed it with `(void)`: * `write_free_diffs()` in `lib/libzfs/libzfs_diff.c` ignored the error value from `describe_free()`. A look through the commit history revealed that this was intentional. * `arc_evict_hdr()` in `module/zfs/arc.c` did not need to use the returned handle from `arc_hdr_realloc()` because it is already referenced in lists. * `spa_vdev_detach()` in `module/zfs/spa.c` has a comment explicitly saying not to use the error from `vdev_label_init()` because whatever causes the error could be the reason why a detach is being done. Unfortunately, I am not presently able to analyze the kernel modules with Clang's static analyzer, so I could have missed some cases of this. In cases where reports were present in code that is duplicated between Linux and FreeBSD, I made a conscious effort to fix the FreeBSD version too. After this commit is merged, regressions like dee8934 should become extremely obvious with Clang's static analyzer since a regression would appear in the results as the only instance of unused code. That assumes that Coverity does not catch the issue first. My local branch with fixes from all of my outstanding non-draft pull requests shows 118 reports from Clang's static anlayzer after this patch. That is down by 51 from 169. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Cedric Berger <cedric@precidata.com> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Closes #13986
1 parent 19516b6 commit 6a42939

File tree

32 files changed

+45
-75
lines changed

32 files changed

+45
-75
lines changed

cmd/raidz_test/raidz_test.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,6 @@ static void process_options(int argc, char **argv)
146146
memcpy(o, &rto_opts_defaults, sizeof (*o));
147147

148148
while ((opt = getopt(argc, argv, "TDBSvha:er:o:d:s:t:")) != -1) {
149-
value = 0;
150-
151149
switch (opt) {
152150
case 'a':
153151
value = strtoull(optarg, NULL, 0);

cmd/zfs/zfs_main.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1453,8 +1453,13 @@ destroy_callback(zfs_handle_t *zhp, void *data)
14531453
if (zfs_get_type(zhp) == ZFS_TYPE_SNAPSHOT) {
14541454
cb->cb_snap_count++;
14551455
fnvlist_add_boolean(cb->cb_batchedsnaps, name);
1456-
if (cb->cb_snap_count % 10 == 0 && cb->cb_defer_destroy)
1456+
if (cb->cb_snap_count % 10 == 0 && cb->cb_defer_destroy) {
14571457
error = destroy_batched(cb);
1458+
if (error != 0) {
1459+
zfs_close(zhp);
1460+
return (-1);
1461+
}
1462+
}
14581463
} else {
14591464
error = destroy_batched(cb);
14601465
if (error != 0 ||
@@ -2576,7 +2581,7 @@ zfs_do_upgrade(int argc, char **argv)
25762581
cb.cb_foundone = B_FALSE;
25772582
cb.cb_newer = B_TRUE;
25782583

2579-
ret = zfs_for_each(0, NULL, flags, ZFS_TYPE_FILESYSTEM,
2584+
ret |= zfs_for_each(0, NULL, flags, ZFS_TYPE_FILESYSTEM,
25802585
NULL, NULL, 0, upgrade_list_callback, &cb);
25812586

25822587
if (!cb.cb_foundone && !found) {

lib/libefi/rdwr_efi.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,6 @@ efi_alloc_and_read(int fd, struct dk_gpt **vtoc)
423423
void *tmp;
424424
length = (int) sizeof (struct dk_gpt) +
425425
(int) sizeof (struct dk_part) * (vptr->efi_nparts - 1);
426-
nparts = vptr->efi_nparts;
427426
if ((tmp = realloc(vptr, length)) == NULL) {
428427
/* cppcheck-suppress doubleFree */
429428
free(vptr);
@@ -565,10 +564,9 @@ int
565564
efi_rescan(int fd)
566565
{
567566
int retry = 10;
568-
int error;
569567

570568
/* Notify the kernel a devices partition table has been updated */
571-
while ((error = ioctl(fd, BLKRRPART)) != 0) {
569+
while (ioctl(fd, BLKRRPART) != 0) {
572570
if ((--retry == 0) || (errno != EBUSY)) {
573571
(void) fprintf(stderr, "the kernel failed to rescan "
574572
"the partition table: %d\n", errno);

lib/libzfs/libzfs_dataset.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2006,7 +2006,7 @@ zfs_prop_inherit(zfs_handle_t *zhp, const char *propname, boolean_t received)
20062006
if ((ret = changelist_prefix(cl)) != 0)
20072007
goto error;
20082008

2009-
if ((ret = zfs_ioctl(zhp->zfs_hdl, ZFS_IOC_INHERIT_PROP, &zc)) != 0) {
2009+
if (zfs_ioctl(zhp->zfs_hdl, ZFS_IOC_INHERIT_PROP, &zc) != 0) {
20102010
changelist_free(cl);
20112011
return (zfs_standard_error(hdl, errno, errbuf));
20122012
} else {

lib/libzfs/libzfs_diff.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ write_free_diffs(FILE *fp, differ_info_t *di, dmu_diff_record_t *dr)
377377
if (zc.zc_obj > dr->ddr_last) {
378378
break;
379379
}
380-
err = describe_free(fp, di, zc.zc_obj, fobjname,
380+
(void) describe_free(fp, di, zc.zc_obj, fobjname,
381381
MAXPATHLEN);
382382
} else if (errno == ESRCH) {
383383
break;

lib/libzfs/libzfs_pool.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2214,7 +2214,6 @@ zpool_import_props(libzfs_handle_t *hdl, nvlist_t *config, const char *newname,
22142214
((policy.zlp_rewind & ZPOOL_TRY_REWIND) != 0), nv);
22152215
}
22162216
nvlist_free(nv);
2217-
return (0);
22182217
}
22192218

22202219
return (ret);

lib/libzfs/libzfs_sendrecv.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2117,9 +2117,9 @@ send_prelim_records(zfs_handle_t *zhp, const char *from, int fd,
21172117
fnvlist_add_boolean(hdrnv, "raw");
21182118
}
21192119

2120-
if ((err = gather_nvlist(zhp->zfs_hdl, tofs,
2120+
if (gather_nvlist(zhp->zfs_hdl, tofs,
21212121
from, tosnap, recursive, raw, doall, replicate, skipmissing,
2122-
verbose, backup, holds, props, &fss, fsavlp)) != 0) {
2122+
verbose, backup, holds, props, &fss, fsavlp) != 0) {
21232123
return (zfs_error(zhp->zfs_hdl, EZFS_BADBACKUP,
21242124
errbuf));
21252125
}

lib/libzfs/libzfs_util.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1249,7 +1249,7 @@ zcmd_read_dst_nvlist(libzfs_handle_t *hdl, zfs_cmd_t *zc, nvlist_t **nvlp)
12491249
static void
12501250
zprop_print_headers(zprop_get_cbdata_t *cbp, zfs_type_t type)
12511251
{
1252-
zprop_list_t *pl = cbp->cb_proplist;
1252+
zprop_list_t *pl;
12531253
int i;
12541254
char *title;
12551255
size_t len;

lib/libzutil/os/linux/zutil_device_path_os.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,6 @@ dm_get_underlying_path(const char *dm_name)
428428
char *tmp = NULL;
429429
char *path = NULL;
430430
char *dev_str;
431-
int size;
432431
char *first_path = NULL;
433432
char *enclosure_path;
434433

@@ -450,7 +449,7 @@ dm_get_underlying_path(const char *dm_name)
450449
else
451450
dev_str = tmp;
452451

453-
if ((size = asprintf(&tmp, "/sys/block/%s/slaves/", dev_str)) == -1) {
452+
if (asprintf(&tmp, "/sys/block/%s/slaves/", dev_str) == -1) {
454453
tmp = NULL;
455454
goto end;
456455
}
@@ -479,8 +478,7 @@ dm_get_underlying_path(const char *dm_name)
479478
if (!enclosure_path)
480479
continue;
481480

482-
if ((size = asprintf(
483-
&path, "/dev/%s", ep->d_name)) == -1)
481+
if (asprintf(&path, "/dev/%s", ep->d_name) == -1)
484482
path = NULL;
485483
free(enclosure_path);
486484
break;
@@ -499,7 +497,7 @@ dm_get_underlying_path(const char *dm_name)
499497
* enclosure devices. Throw up out hands and return the first
500498
* underlying path.
501499
*/
502-
if ((size = asprintf(&path, "/dev/%s", first_path)) == -1)
500+
if (asprintf(&path, "/dev/%s", first_path) == -1)
503501
path = NULL;
504502
}
505503

module/icp/algs/blake3/blake3.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -189,9 +189,7 @@ static void chunk_state_update(const blake3_ops_t *ops,
189189
input_len -= BLAKE3_BLOCK_LEN;
190190
}
191191

192-
size_t take = chunk_state_fill_buf(ctx, input, input_len);
193-
input += take;
194-
input_len -= take;
192+
chunk_state_fill_buf(ctx, input, input_len);
195193
}
196194

197195
static output_t chunk_state_output(const blake3_chunk_state_t *ctx)

0 commit comments

Comments
 (0)