All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] btrfs-progs: minor fixes for clang warnings
@ 2023-01-27  5:41 Qu Wenruo
  2023-01-27  5:41 ` [PATCH 1/5] btrfs-progs: remove an unnecessary branch to silent the clang warning Qu Wenruo
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Qu Wenruo @ 2023-01-27  5:41 UTC (permalink / raw)
  To: linux-btrfs

Recently I'm migrating my default compiler from GCC to clang, mostly to
get short comiling time (especially important on my aarch64 machines).

Thus I hit those (mostly false alerts) warnings in btrfs-progs, and come
up with this patchset.

Unfortunately there is still libbtrfsutils causing warnings in
setuptools, as it's still using the default flags from gcc no matter
what.

Qu Wenruo (5):
  btrfs-progs: remove an unnecessary branch to silent the clang warning
  btrfs-progs: fix a false alert on an uninitialized variable when
    BUG_ON() is involved.
  btrfs-progs: fix fallthrough cases with proper attributes
  btrfs-progs: move a union with variable sized type to the end
  btrfs-progs: fix set but not used variables

 cmds/reflink.c              |  2 +-
 cmds/scrub.c                | 12 +++---
 common/format-output.c      |  2 +-
 common/parse-utils.c        | 12 +++---
 common/units.c              |  6 +--
 crypto/xxhash.c             | 73 +++++++++++++++++++------------------
 image/main.c                | 15 +++-----
 kerncompat.h                |  8 ++++
 kernel-shared/ctree.c       | 18 +++++----
 kernel-shared/extent-tree.c |  4 +-
 kernel-shared/print-tree.c  |  2 +-
 kernel-shared/volumes.c     |  3 +-
 kernel-shared/zoned.c       |  6 +--
 mkfs/main.c                 |  4 --
 14 files changed, 84 insertions(+), 83 deletions(-)

-- 
2.39.1


^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH 1/5] btrfs-progs: remove an unnecessary branch to silent the clang warning
  2023-01-27  5:41 [PATCH 0/5] btrfs-progs: minor fixes for clang warnings Qu Wenruo
@ 2023-01-27  5:41 ` Qu Wenruo
  2023-01-27  5:41 ` [PATCH 2/5] btrfs-progs: fix a false alert on an uninitialized variable when BUG_ON() is involved Qu Wenruo
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2023-01-27  5:41 UTC (permalink / raw)
  To: linux-btrfs

[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 --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;
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 2/5] btrfs-progs: fix a false alert on an uninitialized variable when BUG_ON() is involved.
  2023-01-27  5:41 [PATCH 0/5] btrfs-progs: minor fixes for clang warnings Qu Wenruo
  2023-01-27  5:41 ` [PATCH 1/5] btrfs-progs: remove an unnecessary branch to silent the clang warning Qu Wenruo
@ 2023-01-27  5:41 ` Qu Wenruo
  2023-01-27  5:41 ` [PATCH 3/5] btrfs-progs: fix fallthrough cases with proper attributes Qu Wenruo
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2023-01-27  5:41 UTC (permalink / raw)
  To: linux-btrfs

[FALSE ALERT]
Clang 15.0.7 gives the following false alert in get_dev_extent_len():

  kernel-shared/extent-tree.c:3328:2: warning: variable 'div' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
          default:
          ^~~~~~~
  kernel-shared/extent-tree.c:3332:24: note: uninitialized use occurs here
          return map->ce.size / div;
                                ^~~
  kernel-shared/extent-tree.c:3311:9: note: initialize the variable 'div' to silence this warning
          int div;
                 ^
                  = 0

And one in btrfs_stripe_length() too:

  kernel-shared/volumes.c:2781:2: warning: variable 'stripe_len' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
          default:
          ^~~~~~~
  kernel-shared/volumes.c:2785:9: note: uninitialized use occurs here
          return stripe_len;
                 ^~~~~~~~~~
  kernel-shared/volumes.c:2754:16: note: initialize the variable 'stripe_len' to silence this warning
          u64 stripe_len;
                        ^
                         = 0

[CAUSE]
Clang doesn't really understand what BUG_ON() means, thus in that
default case, we won't get uninitialized value but crash directly.

[FIX]
Silent the errors by assigning the default value properly using the
value of SINGLE profile.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 kernel-shared/extent-tree.c | 4 ++--
 kernel-shared/volumes.c     | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/kernel-shared/extent-tree.c b/kernel-shared/extent-tree.c
