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 <[email protected]>
Reviewed-by: Cedric Berger <[email protected]>
Signed-off-by: Richard Yao <[email protected]>
Closes #13986 

5 files changed, 13 insertions, 31 deletions

diff --git a/tests/zfs-tests/cmd/btree_test.c b/tests/zfs-tests/cmd/btree_test.c
index ab8967b22..fb9de9c77 100644
--- a/tests/zfs-tests/cmd/btree_test.c
+++ b/tests/zfs-tests/cmd/btree_test.c
@@ -220,7 +220,6 @@ insert_find_remove(zfs_btree_t *bt, char *why)
 static int
 drain_tree(zfs_btree_t *bt, char *why)
 {
-	uint64_t *p;
 	avl_tree_t avl;
 	int i = 0;
 	int_node_t *node;
@@ -232,11 +231,8 @@ drain_tree(zfs_btree_t *bt, char *why)
 
 	/* Fill both trees with the same data */
 	for (i = 0; i < 64 * 1024; i++) {
-		void *ret;
-
 		u_longlong_t randval = random();
-		if ((p = (uint64_t *)zfs_btree_find(bt, &randval, &bt_idx)) !=
-		    NULL) {
+		if (zfs_btree_find(bt, &randval, &bt_idx) != NULL) {
 			continue;
 		}
 		zfs_btree_add_idx(bt, &randval, &bt_idx);
@@ -248,7 +244,7 @@ drain_tree(zfs_btree_t *bt, char *why)
 		}
 
 		node->data = randval;
-		if ((ret = avl_find(&avl, node, &avl_idx)) != NULL) {
+		if (avl_find(&avl, node, &avl_idx) != NULL) {
 			(void) snprintf(why, BUFSIZE,
 			    "Found in avl: %llu\n", randval);
 			return (1);
@@ -372,9 +368,7 @@ stress_tree(zfs_btree_t *bt, char *why)
 
 	if (stress_only) {
 		zfs_btree_index_t *idx = NULL;
-		uint64_t *rv;
-
-		while ((rv = zfs_btree_destroy_nodes(bt, &idx)) != NULL)
+		while (zfs_btree_destroy_nodes(bt, &idx) != NULL)
 			;
 		zfs_btree_verify(bt);
 	}
@@ -389,15 +383,15 @@ stress_tree(zfs_btree_t *bt, char *why)
 static int
 insert_duplicate(zfs_btree_t *bt)
 {
-	uint64_t *p, i = 23456;
+	uint64_t i = 23456;
 	zfs_btree_index_t bt_idx = {0};
 
-	if ((p = (uint64_t *)zfs_btree_find(bt, &i, &bt_idx)) != NULL) {
+	if (zfs_btree_find(bt, &i, &bt_idx) != NULL) {
 		fprintf(stderr, "Found value in empty tree.\n");
 		return (0);
 	}
 	zfs_btree_add_idx(bt, &i, &bt_idx);
-	if ((p = (uint64_t *)zfs_btree_find(bt, &i, &bt_idx)) == NULL) {
+	if (zfs_btree_find(bt, &i, &bt_idx) == NULL) {
 		fprintf(stderr, "Did not find expected value.\n");
 		return (0);
 	}
@@ -415,10 +409,10 @@ insert_duplicate(zfs_btree_t *bt)
 static int
 remove_missing(zfs_btree_t *bt)
 {
-	uint64_t *p, i = 23456;
+	uint64_t i = 23456;
 	zfs_btree_index_t bt_idx = {0};
 
-	if ((p = (uint64_t *)zfs_btree_find(bt, &i, &bt_idx)) != NULL) {
+	if (zfs_btree_find(bt, &i, &bt_idx) != NULL) {
 		fprintf(stderr, "Found value in empty tree.\n");
 		return (0);
 	}
@@ -499,10 +493,6 @@ main(int argc, char *argv[])
 			break;
 		}
 	}
-	argc -= optind;
-	argv += optind;
-	optind = 1;
-
 
 	if (seed == 0) {
 		(void) gettimeofday(&tp, NULL);
@@ -536,7 +526,6 @@ main(int argc, char *argv[])
 	btree_test_t *test = &test_table[0];
 	while (test->name) {
 		int retval;
-		uint64_t *rv;
 		char why[BUFSIZE] = {0};
 		zfs_btree_index_t *idx = NULL;
 
@@ -554,7 +543,7 @@ main(int argc, char *argv[])
 		}
 
 		/* Remove all the elements and re-verify the tree */
-		while ((rv = zfs_btree_destroy_nodes(&bt, &idx)) != NULL)
+		while (zfs_btree_destroy_nodes(&bt, &idx) != NULL)
 			;
 		zfs_btree_verify(&bt);
 
diff --git a/tests/zfs-tests/cmd/ctime.c b/tests/zfs-tests/cmd/ctime.c
index f0f3d526e..0f5d81aea 100644
--- a/tests/zfs-tests/cmd/ctime.c
+++ b/tests/zfs-tests/cmd/ctime.c
@@ -327,7 +327,6 @@ main(void)
 	if (access(tfile, F_OK) == 0) {
 		(void) unlink(tfile);
 	}
-	ret = 0;
 	if ((fd = open(tfile, O_WRONLY | O_CREAT | O_TRUNC, ALL_MODE)) == -1) {
 		(void) fprintf(stderr, "open(%s) failed: %d\n", tfile, errno);
 		return (1);
diff --git a/tests/zfs-tests/cmd/draid.c b/tests/zfs-tests/cmd/draid.c
index 46d7b4dcc..3e5ff59f7 100644
--- a/tests/zfs-tests/cmd/draid.c
+++ b/tests/zfs-tests/cmd/draid.c
@@ -1285,12 +1285,11 @@ draid_merge_impl(nvlist_t *allcfgs, const char *srcfilename, int *mergedp)
 
 				if (nv_worst_ratio < allcfg_worst_ratio) {
 					fnvlist_remove(allcfgs, key);
-					error = nvlist_add_nvlist(allcfgs,
-					    key, cfg);
+					fnvlist_add_nvlist(allcfgs, key, cfg);
 					merged++;
 				}
 			} else if (error == ENOENT) {
-				error = nvlist_add_nvlist(allcfgs, key, cfg);
+				fnvlist_add_nvlist(allcfgs, key, cfg);
 				merged++;
 			} else {
 				return (error);
diff --git a/tests/zfs-tests/cmd/mkbusy.c b/tests/zfs-tests/cmd/mkbusy.c
index cc4a6cfcb..78860381d 100644
--- a/tests/zfs-tests/cmd/mkbusy.c
+++ b/tests/zfs-tests/cmd/mkbusy.c
@@ -148,14 +148,10 @@ main(int argc, char *argv[])
 		}
 
 	if (!isdir) {
-		int	fd;
-
-		if ((fd = open(fpath, O_CREAT | O_RDWR, 0600)) < 0)
+		if (open(fpath, O_CREAT | O_RDWR, 0600) < 0)
 			fail("open");
 	} else {
-		DIR	*dp;
-
-		if ((dp = opendir(fpath)) == NULL)
+		if (opendir(fpath) == NULL)
 			fail("opendir");
 	}
 	free(fpath);
diff --git a/tests/zfs-tests/cmd/user_ns_exec.c b/tests/zfs-tests/cmd/user_ns_exec.c
index 865936223..d78130147 100644
--- a/tests/zfs-tests/cmd/user_ns_exec.c
+++ b/tests/zfs-tests/cmd/user_ns_exec.c
@@ -97,7 +97,6 @@ set_idmap(pid_t pid, const char *file)
 
 	mapfd = open(path, O_WRONLY);
 	if (mapfd < 0) {
-		result = errno;
 		perror("open");
 		return (errno);
 	}