Skip to content

Commit

Permalink
Fix unchecked return values and unused return values
Browse files Browse the repository at this point in the history
Coverity complained about unchecked return values and unused values that
turned out to be unused return values.

Different approaches were used to handle the different cases of
unchecked return values:

* cmd/zdb/zdb.c: VERIFY0 was used in one place since the existing code
  had no error handling. An error message was printed in another to
  match the rest of the code.

* cmd/zed/agents/zfs_retire.c: We dismiss the return value with `(void)`
  because the value is expected to be potentially unset.

* cmd/zpool_influxdb/zpool_influxdb.c: We dismiss the return value with
  `(void)` because the values are expected to be potentially unset.

* cmd/ztest.c: VERIFY0 was used since we want failures if something goes
  wrong in ztest.

* module/zfs/dsl_dir.c: We dismiss the return value with `(void)`
  because there is no guarantee that the zap entry will always be there.
  For example, old pools imported readonly would not have it and we do
  not want to fail here because of that.

* module/zfs/zfs_fm.c: `fnvlist_add_*()` was used since the
  allocations sleep and thus can never fail.

* module/zfs/zvol.c: We dismiss the return value with `(void)` because
  we do not need it. This matches what is already done in the analogous
  `zfs_replay_write2()`.

* tests/zfs-tests/cmd/draid.c: We suppress one return value with
  `(void)` since the code handles errors already. The other return value
  is handled by switching to `fnvlist_lookup_uint8_array()`.

* tests/zfs-tests/cmd/file/file_fadvise.c: We add error handling.

* tests/zfs-tests/cmd/mmap_sync.c: We add error handling for munmap, but
  ignore failures on remove() with (void) since it is expected to be
  able to fail.

* tests/zfs-tests/cmd/mmapwrite.c: We add error handling.

As for unused return values, they were all in places where there was
error handling, so logic was added to handle the return values.

Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #13920
  • Loading branch information
