diff mbox series

[1/5] btrfs-progs: remove an unnecessary branch to silent the clang warning

Message ID 0268c78594151742044d27b6e1dce877c63ef5fb.1674797823.git.wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs-progs: minor fixes for clang warnings | expand

Commit Message

Qu Wenruo Jan. 27, 2023, 5:41 a.m. UTC
[FALSE ALERT]
With clang 15.0.7, there is a false alert on uninitialized value in
ctree.c:

  kernel-shared/ctree.c:3418:13: warning: variable 'offset' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
          } else if (ret < 0) {
                     ^~~~~~~
  kernel-shared/ctree.c:3428:41: note: uninitialized use occurs here
          write_extent_buffer(eb, &subvol_id_le, offset, sizeof(subvol_id_le));
                                                 ^~~~~~
  kernel-shared/ctree.c:3418:9: note: remove the 'if' if its condition is always true
          } else if (ret < 0) {
                 ^~~~~~~~~~~~~
  kernel-shared/ctree.c:3380:22: note: initialize the variable 'offset' to silence this warning
          unsigned long offset;
                              ^
                               = 0
  kernel-shared/ctree.c:3418:13: warning: variable 'eb' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
          } else if (ret < 0) {
                     ^~~~~~~
  kernel-shared/ctree.c:3429:26: note: uninitialized use occurs here
          btrfs_mark_buffer_dirty(eb);
                                  ^~
  kernel-shared/ctree.c:3418:9: note: remove the 'if' if its condition is always true
          } else if (ret < 0) {
                 ^~~~~~~~~~~~~
  kernel-shared/ctree.c:3378:26: note: initialize the variable 'eb' to silence this warning
          struct extent_buffer *eb;
                                  ^
                                   = NULL

[CAUSE]
The original code is handling the return value from
btrfs_insert_empty_item() like this:

	ret = btrfs_insert_empty_item();
	if (ret >= 0) {
		/* Do something for it. */
	} else if (ret == -EEXIST) {
		/* Do something else. */
	} else if (ret < 0) {
		/* Error handling. */
	}

But the problem is, the last one check is always true if we can reach
there.

Thus clang is providing the hint to remove the if () chekc.

[FIX]
Normally we prefer to do error handling first, so move the error
handling first so we don't need the if () else if () chain.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 kernel-shared/ctree.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)
diff mbox series

Patch

diff --git a/kernel-shared/ctree.c b/kernel-shared/ctree.c
index 9f8bc9a5904c..759ee47aaadd 100644
--- a/kernel-shared/ctree.c
+++ b/kernel-shared/ctree.c
@@ -3400,14 +3400,22 @@  int btrfs_uuid_tree_add(struct btrfs_trans_handle *trans, u8 *uuid, u8 type,
 
 	ret = btrfs_insert_empty_item(trans, uuid_root, path, &key,
 				      sizeof(subvol_id_le));
+	if (ret < 0 && ret != -EEXIST) {
+		warning(
+		"inserting uuid item failed (0x%016llx, 0x%016llx) type %u: %d",
+			(unsigned long long)key.objectid,
+			(unsigned long long)key.offset, type, ret);
+		goto out;
+	}
+
 	if (ret >= 0) {
 		/* Add an item for the type for the first time */
 		eb = path->nodes[0];
 		slot = path->slots[0];
 		offset = btrfs_item_ptr_offset(eb, slot);
-	} else if (ret == -EEXIST) {
+	} else {
 		/*
-		 * An item with that type already exists.
+		 * ret == -EEXIST case, An item with that type already exists.
 		 * Extend the item and store the new subvol_id at the end.
 		 */
 		btrfs_extend_item(uuid_root, path, sizeof(subvol_id_le));
@@ -3415,12 +3423,6 @@  int btrfs_uuid_tree_add(struct btrfs_trans_handle *trans, u8 *uuid, u8 type,
 		slot = path->slots[0];
 		offset = btrfs_item_ptr_offset(eb, slot);
 		offset += btrfs_item_size(eb, slot) - sizeof(subvol_id_le);
-	} else if (ret < 0) {
-		warning(
-		"inserting uuid item failed (0x%016llx, 0x%016llx) type %u: %d",
-			(unsigned long long)key.objectid,
-			(unsigned long long)key.offset, type, ret);
-		goto out;
 	}
 
 	ret = 0;