Skip to content

Commit

Permalink
Cleanup: Address Clang's static analyzer's unused code complaints
Browse files Browse the repository at this point in the history
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()`.

 * `get_zap_prop()` in `module/zfs/zcp_get.c` ignored the
   `dsl_prop_get_ds()` when the property is not of type string. We
   return an error if 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.

Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
  • Loading branch information
ryao committed Oct 3, 2022
1 parent 67395be commit 3b8ebc9
Show file tree
Hide file tree
Showing 32 changed files with 45 additions and 73 deletions.
2 changes: 0 additions & 2 deletions cmd/raidz_test/raidz_test.c
Expand Up @@ -146,8 +146,6 @@ static void process_options(int argc, char **argv)
memcpy(o, &rto_opts_defaults, sizeof (*o));

while ((opt = getopt(argc, argv, "TDBSvha:er:o:d:s:t:")) != -1) {
value = 0;

switch (opt) {
case 'a':
value = strtoull(optarg, NULL, 0);
Expand Down
9 changes: 7 additions & 2 deletions cmd/zfs/zfs_main.c
Expand Up @@ -1453,8 +1453,13 @@ destroy_callback(zfs_handle_t *zhp, void *data)
if (zfs_get_type(zhp) == ZFS_TYPE_SNAPSHOT) {
cb->cb_snap_count++;
fnvlist_add_boolean(cb->cb_batchedsnaps, name);
if (cb->cb_snap_count % 10 == 0 && cb->cb_defer_destroy)
if (cb->cb_snap_count % 10 == 0 && cb->cb_defer_destroy) {
error = destroy_batched(cb);
if (error != 0) {
zfs_close(zhp);
return (-1);
}
}
} else {
error = destroy_batched(cb);
if (error != 0 ||
Expand Down Expand Up @@ -2576,7 +2581,7 @@ zfs_do_upgrade(int argc, char **argv)
cb.cb_foundone = B_FALSE;
cb.cb_newer = B_TRUE;

ret = zfs_for_each(0, NULL, flags, ZFS_TYPE_FILESYSTEM,
ret |= zfs_for_each(0, NULL, flags, ZFS_TYPE_FILESYSTEM,
NULL, NULL, 0, upgrade_list_callback, &cb);

if (!cb.cb_foundone && !found) {
Expand Down
4 changes: 1 addition & 3 deletions lib/libefi/rdwr_efi.c
Expand Up @@ -423,7 +423,6 @@ efi_alloc_and_read(int fd, struct dk_gpt **vtoc)
void *tmp;
length = (int) sizeof (struct dk_gpt) +
(int) sizeof (struct dk_part) * (vptr->efi_nparts - 1);
nparts = vptr->efi_nparts;
if ((tmp = realloc(vptr, length)) == NULL) {
/* cppcheck-suppress doubleFree */
free(vptr);
Expand Down Expand Up @@ -565,10 +564,9 @@ int
efi_rescan(int fd)
{
int retry = 10;
int error;

/* Notify the kernel a devices partition table has been updated */
while ((error = ioctl(fd, BLKRRPART)) != 0) {
while ((ioctl(fd, BLKRRPART)) != 0) {
if ((--retry == 0) || (errno != EBUSY)) {
(void) fprintf(stderr, "the kernel failed to rescan "
"the partition table: %d\n", errno);
Expand Down
2 changes: 1 addition & 1 deletion lib/libzfs/libzfs_dataset.c
Expand Up @@ -2006,7 +2006,7 @@ zfs_prop_inherit(zfs_handle_t *zhp, const char *propname, boolean_t received)
if ((ret = changelist_prefix(cl)) != 0)
goto error;

if ((ret = zfs_ioctl(zhp->zfs_hdl, ZFS_IOC_INHERIT_PROP, &zc)) != 0) {
if (zfs_ioctl(zhp->zfs_hdl, ZFS_IOC_INHERIT_PROP, &zc) != 0) {
changelist_free(cl);
return (zfs_standard_error(hdl, errno, errbuf));
} else {
Expand Down
2 changes: 1 addition & 1 deletion lib/libzfs/libzfs_diff.c
Expand Up @@ -377,7 +377,7 @@ write_free_diffs(FILE *fp, differ_info_t *di, dmu_diff_record_t *dr)
if (zc.zc_obj > dr->ddr_last) {
break;
}
err = describe_free(fp, di, zc.zc_obj, fobjname,
(void) describe_free(fp, di, zc.zc_obj, fobjname,
MAXPATHLEN);
} else if (errno == ESRCH) {
break;
Expand Down
1 change: 0 additions & 1 deletion lib/libzfs/libzfs_pool.c
Expand Up @@ -2214,7 +2214,6 @@ zpool_import_props(libzfs_handle_t *hdl, nvlist_t *config, const char *newname,
((policy.zlp_rewind & ZPOOL_TRY_REWIND) != 0), nv);
}
nvlist_free(nv);
return (0);
}

return (ret);
Expand Down
4 changes: 2 additions & 2 deletions lib/libzfs/libzfs_sendrecv.c
Expand Up @@ -2117,9 +2117,9 @@ send_prelim_records(zfs_handle_t *zhp, const char *from, int fd,
fnvlist_add_boolean(hdrnv, "raw");
}

if ((err = gather_nvlist(zhp->zfs_hdl, tofs,
if (gather_nvlist(zhp->zfs_hdl, tofs,
from, tosnap, recursive, raw, doall, replicate, skipmissing,
verbose, backup, holds, props, &fss, fsavlp)) != 0) {
verbose, backup, holds, props, &fss, fsavlp) != 0) {
return (zfs_error(zhp->zfs_hdl, EZFS_BADBACKUP,
errbuf));
}
Expand Down
2 changes: 1 addition & 1 deletion lib/libzfs/libzfs_util.c
Expand Up @@ -1249,7 +1249,7 @@ zcmd_read_dst_nvlist(libzfs_handle_t *hdl, zfs_cmd_t *zc, nvlist_t **nvlp)
static void
zprop_print_headers(zprop_get_cbdata_t *cbp, zfs_type_t type)
{
zprop_list_t *pl = cbp->cb_proplist;
zprop_list_t *pl;
int i;
char *title;
size_t len;
Expand Down
8 changes: 3 additions & 5 deletions lib/libzutil/os/linux/zutil_device_path_os.c
Expand Up @@ -428,7 +428,6 @@ dm_get_underlying_path(const char *dm_name)
char *tmp = NULL;
char *path = NULL;
char *dev_str;
int size;
char *first_path = NULL;
char *enclosure_path;

Expand All @@ -450,7 +449,7 @@ dm_get_underlying_path(const char *dm_name)
else
dev_str = tmp;

if ((size = asprintf(&tmp, "/sys/block/%s/slaves/", dev_str)) == -1) {
if ((asprintf(&tmp, "/sys/block/%s/slaves/", dev_str)) == -1) {
tmp = NULL;
goto end;
}
Expand Down Expand Up @@ -479,8 +478,7 @@ dm_get_underlying_path(const char *dm_name)
if (!enclosure_path)
continue;

if ((size = asprintf(
&path, "/dev/%s", ep->d_name)) == -1)
if ((asprintf(&path, "/dev/%s", ep->d_name)) == -1)
path = NULL;
free(enclosure_path);
break;
Expand All @@ -499,7 +497,7 @@ dm_get_underlying_path(const char *dm_name)
* enclosure devices. Throw up out hands and return the first
* underlying path.
*/
if ((size = asprintf(&path, "/dev/%s", first_path)) == -1)
if ((asprintf(&path, "/dev/%s", first_path)) == -1)
path = NULL;
}

Expand Down
4 changes: 1 addition & 3 deletions module/icp/algs/blake3/blake3.c
Expand Up @@ -189,9 +189,7 @@ static void chunk_state_update(const blake3_ops_t *ops,
input_len -= BLAKE3_BLOCK_LEN;
}

size_t take = chunk_state_fill_buf(ctx, input, input_len);
input += take;
input_len -= take;
chunk_state_fill_buf(ctx, input, input_len);
}

static output_t chunk_state_output(const blake3_chunk_state_t *ctx)
Expand Down
1 change: 0 additions & 1 deletion module/icp/algs/modes/ccm.c
Expand Up @@ -67,7 +67,6 @@ ccm_mode_encrypt_contiguous_blocks(ccm_ctx_t *ctx, char *data, size_t length,
return (CRYPTO_SUCCESS);
}

lastp = (uint8_t *)ctx->ccm_cb;
crypto_init_ptrs(out, &iov_or_mp, &offset);

mac_buf = (uint8_t *)ctx->ccm_mac_buf;
Expand Down
1 change: 0 additions & 1 deletion module/icp/algs/modes/ctr.c
Expand Up @@ -60,7 +60,6 @@ ctr_mode_contiguous_blocks(ctr_ctx_t *ctx, char *data, size_t length,
return (CRYPTO_SUCCESS);
}

lastp = (uint8_t *)ctx->ctr_cb;
crypto_init_ptrs(out, &iov_or_mp, &offset);

do {
Expand Down
1 change: 0 additions & 1 deletion module/icp/algs/modes/gcm.c
Expand Up @@ -118,7 +118,6 @@ gcm_mode_encrypt_contiguous_blocks(gcm_ctx_t *ctx, char *data, size_t length,
return (CRYPTO_SUCCESS);
}

lastp = (uint8_t *)ctx->gcm_cb;
crypto_init_ptrs(out, &iov_or_mp, &offset);

gops = gcm_impl_get_ops();
Expand Down
2 changes: 1 addition & 1 deletion module/lua/ldo.c
Expand Up @@ -452,7 +452,7 @@ int luaD_poscall (lua_State *L, StkId firstResult) {
}
res = ci->func; /* res == final position of 1st result */
wanted = ci->nresults;
L->ci = ci = ci->previous; /* back to caller */
L->ci = ci->previous; /* back to caller */
/* move results to correct place */
for (i = wanted; i != 0 && firstResult < L->top; i--)
setobjs2s(L, res++, firstResult++);
Expand Down
1 change: 0 additions & 1 deletion module/os/freebsd/zfs/zfs_znode.c
Expand Up @@ -1949,7 +1949,6 @@ zfs_obj_to_path_impl(objset_t *osp, uint64_t obj, sa_handle_t *hdl,
} else if (error != ENOENT) {
return (error);
}
error = 0;

for (;;) {
uint64_t pobj;
Expand Down
1 change: 0 additions & 1 deletion module/os/freebsd/zfs/zio_crypt.c
Expand Up @@ -1735,7 +1735,6 @@ zio_do_crypt_data(boolean_t encrypt, zio_crypt_key_t *key,
goto error;
if (locked) {
rw_exit(&key->zk_salt_lock);
locked = B_FALSE;
}

if (authbuf != NULL)
Expand Down
1 change: 0 additions & 1 deletion module/os/linux/zfs/zfs_znode.c
Expand Up @@ -2136,7 +2136,6 @@ zfs_obj_to_path_impl(objset_t *osp, uint64_t obj, sa_handle_t *hdl,
} else if (error != ENOENT) {
return (error);
}
error = 0;

for (;;) {
uint64_t pobj = 0;
Expand Down
1 change: 0 additions & 1 deletion module/os/linux/zfs/zio_crypt.c
Expand Up @@ -1968,7 +1968,6 @@ zio_do_crypt_data(boolean_t encrypt, zio_crypt_key_t *key,

if (locked) {
rw_exit(&key->zk_salt_lock);
locked = B_FALSE;
}

if (authbuf != NULL)
Expand Down
2 changes: 1 addition & 1 deletion module/zfs/arc.c
Expand Up @@ -3939,7 +3939,7 @@ arc_evict_hdr(arc_buf_hdr_t *hdr, kmutex_t *hash_lock, uint64_t *real_evicted)
* dropping from L1+L2 cached to L2-only,
* realloc to remove the L1 header.
*/
hdr = arc_hdr_realloc(hdr, hdr_full_cache,
(void) arc_hdr_realloc(hdr, hdr_full_cache,
hdr_l2only_cache);
*real_evicted += HDR_FULL_SIZE - HDR_L2ONLY_SIZE;
} else {
Expand Down
1 change: 0 additions & 1 deletion module/zfs/dbuf.c
Expand Up @@ -1549,7 +1549,6 @@ dbuf_read_impl(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags,
uint32_t aflags = ARC_FLAG_NOWAIT;
int err, zio_flags;

err = zio_flags = 0;
DB_DNODE_ENTER(db);
dn = DB_DNODE(db);
ASSERT(!zfs_refcount_is_zero(&db->db_holds));
Expand Down
1 change: 0 additions & 1 deletion module/zfs/dsl_bookmark.c
Expand Up @@ -229,7 +229,6 @@ dsl_bookmark_create_check_impl(dsl_pool_t *dp,
switch (error) {
case ESRCH:
/* happy path: new bmark doesn't exist, proceed after switch */
error = 0;
break;
case 0:
error = SET_ERROR(EEXIST);
Expand Down
3 changes: 2 additions & 1 deletion module/zfs/dsl_dataset.c
Expand Up @@ -3421,7 +3421,8 @@ dsl_dataset_promote_check(void *arg, dmu_tx_t *tx)
conflicting_snaps = B_TRUE;
} else if (err == ESRCH) {
err = 0;
} else if (err != 0) {
}
if (err != 0) {
goto out;
}
}
Expand Down
1 change: 0 additions & 1 deletion module/zfs/dsl_pool.c
Expand Up @@ -331,7 +331,6 @@ dsl_pool_open(dsl_pool_t *dp)
/*
* We might not have created the remap bpobj yet.
*/
err = 0;
} else {
goto out;
}
Expand Down
10 changes: 5 additions & 5 deletions module/zfs/mmp.c
Expand Up @@ -548,11 +548,11 @@ mmp_thread(void *arg)
uint32_t mmp_fail_intervals = MMP_FAIL_INTVS_OK(
zfs_multihost_fail_intervals);
hrtime_t mmp_fail_ns = mmp_fail_intervals * mmp_interval;
boolean_t last_spa_suspended = suspended;
boolean_t last_spa_multihost = multihost;
uint64_t last_mmp_interval = mmp_interval;
uint32_t last_mmp_fail_intervals = mmp_fail_intervals;
hrtime_t last_mmp_fail_ns = mmp_fail_ns;
boolean_t last_spa_suspended;
boolean_t last_spa_multihost;
uint64_t last_mmp_interval;
uint32_t last_mmp_fail_intervals;
hrtime_t last_mmp_fail_ns;
callb_cpr_t cpr;
int skip_wait = 0;

Expand Down
6 changes: 3 additions & 3 deletions module/zfs/spa.c
Expand Up @@ -6803,8 +6803,8 @@ spa_vdev_attach(spa_t *spa, uint64_t guid, nvlist_t *nvroot, int replacing,

pvd = oldvd->vdev_parent;

if ((error = spa_config_parse(spa, &newrootvd, nvroot, NULL, 0,
VDEV_ALLOC_ATTACH)) != 0)
if (spa_config_parse(spa, &newrootvd, nvroot, NULL, 0,
VDEV_ALLOC_ATTACH) != 0)
return (spa_vdev_exit(spa, NULL, txg, EINVAL));

if (newrootvd->vdev_children != 1)
Expand Down Expand Up @@ -7160,7 +7160,7 @@ spa_vdev_detach(spa_t *spa, uint64_t guid, uint64_t pguid, int replace_done)
* it may be that the unwritability of the disk is the reason
* it's being detached!
*/
error = vdev_label_init(vd, 0, VDEV_LABEL_REMOVE);
(void) vdev_label_init(vd, 0, VDEV_LABEL_REMOVE);

/*
* Remove vd from its parent and compact the parent's children.
Expand Down
1 change: 0 additions & 1 deletion module/zfs/vdev.c
Expand Up @@ -6078,7 +6078,6 @@ vdev_prop_get(vdev_t *vd, nvlist_t *innvl, nvlist_t *outnvl)
strval = NULL;
zprop_source_t src = ZPROP_SRC_DEFAULT;
propname = za.za_name;
prop = vdev_name_to_prop(propname);

switch (za.za_integer_length) {
case 8:
Expand Down
4 changes: 3 additions & 1 deletion module/zfs/zcp_get.c
Expand Up @@ -467,7 +467,8 @@ get_zap_prop(lua_State *state, dsl_dataset_t *ds, zfs_prop_t zfs_prop)
} else {
error = dsl_prop_get_ds(ds, prop_name, sizeof (numval),
1, &numval, setpoint);

if (error != 0)
goto out;
#ifdef _KERNEL
/* Fill in temporary value for prop, if applicable */
(void) zfs_get_temporary_prop(ds, zfs_prop, &numval, setpoint);
Expand All @@ -489,6 +490,7 @@ get_zap_prop(lua_State *state, dsl_dataset_t *ds, zfs_prop_t zfs_prop)
(void) lua_pushnumber(state, numval);
}
}
out:
kmem_free(strval, ZAP_MAXVALUELEN);
if (error == 0)
get_prop_src(state, setpoint, zfs_prop);
Expand Down

0 comments on commit 3b8ebc9

Please sign in to comment.