ryao authored Sep 23, 2022
1 parent d25153d commit 2a493a4
Show file tree
Hide file tree
Showing 14 changed files with 51 additions and 28 deletions.
15 changes: 11 additions & 4 deletions cmd/zdb/zdb.c
Original file line number Diff line number Diff line change
Expand Up @@ -4418,9 +4418,14 @@ dump_l2arc_log_blocks(int fd, l2arc_dev_hdr_phys_t l2dhdr,
default:
abd = abd_alloc_for_io(asize, B_TRUE);
abd_copy_from_buf_off(abd, &this_lb, 0, asize);
zio_decompress_data(L2BLK_GET_COMPRESS(
if (zio_decompress_data(L2BLK_GET_COMPRESS(
(&lbps[0])->lbp_prop), abd, &this_lb,
asize, sizeof (this_lb), NULL);
asize, sizeof (this_lb), NULL) != 0) {
(void) printf("L2ARC block decompression "
"failed\n");
abd_free(abd);
goto out;
}
abd_free(abd);
break;
}
Expand Down Expand Up @@ -4455,7 +4460,7 @@ dump_l2arc_log_blocks(int fd, l2arc_dev_hdr_phys_t l2dhdr,
lbps[0] = lbps[1];
lbps[1] = this_lb.lb_prev_lbp;
}

out:
if (!dump_opt['q']) {
(void) printf("log_blk_count:\t %llu with valid cksum\n",
(u_longlong_t)rebuild->dh_lb_count);
Expand Down Expand Up @@ -7586,7 +7591,7 @@ dump_mos_leaks(spa_t *spa)
} else {
dmu_object_info_t doi;
const char *name;
dmu_object_info(mos, object, &doi);
VERIFY0(dmu_object_info(mos, object, &doi));
if (doi.doi_type & DMU_OT_NEWTYPE) {
dmu_object_byteswap_t bswap =
DMU_OT_BYTESWAP(doi.doi_type);
Expand Down Expand Up @@ -8698,6 +8703,8 @@ main(int argc, char **argv)
usage();
dump_opt['v'] = verbose;
error = dump_path(argv[0], argv[1], &object);
if (error != 0)
fatal("internal error: %s", strerror(error));
}

if (dump_opt['X'] || dump_opt['F'])
Expand Down
3 changes: 2 additions & 1 deletion cmd/zed/agents/zfs_retire.c
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,8 @@ zfs_retire_recv(fmd_hdl_t *hdl, fmd_event_t *ep, nvlist_t *nvl,

fmd_hdl_debug(hdl, "zfs_retire_recv: '%s'", class);

nvlist_lookup_uint64(nvl, FM_EREPORT_PAYLOAD_ZFS_VDEV_STATE, &state);
(void) nvlist_lookup_uint64(nvl, FM_EREPORT_PAYLOAD_ZFS_VDEV_STATE,
&state);

/*
* If this is a resource notifying us of device removal then simply
Expand Down
8 changes: 4 additions & 4 deletions cmd/zpool_influxdb/zpool_influxdb.c
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ get_vdev_name(nvlist_t *nvroot, const char *parent_name)
uint64_t vdev_id = 0;

char *vdev_type = (char *)"unknown";
nvlist_lookup_string(nvroot, ZPOOL_CONFIG_TYPE, &vdev_type);
(void) nvlist_lookup_string(nvroot, ZPOOL_CONFIG_TYPE, &vdev_type);

if (nvlist_lookup_uint64(
nvroot, ZPOOL_CONFIG_ID, &vdev_id) != 0)
Expand Down Expand Up @@ -302,9 +302,9 @@ get_vdev_desc(nvlist_t *nvroot, const char *parent_name)
char *vdev_type = (char *)"unknown";
uint64_t vdev_id = UINT64_MAX;
char *vdev_path = NULL;
nvlist_lookup_string(nvroot, ZPOOL_CONFIG_TYPE, &vdev_type);
nvlist_lookup_uint64(nvroot, ZPOOL_CONFIG_ID, &vdev_id);
nvlist_lookup_string(nvroot, ZPOOL_CONFIG_PATH, &vdev_path);
(void) nvlist_lookup_string(nvroot, ZPOOL_CONFIG_TYPE, &vdev_type);
(void) nvlist_lookup_uint64(nvroot, ZPOOL_CONFIG_ID, &vdev_id);
(void) nvlist_lookup_string(nvroot, ZPOOL_CONFIG_PATH, &vdev_path);

if (parent_name == NULL) {
s = escape_string(vdev_type);
Expand Down
2 changes: 1 addition & 1 deletion cmd/ztest.c
Original file line number Diff line number Diff line change
Expand Up @@ -2214,7 +2214,7 @@ ztest_replay_write(void *arg1, void *arg2, boolean_t byteswap)
dmu_write(os, lr->lr_foid, offset, length, data, tx);
} else {
memcpy(abuf->b_data, data, length);
dmu_assign_arcbuf_by_dbuf(db, offset, abuf, tx);
VERIFY0(dmu_assign_arcbuf_by_dbuf(db, offset, abuf, tx));
}

(void) ztest_log_write(zd, tx, lr);
Expand Down
5 changes: 3 additions & 2 deletions lib/libzfsbootenv/lzbe_device.c
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ lzbe_set_boot_device(const char *pool, lzbe_flags_t flag, const char *device)
/* version is mandatory */
fnvlist_add_uint64(nv, BOOTENV_VERSION, VB_NVLIST);

rv = 0;
/*
* If device name is empty, remove boot device configuration.
*/
Expand All @@ -95,8 +96,8 @@ lzbe_set_boot_device(const char *pool, lzbe_flags_t flag, const char *device)
rv = ENOMEM;
}
}

rv = zpool_set_bootenv(zphdl, nv);
if (rv == 0)
rv = zpool_set_bootenv(zphdl, nv);
if (rv != 0)
fprintf(stderr, "%s\n", libzfs_error_description(hdl));

Expand Down
2 changes: 2 additions & 0 deletions module/zfs/dmu_recv.c
Original file line number Diff line number Diff line change
Expand Up @@ -1874,6 +1874,8 @@ receive_object(struct receive_writer_arg *rwa, struct drr_object *drro,
if (err == 0) {
err = receive_handle_existing_object(rwa, drro, &doi, data,
&object_to_hold, &new_blksz);
if (err != 0)
return (err);
} else if (err == EEXIST) {
/*
* The object requested is currently an interior slot of a
Expand Down
5 changes: 4 additions & 1 deletion module/zfs/dsl_dataset.c
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,8 @@ dsl_dataset_hold_obj(dsl_pool_t *dp, uint64_t dsobj, const void *tag,
dsl_dataset_phys(ds)->ds_prev_snap_obj,
ds, &ds->ds_prev);
}
if (err != 0)
goto after_dsl_bookmark_fini;
err = dsl_bookmark_init_ds(ds);
} else {
if (zfs_flags & ZFS_DEBUG_SNAPNAMES)
Expand Down Expand Up @@ -688,11 +690,11 @@ dsl_dataset_hold_obj(dsl_pool_t *dp, uint64_t dsobj, const void *tag,
winner = dmu_buf_set_user_ie(dbuf, &ds->ds_dbu);

if (err != 0 || winner != NULL) {
bplist_destroy(&ds->ds_pending_deadlist);
dsl_deadlist_close(&ds->ds_deadlist);
if (dsl_deadlist_is_open(&ds->ds_remap_deadlist))
dsl_deadlist_close(&ds->ds_remap_deadlist);
dsl_bookmark_fini_ds(ds);
after_dsl_bookmark_fini:
if (ds->ds_prev)
dsl_dataset_rele(ds->ds_prev, ds);
dsl_dir_rele(ds->ds_dir, ds);
Expand All @@ -703,6 +705,7 @@ dsl_dataset_hold_obj(dsl_pool_t *dp, uint64_t dsobj, const void *tag,

list_destroy(&ds->ds_prop_cbs);
list_destroy(&ds->ds_sendstreams);
bplist_destroy(&ds->ds_pending_deadlist);
mutex_destroy(&ds->ds_lock);
mutex_destroy(&ds->ds_opening_lock);
mutex_destroy(&ds->ds_sendstream_lock);
Expand Down
2 changes: 1 addition & 1 deletion module/zfs/dsl_dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ dsl_dir_hold_obj(dsl_pool_t *dp, uint64_t ddobj,

if (dsl_dir_is_zapified(dd)) {
inode_timespec_t t = {0};
zap_lookup(dp->dp_meta_objset, ddobj,
(void) zap_lookup(dp->dp_meta_objset, ddobj,
DD_FIELD_SNAPSHOTS_CHANGED,
sizeof (uint64_t),
sizeof (inode_timespec_t) / sizeof (uint64_t),
Expand Down
12 changes: 6 additions & 6 deletions module/zfs/zfs_fm.c
Original file line number Diff line number Diff line change
Expand Up @@ -1389,17 +1389,17 @@ zfs_post_state_change(spa_t *spa, vdev_t *vd, uint64_t laststate)
aux = fm_nvlist_create(NULL);
if (vd && aux) {
if (vd->vdev_physpath) {
(void) nvlist_add_string(aux,
fnvlist_add_string(aux,
FM_EREPORT_PAYLOAD_ZFS_VDEV_PHYSPATH,
vd->vdev_physpath);
}
if (vd->vdev_enc_sysfs_path) {
(void) nvlist_add_string(aux,
fnvlist_add_string(aux,
FM_EREPORT_PAYLOAD_ZFS_VDEV_ENC_SYSFS_PATH,
vd->vdev_enc_sysfs_path);
}

(void) nvlist_add_uint64(aux,
fnvlist_add_uint64(aux,
FM_EREPORT_PAYLOAD_ZFS_VDEV_LASTSTATE, laststate);
}

Expand Down Expand Up @@ -1496,12 +1496,12 @@ zfs_ereport_zvol_post(const char *subclass, const char *name,
return;

aux = fm_nvlist_create(NULL);
nvlist_add_string(aux, FM_EREPORT_PAYLOAD_ZFS_DEVICE_NAME, dev_name);
nvlist_add_string(aux, FM_EREPORT_PAYLOAD_ZFS_RAW_DEVICE_NAME,
fnvlist_add_string(aux, FM_EREPORT_PAYLOAD_ZFS_DEVICE_NAME, dev_name);
fnvlist_add_string(aux, FM_EREPORT_PAYLOAD_ZFS_RAW_DEVICE_NAME,
raw_name);
r = strchr(name, '/');
if (r && r[1])
nvlist_add_string(aux, FM_EREPORT_PAYLOAD_ZFS_VOLUME, &r[1]);
fnvlist_add_string(aux, FM_EREPORT_PAYLOAD_ZFS_VOLUME, &r[1]);

zfs_post_common(spa, NULL, FM_RSRC_CLASS, subclass, aux);
fm_nvlist_destroy(aux, FM_NVA_FREE);
Expand Down
4 changes: 2 additions & 2 deletions module/zfs/zvol.c
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ zvol_replay_truncate(void *arg1, void *arg2, boolean_t byteswap)
if (error != 0) {
dmu_tx_abort(tx);
} else {
zil_replaying(zv->zv_zilog, tx);
(void) zil_replaying(zv->zv_zilog, tx);
dmu_tx_commit(tx);
error = dmu_free_long_range(zv->zv_objset, ZVOL_OBJ, offset,
length);
Expand Down Expand Up @@ -475,7 +475,7 @@ zvol_replay_write(void *arg1, void *arg2, boolean_t byteswap)
dmu_tx_abort(tx);
} else {
dmu_write(os, ZVOL_OBJ, offset, length, data, tx);
zil_replaying(zv->zv_zilog, tx);
(void) zil_replaying(zv->zv_zilog, tx);
dmu_tx_commit(tx);
}

Expand Down
4 changes: 2 additions & 2 deletions tests/zfs-tests/cmd/draid.c
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ read_map_key(const char *filename, const char *key, nvlist_t **cfg)
if (error != 0)
return (error);

nvlist_lookup_nvlist(allcfgs, key, &foundcfg);
(void) nvlist_lookup_nvlist(allcfgs, key, &foundcfg);
if (foundcfg != NULL) {
nvlist_dup(foundcfg, cfg, KM_SLEEP);
error = 0;
Expand Down Expand Up @@ -375,7 +375,7 @@ dump_map_nv(const char *key, nvlist_t *cfg, int verbose)
map.dm_checksum = fnvlist_lookup_uint64(cfg, MAP_CHECKSUM);
map.dm_children = fnvlist_lookup_uint64(cfg, MAP_CHILDREN);
map.dm_nperms = fnvlist_lookup_uint64(cfg, MAP_NPERMS);
nvlist_lookup_uint8_array(cfg, MAP_PERMS, &map.dm_perms, &c);
map.dm_perms = fnvlist_lookup_uint8_array(cfg, MAP_PERMS, &c);

dump_map(&map, key, (double)worst_ratio / 1000.0,
avg_ratio / 1000.0, verbose);
Expand Down
6 changes: 5 additions & 1 deletion tests/zfs-tests/cmd/file/file_fadvise.c
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,11 @@ main(int argc, char *argv[])
return (1);
}

posix_fadvise(fd, 0, 0, advise);
if (posix_fadvise(fd, 0, 0, advise) != 0) {
perror("posix_fadvise");
close(fd);
return (1);
}

close(fd);

Expand Down
5 changes: 3 additions & 2 deletions tests/zfs-tests/cmd/mmap_sync.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
static void
cleanup(char *file)
{
remove(file);
(void) remove(file);
}

int
Expand Down Expand Up @@ -125,7 +125,8 @@ main(int argc, char *argv[])
elapsed += ((t2.tv_usec - t1.tv_usec) / 1000.0);
if (elapsed > max_msync_time_ms) {
fprintf(stderr, "slow msync: %f ms\n", elapsed);
munmap(ptr, LEN);
if (munmap(ptr, LEN) != 0)
perror("munmap");
cleanup(file);
return (1);
}
Expand Down
6 changes: 5 additions & 1 deletion tests/zfs-tests/cmd/mmapwrite.c
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,11 @@ normal_writer(void *filename)
err(1, "write failed!");
break;
}
lseek(fd, page_size, SEEK_CUR);
if (lseek(fd, page_size, SEEK_CUR) == -1) {
err(1, "lseek failed on %s: %s", file_path,
strerror(errno));
break;
}
}
}

Expand Down

0 comments on commit 2a493a4

Please sign in to comment.