index 0b0c40afe9a9..4c6ad64f5ba8 100644
--- a/kernel-shared/extent-tree.c
+++ b/kernel-shared/extent-tree.c
@@ -3308,7 +3308,7 @@ out:
 
 static u64 get_dev_extent_len(struct map_lookup *map)
 {
-	int div;
+	int div = 1;
 
 	switch (map->type & BTRFS_BLOCK_GROUP_PROFILE_MASK) {
 	case 0: /* Single */
@@ -3316,7 +3316,7 @@ static u64 get_dev_extent_len(struct map_lookup *map)
 	case BTRFS_BLOCK_GROUP_RAID1:
 	case BTRFS_BLOCK_GROUP_RAID1C3:
 	case BTRFS_BLOCK_GROUP_RAID1C4:
-		div = 1;
+		/* The default value can already handle it. */
 		break;
 	case BTRFS_BLOCK_GROUP_RAID5:
 	case BTRFS_BLOCK_GROUP_RAID6:
diff --git a/kernel-shared/volumes.c b/kernel-shared/volumes.c
index 65147d064934..1e2c88955719 100644
--- a/kernel-shared/volumes.c
+++ b/kernel-shared/volumes.c
@@ -2758,6 +2758,7 @@ u64 btrfs_stripe_length(struct btrfs_fs_info *fs_info,
 		      BTRFS_BLOCK_GROUP_PROFILE_MASK;
 
 	chunk_len = btrfs_chunk_length(leaf, chunk);
+	stripe_len = chunk_len;
 
 	switch (profile) {
 	case 0: /* Single profile */
@@ -2765,7 +2766,7 @@ u64 btrfs_stripe_length(struct btrfs_fs_info *fs_info,
 	case BTRFS_BLOCK_GROUP_RAID1C3:
 	case BTRFS_BLOCK_GROUP_RAID1C4:
 	case BTRFS_BLOCK_GROUP_DUP:
-		stripe_len = chunk_len;
+		/* The default value is already fine. */
 		break;
 	case BTRFS_BLOCK_GROUP_RAID0:
 		stripe_len = chunk_len / num_stripes;
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 3/5] btrfs-progs: fix fallthrough cases with proper attributes
  2023-01-27  5:41 [PATCH 0/5] btrfs-progs: minor fixes for clang warnings Qu Wenruo
  2023-01-27  5:41 ` [PATCH 1/5] btrfs-progs: remove an unnecessary branch to silent the clang warning Qu Wenruo
  2023-01-27  5:41 ` [PATCH 2/5] btrfs-progs: fix a false alert on an uninitialized variable when BUG_ON() is involved Qu Wenruo
@ 2023-01-27  5:41 ` Qu Wenruo
  2023-02-09 19:24   ` David Sterba
  2023-01-27  5:41 ` [PATCH 4/5] btrfs-progs: move a union with variable sized type to the end Qu Wenruo
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 10+ messages in thread
From: Qu Wenruo @ 2023-01-27  5:41 UTC (permalink / raw)
  To: linux-btrfs

[FALSE ALERT]
Unlike gcc, clang doesn't really understand the comments, thus it's
reportings tons of fall through related errors:

  cmds/reflink.c:124:3: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
                  case 'r':
                  ^
  cmds/reflink.c:124:3: note: insert '__attribute__((fallthrough));' to silence this warning
                  case 'r':
                  ^
                  __attribute__((fallthrough));
  cmds/reflink.c:124:3: note: insert 'break;' to avoid fall-through
                  case 'r':
                  ^
                  break;

[CAUSE]
Although gcc is fine with /* fallthrough */ comments, clang is not.

[FIX]
So just introduce a fallthrough macro to handle the situation properly,
and use that macro instead.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 cmds/reflink.c             |  2 +-
 cmds/scrub.c               | 10 +++---
 common/format-output.c     |  2 +-
 common/parse-utils.c       | 12 +++----
 common/units.c             |  6 ++--
 crypto/xxhash.c            | 73 +++++++++++++++++++-------------------
 image/main.c               |  2 +-
 kerncompat.h               |  8 +++++
 kernel-shared/print-tree.c |  2 +-
 9 files changed, 63 insertions(+), 54 deletions(-)

diff --git a/cmds/reflink.c b/cmds/reflink.c
index 5ed9a12770e2..53f4784838c1 100644
--- a/cmds/reflink.c
+++ b/cmds/reflink.c
@@ -120,7 +120,7 @@ static int cmd_reflink_clone(const struct cmd_struct *cmd, int argc, char **argv
 		switch (c) {
 		case 's':
 			same_file = true;
-			/* fallthrough */
+			fallthrough;
 		case 'r':
 			range = malloc(sizeof(struct reflink_range));
 			if (!range) {
diff --git a/cmds/scrub.c b/cmds/scrub.c
index e6513b39f2bf..782a1310816b 100644
--- a/cmds/scrub.c
+++ b/cmds/scrub.c
@@ -602,7 +602,7 @@ again:
 			memset(p[curr], 0, sizeof(**p));
 			p[curr + 1] = NULL;
 			++state;
-			/* fall through */
+			fallthrough;
 		case 2: /* start of line, skip space */
 			while (isspace(l[i]) && i < avail) {
 				if (l[i] == '\n')
@@ -613,7 +613,7 @@ again:
 			    (!eof && !memchr(l + i, '\n', avail - i)))
 				goto again;
 			++state;
-			/* fall through */
+			fallthrough;
 		case 3: /* read fsid */
 			if (i == avail)
 				continue;
@@ -629,7 +629,7 @@ again:
 				_SCRUB_INVALID;
 			i += j + 1;
 			++state;
-			/* fall through */
+			fallthrough;
 		case 4: /* read dev id */
 			for (j = 0; isdigit(l[i + j]) && i+j < avail; ++j)
 				;
@@ -638,7 +638,7 @@ again:
 			p[curr]->devid = atoll(&l[i]);
 			i += j + 1;
 			++state;
-			/* fall through */
+			fallthrough;
 		case 5: /* read key/value pair */
 			ret = 0;
 			_SCRUB_KVREAD(ret, &i, data_extents_scrubbed, avail, l,
@@ -682,7 +682,7 @@ again:
 			if (ret != 1)
 				_SCRUB_INVALID;
 			++state;
-			/* fall through */
+			fallthrough;
 		case 6: /* after number */
 			if (l[i] == '|')
 				state = 5;
diff --git a/common/format-output.c b/common/format-output.c
index c7e021b58f2c..18993ad66636 100644
--- a/common/format-output.c
+++ b/common/format-output.c
@@ -74,7 +74,7 @@ static void print_escaped(const char *str)
 		case '"':
 		case '\\':
 			putchar('\\');
-			/* fallthrough */
+			fallthrough;
 		default:
 			putchar(*str);
 		}
diff --git a/common/parse-utils.c b/common/parse-utils.c
index f16b7aac1b3b..bb3c2bbf366a 100644
--- a/common/parse-utils.c
+++ b/common/parse-utils.c
@@ -184,22 +184,22 @@ u64 parse_size_from_string(const char *s)
 		switch (c) {
 		case 'e':
 			mult *= 1024;
-			/* fallthrough */
+			fallthrough;
 		case 'p':
 			mult *= 1024;
-			/* fallthrough */
+			fallthrough;
 		case 't':
 			mult *= 1024;
-			/* fallthrough */
+			fallthrough;
 		case 'g':
 			mult *= 1024;
-			/* fallthrough */
+			fallthrough;
 		case 'm':
 			mult *= 1024;
-			/* fallthrough */
+			fallthrough;
 		case 'k':
 			mult *= 1024;
-			/* fallthrough */
+			fallthrough;
 		case 'b':
 			break;
 		default:
diff --git a/common/units.c b/common/units.c
index 698dc1d0508e..8d708cddc03e 100644
--- a/common/units.c
+++ b/common/units.c
@@ -89,15 +89,15 @@ int pretty_size_snprintf(u64 size, char *str, size_t str_size, unsigned unit_mod
 	case UNITS_TBYTES:
 		base *= mult;
 		num_divs++;
-		/* fallthrough */
+		fallthrough;
 	case UNITS_GBYTES:
 		base *= mult;
 		num_divs++;
-		/* fallthrough */
+		fallthrough;
 	case UNITS_MBYTES:
 		base *= mult;
 		num_divs++;
-		/* fallthrough */
+		fallthrough;
 	case UNITS_KBYTES:
 		num_divs++;
 		break;
diff --git a/crypto/xxhash.c b/crypto/xxhash.c
index c2f1be9f7449..7f7f3f74328c 100644
--- a/crypto/xxhash.c
+++ b/crypto/xxhash.c
@@ -116,6 +116,7 @@ static void* XXH_memcpy(void* dest, const void* src, size_t size) { return memcp
 #define XXH_STATIC_LINKING_ONLY
 #include "xxhash.h"
 
+#include "kerncompat.h"
 
 /* *************************************
 *  Compiler Specific Options
@@ -397,41 +398,41 @@ XXH32_finalize(U32 h32, const void* ptr, size_t len, XXH_alignment align)
     } else {
          switch(len&15) /* or switch(bEnd - p) */ {
            case 12:      PROCESS4;
-                         /* fallthrough */
+			 fallthrough;
            case 8:       PROCESS4;
-                         /* fallthrough */
+			 fallthrough;
            case 4:       PROCESS4;
                          return XXH32_avalanche(h32);
 
            case 13:      PROCESS4;
-                         /* fallthrough */
+			 fallthrough;
            case 9:       PROCESS4;
-                         /* fallthrough */
+			 fallthrough;
            case 5:       PROCESS4;
                          PROCESS1;
                          return XXH32_avalanche(h32);
 
            case 14:      PROCESS4;
-                         /* fallthrough */
+			 fallthrough;
            case 10:      PROCESS4;
-                         /* fallthrough */
+			 fallthrough;
            case 6:       PROCESS4;
                          PROCESS1;
                          PROCESS1;
                          return XXH32_avalanche(h32);
 
            case 15:      PROCESS4;
-                         /* fallthrough */
+			 fallthrough;
            case 11:      PROCESS4;
-                         /* fallthrough */
+			 fallthrough;
            case 7:       PROCESS4;
-                         /* fallthrough */
+			 fallthrough;
            case 3:       PROCESS1;
-                         /* fallthrough */
+			 fallthrough;
            case 2:       PROCESS1;
-                         /* fallthrough */
+			 fallthrough;
            case 1:       PROCESS1;
-                         /* fallthrough */
+			 fallthrough;
            case 0:       return XXH32_avalanche(h32);
         }
         XXH_ASSERT(0);
@@ -825,63 +826,63 @@ XXH64_finalize(U64 h64, const void* ptr, size_t len, XXH_alignment align)
     } else {
         switch(len & 31) {
            case 24: PROCESS8_64;
-                         /* fallthrough */
+		    fallthrough;
            case 16: PROCESS8_64;
-                         /* fallthrough */
+		    fallthrough;
            case  8: PROCESS8_64;
                     return XXH64_avalanche(h64);
 
            case 28: PROCESS8_64;
-                         /* fallthrough */
+		    fallthrough;
            case 20: PROCESS8_64;
-                         /* fallthrough */
+		    fallthrough;
            case 12: PROCESS8_64;
-                         /* fallthrough */
+		    fallthrough;
            case  4: PROCESS4_64;
                     return XXH64_avalanche(h64);
 
            case 25: PROCESS8_64;
-                         /* fallthrough */
+		    fallthrough;
            case 17: PROCESS8_64;
-                         /* fallthrough */
+		    fallthrough;
            case  9: PROCESS8_64;
                     PROCESS1_64;
                     return XXH64_avalanche(h64);
 
            case 29: PROCESS8_64;
-                         /* fallthrough */
+		    fallthrough;
            case 21: PROCESS8_64;
-                         /* fallthrough */
+		    fallthrough;
            case 13: PROCESS8_64;
-                         /* fallthrough */
+		    fallthrough;
            case  5: PROCESS4_64;
                     PROCESS1_64;
                     return XXH64_avalanche(h64);
 
            case 26: PROCESS8_64;
-                         /* fallthrough */
+		    fallthrough;
            case 18: PROCESS8_64;
-                         /* fallthrough */
+		    fallthrough;
            case 10: PROCESS8_64;
                     PROCESS1_64;
                     PROCESS1_64;
                     return XXH64_avalanche(h64);
 
            case 30: PROCESS8_64;
-                         /* fallthrough */
+		    fallthrough;
            case 22: PROCESS8_64;
-                         /* fallthrough */
+		    fallthrough;
            case 14: PROCESS8_64;
-                         /* fallthrough */
+		    fallthrough;
            case  6: PROCESS4_64;
                     PROCESS1_64;
                     PROCESS1_64;
                     return XXH64_avalanche(h64);
 
            case 27: PROCESS8_64;
-                         /* fallthrough */
+		    fallthrough;
            case 19: PROCESS8_64;
-                         /* fallthrough */
+		    fallthrough;
            case 11: PROCESS8_64;
                     PROCESS1_64;
                     PROCESS1_64;
@@ -889,19 +890,19 @@ XXH64_finalize(U64 h64, const void* ptr, size_t len, XXH_alignment align)
                     return XXH64_avalanche(h64);
 
            case 31: PROCESS8_64;
-                         /* fallthrough */
+		    fallthrough;
            case 23: PROCESS8_64;
-                         /* fallthrough */
+		    fallthrough;
            case 15: PROCESS8_64;
-                         /* fallthrough */
+		    fallthrough;
            case  7: PROCESS4_64;
-                         /* fallthrough */
+		    fallthrough;
            case  3: PROCESS1_64;
-                         /* fallthrough */
+		    fallthrough;
            case  2: PROCESS1_64;
-                         /* fallthrough */
+		    fallthrough;
            case  1: PROCESS1_64;
-                         /* fallthrough */
+		    fallthrough;
            case  0: return XXH64_avalanche(h64);
         }
     }
diff --git a/image/main.c b/image/main.c
index 8900297ebe1a..ecf464e5b0a2 100644
--- a/image/main.c
+++ b/image/main.c
@@ -1431,7 +1431,7 @@ static int restore_one_work(struct mdrestore_struct *mdres,
 			switch (ret) {
 			case Z_NEED_DICT:
 				ret = Z_DATA_ERROR;
-				__attribute__ ((fallthrough));
+				fallthrough;
 			case Z_DATA_ERROR:
 			case Z_MEM_ERROR:
 				goto out;
diff --git a/kerncompat.h b/kerncompat.h
index 6fb79ac88351..74383ed9ff6c 100644
--- a/kerncompat.h
+++ b/kerncompat.h
@@ -227,6 +227,14 @@ static inline int mutex_is_locked(struct mutex *m)
 #define __attribute_const__	__attribute__((__const__))
 #endif
 
+/* To silent compilers like clang which doesn't understand comments. */
+
+#if __has_attribute(__fallthrough__)
+# define fallthrough			__attribute__((__fallthrough__))
+#else
+# define fallthrough			do {} while (0)  /* fallthrough */
+#endif
+
 /**
  * __set_bit - Set a bit in memory
  * @nr: the bit to set
diff --git a/kernel-shared/print-tree.c b/kernel-shared/print-tree.c
index ba1caa88fcf1..534166283997 100644
--- a/kernel-shared/print-tree.c
+++ b/kernel-shared/print-tree.c
@@ -796,7 +796,7 @@ void print_objectid(FILE *stream, u64 objectid, u8 type)
 			fprintf(stream, "FIRST_CHUNK_TREE");
 			break;
 		}
-		/* fall-thru */
+		fallthrough;
 	default:
 		fprintf(stream, "%llu", (unsigned long long)objectid);
 	}
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 4/5] btrfs-progs: move a union with variable sized type to the end
  2023-01-27  5:41 [PATCH 0/5] btrfs-progs: minor fixes for clang warnings Qu Wenruo
                   ` (2 preceding siblings ...)
  2023-01-27  5:41 ` [PATCH 3/5] btrfs-progs: fix fallthrough cases with proper attributes Qu Wenruo
@ 2023-01-27  5:41 ` Qu Wenruo
  2023-01-27  5:41 ` [PATCH 5/5] btrfs-progs: fix set but not used variables Qu Wenruo
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2023-01-27  5:41 UTC (permalink / raw)
  To: linux-btrfs

[WARNING]
Clang 15.0.7 gives the following warning:

  image/main.c:95:2: warning: field '' with variable sized type 'union metadump_struct::(anonymous at image/main.c:95:2)' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end]
          union {
          ^

[CAUSE]
The union contains meta_cluster, which variable sized:

  struct meta_cluster {
  	struct meta_cluster_header header;
  	struct meta_cluster_item items[];
  } __attribute__ ((__packed__));

Thus clang gives above warning since it's a GNU extension.

[FIX]
Just move the union to the end of the structure.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 image/main.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/image/main.c b/image/main.c
index ecf464e5b0a2..d4a1fe349d31 100644
--- a/image/main.c
+++ b/image/main.c
@@ -92,11 +92,6 @@ struct metadump_struct {
 	struct btrfs_root *root;
 	FILE *out;
 
-	union {
-		struct meta_cluster cluster;
-		char meta_cluster_bytes[IMAGE_BLOCK_SIZE];
-	};
-
 	pthread_t threads[MAX_WORKER_THREADS];
 	size_t num_threads;
 	pthread_mutex_t mutex;
@@ -119,6 +114,11 @@ struct metadump_struct {
 	enum sanitize_mode sanitize_names;
 
 	int error;
+
+	union {
+		struct meta_cluster cluster;
+		char meta_cluster_bytes[IMAGE_BLOCK_SIZE];
+	};
 };
 
 struct mdrestore_struct {
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 5/5] btrfs-progs: fix set but not used variables
  2023-01-27  5:41 [PATCH 0/5] btrfs-progs: minor fixes for clang warnings Qu Wenruo
                   ` (3 preceding siblings ...)
  2023-01-27  5:41 ` [PATCH 4/5] btrfs-progs: move a union with variable sized type to the end Qu Wenruo
@ 2023-01-27  5:41 ` Qu Wenruo
  2023-01-30  7:28 ` [PATCH 0/5] btrfs-progs: minor fixes for clang warnings Qu Wenruo
  2023-02-09 19:26 ` David Sterba
  6 siblings, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2023-01-27  5:41 UTC (permalink / raw)
  To: linux-btrfs

[WARNING]
Clang 15.0.7 warns about several unused variables:

  kernel-shared/zoned.c:829:6: warning: variable 'num_sequential' set but not used [-Wunused-but-set-variable]
          u32 num_sequential = 0, num_conventional = 0;
              ^
  cmds/scrub.c:1174:6: warning: variable 'n_skip' set but not used [-Wunused-but-set-variable]
          int n_skip = 0;
              ^
  mkfs/main.c:493:6: warning: variable 'total_block_count' set but not used [-Wunused-but-set-variable]
          u64 total_block_count = 0;
              ^
  image/main.c:2246:6: warning: variable 'bytenr' set but not used [-Wunused-but-set-variable]
          u64 bytenr = 0;
              ^

[CAUSE]
Most of them just straightforward set but not used variables.

The only exception is total_block_count, which has commented out code
relying on it.

[FIX]
Just remove those variables, and for @total_block_count, also remove the
comments.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 cmds/scrub.c          | 2 --
 image/main.c          | 3 ---
 kernel-shared/zoned.c | 6 ++----
 mkfs/main.c           | 4 ----
 4 files changed, 2 insertions(+), 13 deletions(-)

diff --git a/cmds/scrub.c b/cmds/scrub.c
index 782a1310816b..65c7c5b6fe8a 100644
--- a/cmds/scrub.c
+++ b/cmds/scrub.c
@@ -1171,7 +1171,6 @@ static int scrub_start(const struct cmd_struct *cmd, int argc, char **argv,
 	int ioprio_class = IOPRIO_CLASS_IDLE;
 	int ioprio_classdata = 0;
 	int n_start = 0;
-	int n_skip = 0;
 	int n_resume = 0;
 	struct btrfs_ioctl_fs_info_args fi_args;
 	struct btrfs_ioctl_dev_info_args *di_args = NULL;
@@ -1337,7 +1336,6 @@ static int scrub_start(const struct cmd_struct *cmd, int argc, char **argv,
 			sp[i].scrub_args.start = last_scrub->p.last_physical;
 			sp[i].resumed = last_scrub;
 		} else if (resume) {
-			++n_skip;
 			sp[i].skip = 1;
 			sp[i].resumed = last_scrub;
 			continue;
diff --git a/image/main.c b/image/main.c
index d4a1fe349d31..8aa4c1552807 100644
--- a/image/main.c
+++ b/image/main.c
@@ -2243,7 +2243,6 @@ static int build_chunk_tree(struct mdrestore_struct *mdres,
 	struct meta_cluster_header *header;
 	struct meta_cluster_item *item = NULL;
 	u32 i, nritems;
-	u64 bytenr = 0;
 	u8 *buffer;
 	int ret;
 
@@ -2265,7 +2264,6 @@ static int build_chunk_tree(struct mdrestore_struct *mdres,
 		return -EIO;
 	}
 
-	bytenr += IMAGE_BLOCK_SIZE;
 	mdres->compress_method = header->compress;
 	nritems = le32_to_cpu(header->nritems);
 	for (i = 0; i < nritems; i++) {
@@ -2273,7 +2271,6 @@ static int build_chunk_tree(struct mdrestore_struct *mdres,
 
 		if (le64_to_cpu(item->bytenr) == BTRFS_SUPER_INFO_OFFSET)
 			break;
-		bytenr += le32_to_cpu(item->size);
 		if (fseek(mdres->in, le32_to_cpu(item->size), SEEK_CUR)) {
 			error("seek failed: %m");
 			return -EIO;
diff --git a/kernel-shared/zoned.c b/kernel-shared/zoned.c
index a79fc6a5dbc3..f06ee24322bf 100644
--- a/kernel-shared/zoned.c
+++ b/kernel-shared/zoned.c
@@ -826,7 +826,7 @@ int btrfs_load_block_group_zone_info(struct btrfs_fs_info *fs_info,
 	int i;
 	u64 *alloc_offsets = NULL;
 	u64 last_alloc = 0;
-	u32 num_sequential = 0, num_conventional = 0;
+	u32 num_conventional = 0;
 
 	if (!btrfs_is_zoned(fs_info))
 		return 0;
@@ -870,9 +870,7 @@ int btrfs_load_block_group_zone_info(struct btrfs_fs_info *fs_info,
 		}
 
 		is_sequential = btrfs_dev_is_sequential(device, physical);
-		if (is_sequential)
-			num_sequential++;
-		else
+		if (!is_sequential)
 			num_conventional++;
 
 		if (!is_sequential) {
diff --git a/mkfs/main.c b/mkfs/main.c
index 9f106e33f869..341ba4089484 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -490,7 +490,6 @@ static void list_all_devices(struct btrfs_root *root)
 	struct btrfs_fs_devices *fs_devices;
 	struct btrfs_device *device;
 	int number_of_devices = 0;
-	u64 total_block_count = 0;
 
 	fs_devices = root->fs_info->fs_devices;
 
@@ -500,8 +499,6 @@ static void list_all_devices(struct btrfs_root *root)
 	list_sort(NULL, &fs_devices->devices, _cmp_device_by_id);
 
 	printf("Number of devices:  %d\n", number_of_devices);
-	/* printf("Total devices size: %10s\n", */
-		/* pretty_size(total_block_count)); */
 	printf("Devices:\n");
 	printf("   ID        SIZE  PATH\n");
 	list_for_each_entry(device, &fs_devices->devices, dev_list) {
@@ -509,7 +506,6 @@ static void list_all_devices(struct btrfs_root *root)
 			device->devid,
 			pretty_size(device->total_bytes),
 			device->name);
-		total_block_count += device->total_bytes;
 	}
 
 	printf("\n");
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [PATCH 0/5] btrfs-progs: minor fixes for clang warnings
  2023-01-27  5:41 [PATCH 0/5] btrfs-progs: minor fixes for clang warnings Qu Wenruo
                   ` (4 preceding siblings ...)
  2023-01-27  5:41 ` [PATCH 5/5] btrfs-progs: fix set but not used variables Qu Wenruo
@ 2023-01-30  7:28 ` Qu Wenruo
  2023-01-30  8:14   ` hmsjwzb
  2023-02-09 19:26 ` David Sterba
  6 siblings, 1 reply; 10+ messages in thread
From: Qu Wenruo @ 2023-01-30  7:28 UTC (permalink / raw)
  To: linux-btrfs



On 2023/1/27 13:41, Qu Wenruo wrote:
> Recently I'm migrating my default compiler from GCC to clang, mostly to
> get short comiling time (especially important on my aarch64 machines).

Just to mention, although the cleanup itself still makes sense, it 
doesn't make any sense to migrate to clang, at least on my aarch64 machines.

GCC takes only 22min for my trimmed kernel config, while the same config 
takes clang 32min...

Thanks,
Qu

> 
> Thus I hit those (mostly false alerts) warnings in btrfs-progs, and come
> up with this patchset.
> 
> Unfortunately there is still libbtrfsutils causing warnings in
> setuptools, as it's still using the default flags from gcc no matter
> what.
> 
> Qu Wenruo (5):
>    btrfs-progs: remove an unnecessary branch to silent the clang warning
>    btrfs-progs: fix a false alert on an uninitialized variable when
>      BUG_ON() is involved.
>    btrfs-progs: fix fallthrough cases with proper attributes
>    btrfs-progs: move a union with variable sized type to the end
>    btrfs-progs: fix set but not used variables
> 
>   cmds/reflink.c              |  2 +-
>   cmds/scrub.c                | 12 +++---
>   common/format-output.c      |  2 +-
>   common/parse-utils.c        | 12 +++---
>   common/units.c              |  6 +--
>   crypto/xxhash.c             | 73 +++++++++++++++++++------------------
>   image/main.c                | 15 +++-----
>   kerncompat.h                |  8 ++++
>   kernel-shared/ctree.c       | 18 +++++----
>   kernel-shared/extent-tree.c |  4 +-
>   kernel-shared/print-tree.c  |  2 +-
>   kernel-shared/volumes.c     |  3 +-
>   kernel-shared/zoned.c       |  6 +--
>   mkfs/main.c                 |  4 --
>   14 files changed, 84 insertions(+), 83 deletions(-)
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 0/5] btrfs-progs: minor fixes for clang warnings
  2023-01-30  7:28 ` [PATCH 0/5] btrfs-progs: minor fixes for clang warnings Qu Wenruo
@ 2023-01-30  8:14   ` hmsjwzb
  0 siblings, 0 replies; 10+ messages in thread
From: hmsjwzb @ 2023-01-30  8:14 UTC (permalink / raw)
  To: Qu Wenruo, linux-btrfs

Hi Qu,

	I think maybe clang has some optimization which GCC doesn't.

Thanks

On 1/30/23 02:28, Qu Wenruo wrote:
> 
> 
> On 2023/1/27 13:41, Qu Wenruo wrote:
>> Recently I'm migrating my default compiler from GCC to clang, mostly to
>> get short comiling time (especially important on my aarch64 machines).
> 
> Just to mention, although the cleanup itself still makes sense, it doesn't make any sense to migrate to clang, at least on my aarch64 machines.
> 
> GCC takes only 22min for my trimmed kernel config, while the same config takes clang 32min...
> 
> Thanks,
> Qu
> 
>>
>> Thus I hit those (mostly false alerts) warnings in btrfs-progs, and come
>> up with this patchset.
>>
>> Unfortunately there is still libbtrfsutils causing warnings in
>> setuptools, as it's still using the default flags from gcc no matter
>> what.
>>
>> Qu Wenruo (5):
>>    btrfs-progs: remove an unnecessary branch to silent the clang warning
>>    btrfs-progs: fix a false alert on an uninitialized variable when
>>      BUG_ON() is involved.
>>    btrfs-progs: fix fallthrough cases with proper attributes
>>    btrfs-progs: move a union with variable sized type to the end
>>    btrfs-progs: fix set but not used variables
>>
>>   cmds/reflink.c              |  2 +-
>>   cmds/scrub.c                | 12 +++---
>>   common/format-output.c      |  2 +-
>>   common/parse-utils.c        | 12 +++---
>>   common/units.c              |  6 +--
>>   crypto/xxhash.c             | 73 +++++++++++++++++++------------------
>>   image/main.c                | 15 +++-----
>>   kerncompat.h                |  8 ++++
>>   kernel-shared/ctree.c       | 18 +++++----
>>   kernel-shared/extent-tree.c |  4 +-
>>   kernel-shared/print-tree.c  |  2 +-
>>   kernel-shared/volumes.c     |  3 +-
>>   kernel-shared/zoned.c       |  6 +--
>>   mkfs/main.c                 |  4 --
>>   14 files changed, 84 insertions(+), 83 deletions(-)
>>

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 3/5] btrfs-progs: fix fallthrough cases with proper attributes
  2023-01-27  5:41 ` [PATCH 3/5] btrfs-progs: fix fallthrough cases with proper attributes Qu Wenruo
@ 2023-02-09 19:24   ` David Sterba
  0 siblings, 0 replies; 10+ messages in thread
From: David Sterba @ 2023-02-09 19:24 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs

On Fri, Jan 27, 2023 at 01:41:16PM +0800, Qu Wenruo wrote:
> --- a/kerncompat.h
> +++ b/kerncompat.h
> @@ -227,6 +227,14 @@ static inline int mutex_is_locked(struct mutex *m)
>  #define __attribute_const__	__attribute__((__const__))
>  #endif
>  
> +/* To silent compilers like clang which doesn't understand comments. */
> +
> +#if __has_attribute(__fallthrough__)

The use of __has_attribute should be beihnd and ifdef
(https://gcc.gnu.org/onlinedocs/cpp/_005f_005fhas_005fattribute.html)
otherwise it fails, eg. on the reference Centos 7 build. Fixed.


> +# define fallthrough			__attribute__((__fallthrough__))
> +#else
> +# define fallthrough			do {} while (0)  /* fallthrough */
> +#endif

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 0/5] btrfs-progs: minor fixes for clang warnings
  2023-01-27  5:41 [PATCH 0/5] btrfs-progs: minor fixes for clang warnings Qu Wenruo
                   ` (5 preceding siblings ...)
  2023-01-30  7:28 ` [PATCH 0/5] btrfs-progs: minor fixes for clang warnings Qu Wenruo
@ 2023-02-09 19:26 ` David Sterba
  6 siblings, 0 replies; 10+ messages in thread
From: David Sterba @ 2023-02-09 19:26 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs

On Fri, Jan 27, 2023 at 01:41:13PM +0800, Qu Wenruo wrote:
> Recently I'm migrating my default compiler from GCC to clang, mostly to
> get short comiling time (especially important on my aarch64 machines).
> 
> Thus I hit those (mostly false alerts) warnings in btrfs-progs, and come
> up with this patchset.
> 
> Unfortunately there is still libbtrfsutils causing warnings in
> setuptools, as it's still using the default flags from gcc no matter
> what.
> 
> Qu Wenruo (5):
>   btrfs-progs: remove an unnecessary branch to silent the clang warning
>   btrfs-progs: fix a false alert on an uninitialized variable when
>     BUG_ON() is involved.
>   btrfs-progs: fix fallthrough cases with proper attributes
>   btrfs-progs: move a union with variable sized type to the end
>   btrfs-progs: fix set but not used variables

Added to devel, thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-02-09 19:32 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-27  5:41 [PATCH 0/5] btrfs-progs: minor fixes for clang warnings Qu Wenruo
2023-01-27  5:41 ` [PATCH 1/5] btrfs-progs: remove an unnecessary branch to silent the clang warning Qu Wenruo
2023-01-27  5:41 ` [PATCH 2/5] btrfs-progs: fix a false alert on an uninitialized variable when BUG_ON() is involved Qu Wenruo
2023-01-27  5:41 ` [PATCH 3/5] btrfs-progs: fix fallthrough cases with proper attributes Qu Wenruo
2023-02-09 19:24   ` David Sterba
2023-01-27  5:41 ` [PATCH 4/5] btrfs-progs: move a union with variable sized type to the end Qu Wenruo
2023-01-27  5:41 ` [PATCH 5/5] btrfs-progs: fix set but not used variables Qu Wenruo
2023-01-30  7:28 ` [PATCH 0/5] btrfs-progs: minor fixes for clang warnings Qu Wenruo
2023-01-30  8:14   ` hmsjwzb
2023-02-09 19:26 ` David Sterba

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.