[PATCH 0/9] btrfs-progs: Make W=1 clean (no "again")

[PATCH 0/9] btrfs-progs: Make W=1 clean (no "again")

[Qu Wenruo]

This patchset can be fetched from github:
https://github.com/adam900710/btrfs-progs/tree/warning_fixes
Which is based on v4.19 tag.

This patchset will make "make W=1" reports no warning.

This patch will first introduce fix to Makefile.extrawarn to make
"cc-disable-warning" works, then disable sign-compare warning
completely, as we really don't want extra "unsigned" prefix to slow our
typing.

Then re-use (ok, in fact rework) Yanjun's patch to disable
formwat-truncation warning.

Finally, fix all the remaining warnings reported by make W=1.

Now, we make "make W=1" clean (may 'again' or not, depending on the
distribution rolling speed).

Qu Wenruo (8):
  btrfs-progs: Makefile.extrawarn: Import cc-disable-warning
  btrfs-progs: Makefile.extrawarn: Don't warn on sign compare
  btrfs-progs: Fix Wempty-body warning
  btrfs-progs: Fix Wimplicit-fallthrough warning
  btrfs-progs: Fix Wsuggest-attribute=format warning
  btrfs-progs: Fix Wmaybe-uninitialized warning
  btrfs-progs: Fix Wtype-limits warning
  btrfs-progs: Cleanup warning reported by -Wmissing-prototypes except
    free space tree

Su Yanjun (1):
  btrfs-progs: fix gcc8 default build warning caused by
    '-Wformat-truncation'

 Makefile            |  1 +
 Makefile.extrawarn  |  9 ++++++-
 btrfs.c             |  2 +-
 check/mode-lowmem.c |  6 ++---
 chunk-recover.c     |  1 +
 cmds-rescue.c       |  4 +--
 ctree.c             |  3 ++-
 extent-tree.c       |  2 +-
 free-space-tree.c   | 59 ++++++++++++---------------------------------
 fsfeatures.c        | 22 +++++++++++++++++
 messages.h          | 15 ++++++++----
 rescue.h            | 14 +++++++++++
 send-stream.c       |  3 +--
 string-table.c      |  1 +
 super-recover.c     |  1 +
 utils-lib.c         |  1 +
 utils.c             | 53 +++++++++++++++++-----------------------
 17 files changed, 105 insertions(+), 92 deletions(-)
 create mode 100644 rescue.h

-- 
2.19.1

[PATCH 1/9] btrfs-progs: Makefile.extrawarn: Import cc-disable-warning

[Qu Wenruo]

We imported cc-option but forgot to import cc-disable-warning.

Fixes: b556a992c3ad ("btrfs-progs: build: allow to build with various compiler warnings")
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 Makefile.extrawarn | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Makefile.extrawarn b/Makefile.extrawarn
index 1f4bda94a167..5849036fd166 100644
--- a/Makefile.extrawarn
+++ b/Makefile.extrawarn
@@ -19,6 +19,12 @@ try-run = $(shell set -e;               \
  cc-option = $(call try-run,\
          $(CC) $(CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
 
+# cc-disable-warning
+# Usage: cflags-y += $(call cc-disable-warning,unused-but-set-variable)
+cc-disable-warning = $(call try-run,\
+	$(CC) -Werror $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1)))
+
+
 # From linux.git/scripts/Makefile.extrawarn
 # ==========================================================================
 #
-- 
2.19.1

[PATCH 2/9] btrfs-progs: fix gcc8 default build warning caused by '-Wformat-truncation'

[Qu Wenruo]

From: Su Yanjun <suyj.fnst@cn.fujitsu.com>

When using gcc8 compiles utils.c, it complains as below:

utils.c:852:45: warning: '%s' directive output may be truncated writing
up to 4095 bytes into a region of size 4084 [-Wformat-truncation=]
   snprintf(path, sizeof(path), "/dev/mapper/%s", name);
                                             ^~   ~~~~
In file included from /usr/include/stdio.h:873,
                 from utils.c:20:
/usr/include/bits/stdio2.h:67:10: note: '__builtin___snprintf_chk'
output between 13 and 4108 bytes into a destination of size 4096
   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        __bos (__s), __fmt, __va_arg_pack ());
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This isn't a type of warning we care about, particularly when PATH_MAX
is much less than either.

Using the GCC option -Wno-format-truncation to disable this for default
build.

Signed-off-by: Su Yanjun <suyj.fnst@cn.fujitsu.com>
[Use cc-disable-warning to fix the not working CFLAGS setting in configure.ac]
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 Makefile           | 1 +
 Makefile.extrawarn | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Makefile b/Makefile
index f4ab14ea74c8..252299f8869f 100644
--- a/Makefile
+++ b/Makefile
@@ -49,6 +49,7 @@ CSCOPE_CMD := cscope -u -b -c -q
 include Makefile.extrawarn
 
 EXTRA_CFLAGS :=
+EXTRA_CFLAGS += $(call cc-disable-warning, format-truncation)
 EXTRA_LDFLAGS :=
 
 DEBUG_CFLAGS_DEFAULT = -O0 -U_FORTIFY_SOURCE -ggdb3
diff --git a/Makefile.extrawarn b/Makefile.extrawarn
index 5849036fd166..bbb2d5173846 100644
--- a/Makefile.extrawarn
+++ b/Makefile.extrawarn
@@ -53,6 +53,7 @@ warning-1 += -Wold-style-definition
 warning-1 += $(call cc-option, -Wmissing-include-dirs)
 warning-1 += $(call cc-option, -Wunused-but-set-variable)
 warning-1 += $(call cc-disable-warning, missing-field-initializers)
+warning-1 += $(call cc-disable-warning, format-truncation)
 
 warning-2 := -Waggregate-return
 warning-2 += -Wcast-align
-- 
2.19.1

[PATCH 3/9] btrfs-progs: Makefile.extrawarn: Don't warn on sign compare

[Qu Wenruo]

Under most case, we are just using 'int' for 'unsigned int', and doesn't
care about the sign.

The Wsign-compare is causing tons of false alerts.
Suppressing it would make W=1 less noisy.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 Makefile.extrawarn | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile.extrawarn b/Makefile.extrawarn
index bbb2d5173846..c108e98ee7ec 100644
--- a/Makefile.extrawarn
+++ b/Makefile.extrawarn
@@ -45,7 +45,7 @@ endif
 ifdef BUILD_ENABLE_EXTRA_GCC_CHECKS
 warning-  := $(empty)
 
-warning-1 := -Wextra -Wunused -Wno-unused-parameter
+warning-1 := -Wextra -Wunused -Wno-unused-parameter -Wno-sign-compare
 warning-1 += -Wmissing-declarations
 warning-1 += -Wmissing-format-attribute
 warning-1 += $(call cc-option, -Wmissing-prototypes)
-- 
2.19.1

[PATCH 4/9] btrfs-progs: Fix Wempty-body warning

[Qu Wenruo]

messages.h:49:24: warning: suggest braces around empty body in an 'if' statement [-Wempty-body]
    PRINT_TRACE_ON_ERROR;    \

Just extra braces would solve the problem.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 messages.h | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/messages.h b/messages.h
index ec7d93381a36..16f650d19a4b 100644
--- a/messages.h
+++ b/messages.h
@@ -45,13 +45,16 @@
 
 #define error_on(cond, fmt, ...)					\
 	do {								\
-		if ((cond))						\
+		if ((cond)) {						\
 			PRINT_TRACE_ON_ERROR;				\
-		if ((cond))						\
+		}							\
+		if ((cond)) {						\
 			PRINT_VERBOSE_ERROR;				\
+		}							\
 		__btrfs_error_on((cond), (fmt), ##__VA_ARGS__);		\
-		if ((cond))						\
+		if ((cond)) {						\
 			DO_ABORT_ON_ERROR;				\
+		}							\
 	} while (0)
 
 #define error_btrfs_util(err)						\
@@ -76,10 +79,12 @@
 
 #define warning_on(cond, fmt, ...)					\
 	do {								\
-		if ((cond))						\
+		if ((cond)) {						\
 			PRINT_TRACE_ON_ERROR;				\
-		if ((cond))						\
+		}							\
+		if ((cond)) {						\
 			PRINT_VERBOSE_ERROR;				\
+		}							\
 		__btrfs_warning_on((cond), (fmt), ##__VA_ARGS__);	\
 	} while (0)
 
-- 
2.19.1

[PATCH 5/9] btrfs-progs: Fix Wimplicit-fallthrough warning

[Qu Wenruo]

Although most fallthrough case is pretty obvious, we still need to teach
the dumb compiler that it's an explicit fallthrough.

Also reformat the code to use common indent.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 utils.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/utils.c b/utils.c
index a310300829eb..b274f46fdd9d 100644
--- a/utils.c
+++ b/utils.c
@@ -1134,15 +1134,25 @@ int pretty_size_snprintf(u64 size, char *str, size_t str_size, unsigned unit_mod
 	num_divs = 0;
 	last_size = size;
 	switch (unit_mode & UNITS_MODE_MASK) {
-	case UNITS_TBYTES: base *= mult; num_divs++;
-	case UNITS_GBYTES: base *= mult; num_divs++;
-	case UNITS_MBYTES: base *= mult; num_divs++;
-	case UNITS_KBYTES: num_divs++;
-			   break;
+	case UNITS_TBYTES:
+		base *= mult;
+		num_divs++;
+		__attribute__ ((fallthrough));
+	case UNITS_GBYTES:
+		base *= mult;
+		num_divs++;
+		__attribute__ ((fallthrough));
+	case UNITS_MBYTES:
+		base *= mult;
+		num_divs++;
+		__attribute__ ((fallthrough));
+	case UNITS_KBYTES:
+		num_divs++;
+		break;
 	case UNITS_BYTES:
-			   base = 1;
-			   num_divs = 0;
-			   break;
+		base = 1;
+		num_divs = 0;
+		break;
 	default:
 		if (negative) {
 			s64 ssize = (s64)size;
@@ -1907,13 +1917,17 @@ int test_num_disk_vs_raid(u64 metadata_profile, u64 data_profile,
 	default:
 	case 4:
 		allowed |= BTRFS_BLOCK_GROUP_RAID10;
+		__attribute__ ((fallthrough));
 	case 3:
 		allowed |= BTRFS_BLOCK_GROUP_RAID6;
+		__attribute__ ((fallthrough));
 	case 2:
 		allowed |= BTRFS_BLOCK_GROUP_RAID0 | BTRFS_BLOCK_GROUP_RAID1 |
 			BTRFS_BLOCK_GROUP_RAID5;
+		__attribute__ ((fallthrough));
 	case 1:
 		allowed |= BTRFS_BLOCK_GROUP_DUP;
+		__attribute__ ((fallthrough));
 	}
 
 	if (dev_cnt > 1 && profile & BTRFS_BLOCK_GROUP_DUP) {
-- 
2.19.1

[PATCH 6/9] btrfs-progs: Fix Wsuggest-attribute=format warning

[Qu Wenruo]

Add __attribute__ ((format (printf, 4, 0))) to fix the vprintf calling
function.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 string-table.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/string-table.c b/string-table.c
index 95833768960d..455285702d51 100644
--- a/string-table.c
+++ b/string-table.c
@@ -48,6 +48,7 @@ struct string_table *table_create(int columns, int rows)
  * '>' the text is right aligned. If fmt is equal to '=' the text will
  * be replaced by a '=====' dimensioned on the basis of the column width
  */
+__attribute__ ((format (printf, 4, 0)))
 char *table_vprintf(struct string_table *tab, int column, int row,
 			  const char *fmt, va_list ap)
 {
-- 
2.19.1

[PATCH 7/9] btrfs-progs: Fix Wmaybe-uninitialized warning

[Qu Wenruo]

The only location is the following code:

	int level = path->lowest_level + 1;
 	BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
 	while(level < BTRFS_MAX_LEVEL) {
		slot = path->slots[level] + 1;
		...
	}
	path->slots[level] = slot;

Again, it's the stupid compiler needs some hint for the fact that
we will always enter the while loop for at least once, thus @slot should
always be initialized.

Fix it by assign level after the BUG_ON(), so the stupid compiler knows
we will always go into the while loop.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 ctree.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ctree.c b/ctree.c
index 46e2ccedc0bf..9c9cb0dfdbf2 100644
--- a/ctree.c
+++ b/ctree.c
@@ -2961,11 +2961,12 @@ int btrfs_next_sibling_tree_block(struct btrfs_fs_info *fs_info,
 				  struct btrfs_path *path)
 {
 	int slot;
-	int level = path->lowest_level + 1;
+	int level;
 	struct extent_buffer *c;
 	struct extent_buffer *next = NULL;
 
 	BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
+	level = path->lowest_level + 1;
 	while(level < BTRFS_MAX_LEVEL) {
 		if (!path->nodes[level])
 			return 1;
-- 
2.19.1

[PATCH 8/9] btrfs-progs: Fix Wtype-limits warning

[Qu Wenruo]

The only hit is the following code:

 		tlv_len = le16_to_cpu(tlv_hdr->tlv_len);

		if (tlv_type == 0 || tlv_type > BTRFS_SEND_A_MAX
		    || tlv_len > BTRFS_SEND_BUF_SIZE) {
 			error("invalid tlv in cmd tlv_type = %hu, tlv_len = %hu",
 					tlv_type, tlv_len);

@tlv_len is u16, while BTRFS_SEND_BUF_SIZE is 64K.
u16 MAX is 64K - 1, so the final check is always false.

Just remove it.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 send-stream.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/send-stream.c b/send-stream.c
index 3b8e39c9486a..25461e92c37b 100644
--- a/send-stream.c
+++ b/send-stream.c
@@ -157,8 +157,7 @@ static int read_cmd(struct btrfs_send_stream *sctx)
 		tlv_type = le16_to_cpu(tlv_hdr->tlv_type);
 		tlv_len = le16_to_cpu(tlv_hdr->tlv_len);
 
-		if (tlv_type == 0 || tlv_type > BTRFS_SEND_A_MAX
-		    || tlv_len > BTRFS_SEND_BUF_SIZE) {
+		if (tlv_type == 0 || tlv_type > BTRFS_SEND_A_MAX) {
 			error("invalid tlv in cmd tlv_type = %hu, tlv_len = %hu",
 					tlv_type, tlv_len);
 			ret = -EINVAL;
-- 
2.19.1

[PATCH 9/9] btrfs-progs: Cleanup warning reported by -Wmissing-prototypes except free space tree

[Qu Wenruo]

The following missing prototypes will be fixed:
1)  btrfs.c::handle_special_globals()
2)  check/mode-lowmem.c::repair_ternary_lowmem()
3)  extent-tree.c::btrfs_search_overlap_extent()
    Above 3 can be fixed by making them static

4)  utils.c::btrfs_check_nodesize()
    Fixed by moving it to fsfeatures.c

5)  chunk-recover.c::btrfs_recover_chunk_tree()
6)  super-recover.c::btrfs_recover_superblocks()
    Fixed by moving the declaration from cmds-rescue.c to rescue.h

7)  utils-lib.c::arg_strtou64()
8)  utils-lib.c::lookup_path_rootid()
    Fixed by include "utils.h"

9)  free-space-tree.c::set_free_space_tree_thresholds()
    Fixed by deleting it, as there is no user.

10) free-space-tree.c::convert_free_space_to_bitmaps()
11) free-space-tree.c::convert_free_space_to_extents()
12) free-space-tree.c::__remove_from_free_space_tree()
13) free-space-tree.c::__add_to_free_space_tree()
14) free-space-tree.c::btrfs_create_tree()
    Fixed by making them static.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 btrfs.c             |  2 +-
 check/mode-lowmem.c |  6 ++---
 chunk-recover.c     |  1 +
 cmds-rescue.c       |  4 +--
 extent-tree.c       |  2 +-
 free-space-tree.c   | 59 ++++++++++++---------------------------------
 fsfeatures.c        | 22 +++++++++++++++++
 rescue.h            | 14 +++++++++++
 super-recover.c     |  1 +
 utils-lib.c         |  1 +
 utils.c             | 23 ------------------
 11 files changed, 60 insertions(+), 75 deletions(-)
 create mode 100644 rescue.h

diff --git a/btrfs.c b/btrfs.c
index 2d39f2ced3e8..78c468d2e050 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -210,7 +210,7 @@ static int handle_global_options(int argc, char **argv)
 	return shift;
 }
 
-void handle_special_globals(int shift, int argc, char **argv)
+static void handle_special_globals(int shift, int argc, char **argv)
 {
 	int has_help = 0;
 	int has_full = 0;
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 14bbc9ee6cb6..f56b5e8d45dc 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -953,9 +953,9 @@ out:
  * returns 0 means success.
  * returns not 0 means on error;
  */
-int repair_ternary_lowmem(struct btrfs_root *root, u64 dir_ino, u64 ino,
-			  u64 index, char *name, int name_len, u8 filetype,
-			  int err)
+static int repair_ternary_lowmem(struct btrfs_root *root, u64 dir_ino, u64 ino,
+				 u64 index, char *name, int name_len,
+				 u8 filetype, int err)
 {
 	struct btrfs_trans_handle *trans;
 	int stage = 0;
diff --git a/chunk-recover.c b/chunk-recover.c
index 1d30db51d8ed..1e554b8e8750 100644
--- a/chunk-recover.c
+++ b/chunk-recover.c
@@ -40,6 +40,7 @@
 #include "utils.h"
 #include "btrfsck.h"
 #include "commands.h"
+#include "rescue.h"
 
 struct recover_control {
 	int verbose;
diff --git a/cmds-rescue.c b/cmds-rescue.c
index 2bc50c0841ed..36e9e1277e40 100644
--- a/cmds-rescue.c
+++ b/cmds-rescue.c
@@ -26,15 +26,13 @@
 #include "commands.h"
 #include "utils.h"
 #include "help.h"
+#include "rescue.h"
 
 static const char * const rescue_cmd_group_usage[] = {
 	"btrfs rescue <command> [options] <path>",
 	NULL
 };
 
-int btrfs_recover_chunk_tree(const char *path, int verbose, int yes);
-int btrfs_recover_superblocks(const char *path, int verbose, int yes);
-
 static const char * const cmd_rescue_chunk_recover_usage[] = {
 	"btrfs rescue chunk-recover [options] <device>",
 	"Recover the chunk tree by scanning the devices one by one.",
diff --git a/extent-tree.c b/extent-tree.c
index cd98633992ac..8c9cdeff3b02 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -3749,7 +3749,7 @@ static void __get_extent_size(struct btrfs_root *root, struct btrfs_path *path,
  * Return >0 for not found.
  * Return <0 for err
  */
-int btrfs_search_overlap_extent(struct btrfs_root *root,
+static int btrfs_search_overlap_extent(struct btrfs_root *root,
 				struct btrfs_path *path, u64 bytenr, u64 len)
 {
 	struct btrfs_key key;
diff --git a/free-space-tree.c b/free-space-tree.c
index 6641cdfa42ba..af141e6e611a 100644
--- a/free-space-tree.c
+++ b/free-space-tree.c
@@ -24,35 +24,6 @@
 #include "bitops.h"
 #include "internal.h"
 
-void set_free_space_tree_thresholds(struct btrfs_block_group_cache *cache,
-				    u64 sectorsize)
-{
-	u32 bitmap_range;
-	size_t bitmap_size;
-	u64 num_bitmaps, total_bitmap_size;
-
-	/*
-	 * We convert to bitmaps when the disk space required for using extents
-	 * exceeds that required for using bitmaps.
-	 */
-	bitmap_range = sectorsize * BTRFS_FREE_SPACE_BITMAP_BITS;
-	num_bitmaps = div_u64(cache->key.offset + bitmap_range - 1,
-			      bitmap_range);
-	bitmap_size = sizeof(struct btrfs_item) + BTRFS_FREE_SPACE_BITMAP_SIZE;
-	total_bitmap_size = num_bitmaps * bitmap_size;
-	cache->bitmap_high_thresh = div_u64(total_bitmap_size,
-					    sizeof(struct btrfs_item));
-
-	/*
-	 * We allow for a small buffer between the high threshold and low
-	 * threshold to avoid thrashing back and forth between the two formats.
-	 */
-	if (cache->bitmap_high_thresh > 100)
-		cache->bitmap_low_thresh = cache->bitmap_high_thresh - 100;
-	else
-		cache->bitmap_low_thresh = 0;
-}
-
 static struct btrfs_free_space_info *
 search_free_space_info(struct btrfs_trans_handle *trans,
 		       struct btrfs_fs_info *fs_info,
@@ -202,9 +173,9 @@ static void le_bitmap_set(unsigned long *map, unsigned int start, int len)
 	}
 }
 
-int convert_free_space_to_bitmaps(struct btrfs_trans_handle *trans,
-				  struct btrfs_block_group_cache *block_group,
-				  struct btrfs_path *path)
+static int convert_free_space_to_bitmaps(struct btrfs_trans_handle *trans,
+				struct btrfs_block_group_cache *block_group,
+				struct btrfs_path *path)
 {
 	struct btrfs_fs_info *fs_info = trans->fs_info;
 	struct btrfs_root *root = fs_info->free_space_root;
@@ -341,9 +312,9 @@ out:
 	return ret;
 }
 
-int convert_free_space_to_extents(struct btrfs_trans_handle *trans,
-				  struct btrfs_block_group_cache *block_group,
-				  struct btrfs_path *path)
+static int convert_free_space_to_extents(struct btrfs_trans_handle *trans,
+				struct btrfs_block_group_cache *block_group,
+				struct btrfs_path *path)
 {
 	struct btrfs_fs_info *fs_info = trans->fs_info;
 	struct btrfs_root *root = fs_info->free_space_root;
@@ -780,9 +751,9 @@ out:
 	return ret;
 }
 
-int __remove_from_free_space_tree(struct btrfs_trans_handle *trans,
-				  struct btrfs_block_group_cache *block_group,
-				  struct btrfs_path *path, u64 start, u64 size)
+static int __remove_from_free_space_tree(struct btrfs_trans_handle *trans,
+				struct btrfs_block_group_cache *block_group,
+				struct btrfs_path *path, u64 start, u64 size)
 {
 	struct btrfs_free_space_info *info;
 	u32 flags;
@@ -960,9 +931,9 @@ out:
 	return ret;
 }
 
-int __add_to_free_space_tree(struct btrfs_trans_handle *trans,
-			     struct btrfs_block_group_cache *block_group,
-			     struct btrfs_path *path, u64 start, u64 size)
+static int __add_to_free_space_tree(struct btrfs_trans_handle *trans,
+				struct btrfs_block_group_cache *block_group,
+				struct btrfs_path *path, u64 start, u64 size)
 {
 	struct btrfs_fs_info *fs_info = trans->fs_info;
 	struct btrfs_free_space_info *info;
@@ -1420,9 +1391,9 @@ out:
 	return ret;
 }
 
-struct btrfs_root *btrfs_create_tree(struct btrfs_trans_handle *trans,
-				     struct btrfs_fs_info *fs_info,
-				     u64 objectid)
+static struct btrfs_root *btrfs_create_tree(struct btrfs_trans_handle *trans,
+					    struct btrfs_fs_info *fs_info,
+					    u64 objectid)
 {
 	struct extent_buffer *leaf;
 	struct btrfs_root *tree_root = fs_info->tree_root;
diff --git a/fsfeatures.c b/fsfeatures.c
index 7d85d60f1277..201d0f576688 100644
--- a/fsfeatures.c
+++ b/fsfeatures.c
@@ -225,3 +225,25 @@ u32 get_running_kernel_version(void)
 	return version;
 }
 
+int btrfs_check_nodesize(u32 nodesize, u32 sectorsize, u64 features)
+{
+	if (nodesize < sectorsize) {
+		error("illegal nodesize %u (smaller than %u)",
+				nodesize, sectorsize);
+		return -1;
+	} else if (nodesize > BTRFS_MAX_METADATA_BLOCKSIZE) {
+		error("illegal nodesize %u (larger than %u)",
+			nodesize, BTRFS_MAX_METADATA_BLOCKSIZE);
+		return -1;
+	} else if (nodesize & (sectorsize - 1)) {
+		error("illegal nodesize %u (not aligned to %u)",
+			nodesize, sectorsize);
+		return -1;
+	} else if (features & BTRFS_FEATURE_INCOMPAT_MIXED_GROUPS &&
+		   nodesize != sectorsize) {
+		error("illegal nodesize %u (not equal to %u for mixed block group)",
+			nodesize, sectorsize);
+		return -1;
+	}
+	return 0;
+}
diff --git a/rescue.h b/rescue.h
new file mode 100644
index 000000000000..c9f6e7b80b16
--- /dev/null
+++ b/rescue.h
@@ -0,0 +1,14 @@
+/*
+ * Copyright (C) 2018 SUSE.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+int btrfs_recover_superblocks(const char *path, int verbose, int yes);
+int btrfs_recover_chunk_tree(const char *path, int verbose, int yes);
diff --git a/super-recover.c b/super-recover.c
index 86b3df9867dc..a1af71786034 100644
--- a/super-recover.c
+++ b/super-recover.c
@@ -34,6 +34,7 @@
 #include "crc32c.h"
 #include "volumes.h"
 #include "commands.h"
+#include "rescue.h"
 
 struct btrfs_recover_superblock {
 	struct btrfs_fs_devices *fs_devices;
diff --git a/utils-lib.c b/utils-lib.c
index 044f93fc4446..5bb89f2f1a8d 100644
--- a/utils-lib.c
+++ b/utils-lib.c
@@ -1,4 +1,5 @@
 #include "kerncompat.h"
+#include "utils.h"
 #include <unistd.h>
 #include <stdlib.h>
 #include <limits.h>
diff --git a/utils.c b/utils.c
index b274f46fdd9d..a7e34b804551 100644
--- a/utils.c
+++ b/utils.c
@@ -2266,29 +2266,6 @@ int btrfs_tree_search2_ioctl_supported(int fd)
 	return ret;
 }
 
-int btrfs_check_nodesize(u32 nodesize, u32 sectorsize, u64 features)
-{
-	if (nodesize < sectorsize) {
-		error("illegal nodesize %u (smaller than %u)",
-				nodesize, sectorsize);
-		return -1;
-	} else if (nodesize > BTRFS_MAX_METADATA_BLOCKSIZE) {
-		error("illegal nodesize %u (larger than %u)",
-			nodesize, BTRFS_MAX_METADATA_BLOCKSIZE);
-		return -1;
-	} else if (nodesize & (sectorsize - 1)) {
-		error("illegal nodesize %u (not aligned to %u)",
-			nodesize, sectorsize);
-		return -1;
-	} else if (features & BTRFS_FEATURE_INCOMPAT_MIXED_GROUPS &&
-		   nodesize != sectorsize) {
-		error("illegal nodesize %u (not equal to %u for mixed block group)",
-			nodesize, sectorsize);
-		return -1;
-	}
-	return 0;
-}
-
 /*
  * Copy a path argument from SRC to DEST and check the SRC length if it's at
  * most PATH_MAX and fits into DEST. DESTLEN is supposed to be exact size of
-- 
2.19.1

[PATCH v1.1 3/9] btrfs-progs: Makefile.extrawarn: Don't warn on sign compare

[Qu Wenruo]

Under most case, we are just using 'int' for 'unsigned int', and doesn't
care about the sign.

The Wsign-compare is causing tons of false alerts.
Suppressing it would make W=1 less noisy.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
changelog:
v1.1:
   Use cc-disable-warning to provide much better compatibility against
   older compiler
---
 Makefile.extrawarn | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Makefile.extrawarn b/Makefile.extrawarn
index bbb2d5173846..736bee82759f 100644
--- a/Makefile.extrawarn
+++ b/Makefile.extrawarn
@@ -54,6 +54,7 @@ warning-1 += $(call cc-option, -Wmissing-include-dirs)
 warning-1 += $(call cc-option, -Wunused-but-set-variable)
 warning-1 += $(call cc-disable-warning, missing-field-initializers)
 warning-1 += $(call cc-disable-warning, format-truncation)
+warning-1 += $(call cc-disable-warning, sign-compare)
 
 warning-2 := -Waggregate-return
 warning-2 += -Wcast-align
-- 
2.19.1

Re: [PATCH 4/9] btrfs-progs: Fix Wempty-body warning

[Nikolay Borisov]

On 16.11.18 г. 9:54 ч., Qu Wenruo wrote:
> messages.h:49:24: warning: suggest braces around empty body in an 'if' statement [-Wempty-body]
>     PRINT_TRACE_ON_ERROR;    \
> 
> Just extra braces would solve the problem.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  messages.h | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/messages.h b/messages.h
> index ec7d93381a36..16f650d19a4b 100644
> --- a/messages.h
> +++ b/messages.h
> @@ -45,13 +45,16 @@
>  
>  #define error_on(cond, fmt, ...)					\
>  	do {								\
> -		if ((cond))						\
> +		if ((cond)) {						\
>  			PRINT_TRACE_ON_ERROR;				\
> -		if ((cond))						\
> +		}							\
> +		if ((cond)) {						\
>  			PRINT_VERBOSE_ERROR;				\
> +		}							\
>  		__btrfs_error_on((cond), (fmt), ##__VA_ARGS__);		\
> -		if ((cond))						\
> +		if ((cond)) {						\
>  			DO_ABORT_ON_ERROR;				\
> +		}							\
>  	} while (0)
>  
>  #define error_btrfs_util(err)						\
> @@ -76,10 +79,12 @@
>  
>  #define warning_on(cond, fmt, ...)					\
>  	do {								\
> -		if ((cond))						\
> +		if ((cond)) {						\
>  			PRINT_TRACE_ON_ERROR;				\
> -		if ((cond))						\
> +		}							\
> +		if ((cond)) {						\
>  			PRINT_VERBOSE_ERROR;				\
> +		}							\
>  		__btrfs_warning_on((cond), (fmt), ##__VA_ARGS__);	\
>  	} while (0)
>  
> 

Re: [PATCH 5/9] btrfs-progs: Fix Wimplicit-fallthrough warning

[Nikolay Borisov]

On 16.11.18 г. 9:54 ч., Qu Wenruo wrote:
> Although most fallthrough case is pretty obvious, we still need to teach
> the dumb compiler that it's an explicit fallthrough.
> 
> Also reformat the code to use common indent.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

Is this attribute dependent on a particular compiler version? Does
gcc4.4 for example support it?

> ---
>  utils.c | 30 ++++++++++++++++++++++--------
>  1 file changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/utils.c b/utils.c
> index a310300829eb..b274f46fdd9d 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -1134,15 +1134,25 @@ int pretty_size_snprintf(u64 size, char *str, size_t str_size, unsigned unit_mod
>  	num_divs = 0;
>  	last_size = size;
>  	switch (unit_mode & UNITS_MODE_MASK) {
> -	case UNITS_TBYTES: base *= mult; num_divs++;
> -	case UNITS_GBYTES: base *= mult; num_divs++;
> -	case UNITS_MBYTES: base *= mult; num_divs++;
> -	case UNITS_KBYTES: num_divs++;
> -			   break;
> +	case UNITS_TBYTES:
> +		base *= mult;
> +		num_divs++;
> +		__attribute__ ((fallthrough));
> +	case UNITS_GBYTES:
> +		base *= mult;
> +		num_divs++;
> +		__attribute__ ((fallthrough));
> +	case UNITS_MBYTES:
> +		base *= mult;
> +		num_divs++;
> +		__attribute__ ((fallthrough));
> +	case UNITS_KBYTES:
> +		num_divs++;
> +		break;
>  	case UNITS_BYTES:
> -			   base = 1;
> -			   num_divs = 0;
> -			   break;
> +		base = 1;
> +		num_divs = 0;
> +		break;
>  	default:
>  		if (negative) {
>  			s64 ssize = (s64)size;
> @@ -1907,13 +1917,17 @@ int test_num_disk_vs_raid(u64 metadata_profile, u64 data_profile,
>  	default:
>  	case 4:
>  		allowed |= BTRFS_BLOCK_GROUP_RAID10;
> +		__attribute__ ((fallthrough));
>  	case 3:
>  		allowed |= BTRFS_BLOCK_GROUP_RAID6;
> +		__attribute__ ((fallthrough));
>  	case 2:
>  		allowed |= BTRFS_BLOCK_GROUP_RAID0 | BTRFS_BLOCK_GROUP_RAID1 |
>  			BTRFS_BLOCK_GROUP_RAID5;
> +		__attribute__ ((fallthrough));
>  	case 1:
>  		allowed |= BTRFS_BLOCK_GROUP_DUP;
> +		__attribute__ ((fallthrough));
>  	}
>  
>  	if (dev_cnt > 1 && profile & BTRFS_BLOCK_GROUP_DUP) {
> 

[PATCH v1.1 9/9] btrfs-progs: Cleanup warning reported by -Wmissing-prototypes

[Qu Wenruo]

The following missing prototypes will be fixed:
1)  btrfs.c::handle_special_globals()
2)  check/mode-lowmem.c::repair_ternary_lowmem()
3)  extent-tree.c::btrfs_search_overlap_extent()
    Above 3 can be fixed by making them static

4)  utils.c::btrfs_check_nodesize()
    Fixed by moving it to fsfeatures.c

5)  chunk-recover.c::btrfs_recover_chunk_tree()
6)  super-recover.c::btrfs_recover_superblocks()
    Fixed by moving the declaration from cmds-rescue.c to rescue.h

7)  utils-lib.c::arg_strtou64()
8)  utils-lib.c::lookup_path_rootid()
    Fixed by include "utils.h"

9)  free-space-tree.c::set_free_space_tree_thresholds()
    Fixed by deleting it, as there is no user.

10) free-space-tree.c::convert_free_space_to_bitmaps()
11) free-space-tree.c::convert_free_space_to_extents()
12) free-space-tree.c::__remove_from_free_space_tree()
13) free-space-tree.c::__add_to_free_space_tree()
14) free-space-tree.c::btrfs_create_tree()
    Fixed by making them static.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
changelog:
v1.1
   Update the subject, as we free space tree will also be cleaned up.
---
 btrfs.c             |  2 +-
 check/mode-lowmem.c |  6 ++---
 chunk-recover.c     |  1 +
 cmds-rescue.c       |  4 +--
 extent-tree.c       |  2 +-
 free-space-tree.c   | 59 ++++++++++++---------------------------------
 fsfeatures.c        | 22 +++++++++++++++++
 rescue.h            | 14 +++++++++++
 super-recover.c     |  1 +
 utils-lib.c         |  1 +
 utils.c             | 23 ------------------
 11 files changed, 60 insertions(+), 75 deletions(-)
 create mode 100644 rescue.h

diff --git a/btrfs.c b/btrfs.c
index 2d39f2ced3e8..78c468d2e050 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -210,7 +210,7 @@ static int handle_global_options(int argc, char **argv)
 	return shift;
 }
 
-void handle_special_globals(int shift, int argc, char **argv)
+static void handle_special_globals(int shift, int argc, char **argv)
 {
 	int has_help = 0;
 	int has_full = 0;
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 14bbc9ee6cb6..f56b5e8d45dc 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -953,9 +953,9 @@ out:
  * returns 0 means success.
  * returns not 0 means on error;
  */
-int repair_ternary_lowmem(struct btrfs_root *root, u64 dir_ino, u64 ino,
-			  u64 index, char *name, int name_len, u8 filetype,
-			  int err)
+static int repair_ternary_lowmem(struct btrfs_root *root, u64 dir_ino, u64 ino,
+				 u64 index, char *name, int name_len,
+				 u8 filetype, int err)
 {
 	struct btrfs_trans_handle *trans;
 	int stage = 0;
diff --git a/chunk-recover.c b/chunk-recover.c
index 1d30db51d8ed..1e554b8e8750 100644
--- a/chunk-recover.c
+++ b/chunk-recover.c
@@ -40,6 +40,7 @@
 #include "utils.h"
 #include "btrfsck.h"
 #include "commands.h"
+#include "rescue.h"
 
 struct recover_control {
 	int verbose;
diff --git a/cmds-rescue.c b/cmds-rescue.c
index 2bc50c0841ed..36e9e1277e40 100644
--- a/cmds-rescue.c
+++ b/cmds-rescue.c
@@ -26,15 +26,13 @@
 #include "commands.h"
 #include "utils.h"
 #include "help.h"
+#include "rescue.h"
 
 static const char * const rescue_cmd_group_usage[] = {
 	"btrfs rescue <command> [options] <path>",
 	NULL
 };
 
-int btrfs_recover_chunk_tree(const char *path, int verbose, int yes);
-int btrfs_recover_superblocks(const char *path, int verbose, int yes);
-
 static const char * const cmd_rescue_chunk_recover_usage[] = {
 	"btrfs rescue chunk-recover [options] <device>",
 	"Recover the chunk tree by scanning the devices one by one.",
diff --git a/extent-tree.c b/extent-tree.c
index cd98633992ac..8c9cdeff3b02 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -3749,7 +3749,7 @@ static void __get_extent_size(struct btrfs_root *root, struct btrfs_path *path,
  * Return >0 for not found.
  * Return <0 for err
  */
-int btrfs_search_overlap_extent(struct btrfs_root *root,
+static int btrfs_search_overlap_extent(struct btrfs_root *root,
 				struct btrfs_path *path, u64 bytenr, u64 len)
 {
 	struct btrfs_key key;
diff --git a/free-space-tree.c b/free-space-tree.c
index 6641cdfa42ba..af141e6e611a 100644
--- a/free-space-tree.c
+++ b/free-space-tree.c
@@ -24,35 +24,6 @@
 #include "bitops.h"
 #include "internal.h"
 
-void set_free_space_tree_thresholds(struct btrfs_block_group_cache *cache,
-				    u64 sectorsize)
-{
-	u32 bitmap_range;
-	size_t bitmap_size;
-	u64 num_bitmaps, total_bitmap_size;
-
-	/*
-	 * We convert to bitmaps when the disk space required for using extents
-	 * exceeds that required for using bitmaps.
-	 */
-	bitmap_range = sectorsize * BTRFS_FREE_SPACE_BITMAP_BITS;
-	num_bitmaps = div_u64(cache->key.offset + bitmap_range - 1,
-			      bitmap_range);
-	bitmap_size = sizeof(struct btrfs_item) + BTRFS_FREE_SPACE_BITMAP_SIZE;
-	total_bitmap_size = num_bitmaps * bitmap_size;
-	cache->bitmap_high_thresh = div_u64(total_bitmap_size,
-					    sizeof(struct btrfs_item));
-
-	/*
-	 * We allow for a small buffer between the high threshold and low
-	 * threshold to avoid thrashing back and forth between the two formats.
-	 */
-	if (cache->bitmap_high_thresh > 100)
-		cache->bitmap_low_thresh = cache->bitmap_high_thresh - 100;
-	else
-		cache->bitmap_low_thresh = 0;
-}
-
 static struct btrfs_free_space_info *
 search_free_space_info(struct btrfs_trans_handle *trans,
 		       struct btrfs_fs_info *fs_info,
@@ -202,9 +173,9 @@ static void le_bitmap_set(unsigned long *map, unsigned int start, int len)
 	}
 }
 
-int convert_free_space_to_bitmaps(struct btrfs_trans_handle *trans,
-				  struct btrfs_block_group_cache *block_group,
-				  struct btrfs_path *path)
+static int convert_free_space_to_bitmaps(struct btrfs_trans_handle *trans,
+				struct btrfs_block_group_cache *block_group,
+				struct btrfs_path *path)
 {
 	struct btrfs_fs_info *fs_info = trans->fs_info;
 	struct btrfs_root *root = fs_info->free_space_root;
@@ -341,9 +312,9 @@ out:
 	return ret;
 }
 
-int convert_free_space_to_extents(struct btrfs_trans_handle *trans,
-				  struct btrfs_block_group_cache *block_group,
-				  struct btrfs_path *path)
+static int convert_free_space_to_extents(struct btrfs_trans_handle *trans,
+				struct btrfs_block_group_cache *block_group,
+				struct btrfs_path *path)
 {
 	struct btrfs_fs_info *fs_info = trans->fs_info;
 	struct btrfs_root *root = fs_info->free_space_root;
@@ -780,9 +751,9 @@ out:
 	return ret;
 }
 
-int __remove_from_free_space_tree(struct btrfs_trans_handle *trans,
-				  struct btrfs_block_group_cache *block_group,
-				  struct btrfs_path *path, u64 start, u64 size)
+static int __remove_from_free_space_tree(struct btrfs_trans_handle *trans,
+				struct btrfs_block_group_cache *block_group,
+				struct btrfs_path *path, u64 start, u64 size)
 {
 	struct btrfs_free_space_info *info;
 	u32 flags;
@@ -960,9 +931,9 @@ out:
 	return ret;
 }
 
-int __add_to_free_space_tree(struct btrfs_trans_handle *trans,
-			     struct btrfs_block_group_cache *block_group,
-			     struct btrfs_path *path, u64 start, u64 size)
+static int __add_to_free_space_tree(struct btrfs_trans_handle *trans,
+				struct btrfs_block_group_cache *block_group,
+				struct btrfs_path *path, u64 start, u64 size)
 {
 	struct btrfs_fs_info *fs_info = trans->fs_info;
 	struct btrfs_free_space_info *info;
@@ -1420,9 +1391,9 @@ out:
 	return ret;
 }
 
-struct btrfs_root *btrfs_create_tree(struct btrfs_trans_handle *trans,
-				     struct btrfs_fs_info *fs_info,
-				     u64 objectid)
+static struct btrfs_root *btrfs_create_tree(struct btrfs_trans_handle *trans,
+					    struct btrfs_fs_info *fs_info,
+					    u64 objectid)
 {
 	struct extent_buffer *leaf;
 	struct btrfs_root *tree_root = fs_info->tree_root;
diff --git a/fsfeatures.c b/fsfeatures.c
index 7d85d60f1277..201d0f576688 100644
--- a/fsfeatures.c
+++ b/fsfeatures.c
@@ -225,3 +225,25 @@ u32 get_running_kernel_version(void)
 	return version;
 }
 
+int btrfs_check_nodesize(u32 nodesize, u32 sectorsize, u64 features)
+{
+	if (nodesize < sectorsize) {
+		error("illegal nodesize %u (smaller than %u)",
+				nodesize, sectorsize);
+		return -1;
+	} else if (nodesize > BTRFS_MAX_METADATA_BLOCKSIZE) {
+		error("illegal nodesize %u (larger than %u)",
+			nodesize, BTRFS_MAX_METADATA_BLOCKSIZE);
+		return -1;
+	} else if (nodesize & (sectorsize - 1)) {
+		error("illegal nodesize %u (not aligned to %u)",
+			nodesize, sectorsize);
+		return -1;
+	} else if (features & BTRFS_FEATURE_INCOMPAT_MIXED_GROUPS &&
+		   nodesize != sectorsize) {
+		error("illegal nodesize %u (not equal to %u for mixed block group)",
+			nodesize, sectorsize);
+		return -1;
+	}
+	return 0;
+}
diff --git a/rescue.h b/rescue.h
new file mode 100644
index 000000000000..c9f6e7b80b16
--- /dev/null
+++ b/rescue.h
@@ -0,0 +1,14 @@
+/*
+ * Copyright (C) 2018 SUSE.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+int btrfs_recover_superblocks(const char *path, int verbose, int yes);
+int btrfs_recover_chunk_tree(const char *path, int verbose, int yes);
diff --git a/super-recover.c b/super-recover.c
index 86b3df9867dc..a1af71786034 100644
--- a/super-recover.c
+++ b/super-recover.c
@@ -34,6 +34,7 @@
 #include "crc32c.h"
 #include "volumes.h"
 #include "commands.h"
+#include "rescue.h"
 
 struct btrfs_recover_superblock {
 	struct btrfs_fs_devices *fs_devices;
diff --git a/utils-lib.c b/utils-lib.c
index 044f93fc4446..5bb89f2f1a8d 100644
--- a/utils-lib.c
+++ b/utils-lib.c
@@ -1,4 +1,5 @@
 #include "kerncompat.h"
+#include "utils.h"
 #include <unistd.h>
 #include <stdlib.h>
 #include <limits.h>
diff --git a/utils.c b/utils.c
index b274f46fdd9d..a7e34b804551 100644
--- a/utils.c
+++ b/utils.c
@@ -2266,29 +2266,6 @@ int btrfs_tree_search2_ioctl_supported(int fd)
 	return ret;
 }
 
-int btrfs_check_nodesize(u32 nodesize, u32 sectorsize, u64 features)
-{
-	if (nodesize < sectorsize) {
-		error("illegal nodesize %u (smaller than %u)",
-				nodesize, sectorsize);
-		return -1;
-	} else if (nodesize > BTRFS_MAX_METADATA_BLOCKSIZE) {
-		error("illegal nodesize %u (larger than %u)",
-			nodesize, BTRFS_MAX_METADATA_BLOCKSIZE);
-		return -1;
-	} else if (nodesize & (sectorsize - 1)) {
-		error("illegal nodesize %u (not aligned to %u)",
-			nodesize, sectorsize);
-		return -1;
-	} else if (features & BTRFS_FEATURE_INCOMPAT_MIXED_GROUPS &&
-		   nodesize != sectorsize) {
-		error("illegal nodesize %u (not equal to %u for mixed block group)",
-			nodesize, sectorsize);
-		return -1;
-	}
-	return 0;
-}
-
 /*
  * Copy a path argument from SRC to DEST and check the SRC length if it's at
  * most PATH_MAX and fits into DEST. DESTLEN is supposed to be exact size of
-- 
2.19.1

Re: [PATCH 6/9] btrfs-progs: Fix Wsuggest-attribute=format warning

[Nikolay Borisov]

On 16.11.18 г. 9:54 ч., Qu Wenruo wrote:
> Add __attribute__ ((format (printf, 4, 0))) to fix the vprintf calling
> function.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  string-table.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/string-table.c b/string-table.c
> index 95833768960d..455285702d51 100644
> --- a/string-table.c
> +++ b/string-table.c
> @@ -48,6 +48,7 @@ struct string_table *table_create(int columns, int rows)
>   * '>' the text is right aligned. If fmt is equal to '=' the text will
>   * be replaced by a '=====' dimensioned on the basis of the column width
>   */
> +__attribute__ ((format (printf, 4, 0)))
>  char *table_vprintf(struct string_table *tab, int column, int row,
>  			  const char *fmt, va_list ap)
>  {
> 

Re: [PATCH 8/9] btrfs-progs: Fix Wtype-limits warning

[Nikolay Borisov]

On 16.11.18 г. 9:54 ч., Qu Wenruo wrote:
> The only hit is the following code:
> 
>  		tlv_len = le16_to_cpu(tlv_hdr->tlv_len);
> 
> 		if (tlv_type == 0 || tlv_type > BTRFS_SEND_A_MAX
> 		    || tlv_len > BTRFS_SEND_BUF_SIZE) {
>  			error("invalid tlv in cmd tlv_type = %hu, tlv_len = %hu",
>  					tlv_type, tlv_len);
> 
> @tlv_len is u16, while BTRFS_SEND_BUF_SIZE is 64K.
> u16 MAX is 64K - 1, so the final check is always false.
> 
> Just remove it.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

I had an identical patch:

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  send-stream.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/send-stream.c b/send-stream.c
> index 3b8e39c9486a..25461e92c37b 100644
> --- a/send-stream.c
> +++ b/send-stream.c
> @@ -157,8 +157,7 @@ static int read_cmd(struct btrfs_send_stream *sctx)
>  		tlv_type = le16_to_cpu(tlv_hdr->tlv_type);
>  		tlv_len = le16_to_cpu(tlv_hdr->tlv_len);
>  
> -		if (tlv_type == 0 || tlv_type > BTRFS_SEND_A_MAX
> -		    || tlv_len > BTRFS_SEND_BUF_SIZE) {
> +		if (tlv_type == 0 || tlv_type > BTRFS_SEND_A_MAX) {
>  			error("invalid tlv in cmd tlv_type = %hu, tlv_len = %hu",
>  					tlv_type, tlv_len);
>  			ret = -EINVAL;
> 

Re: [PATCH 5/9] btrfs-progs: Fix Wimplicit-fallthrough warning

[Qu Wenruo]

On 2018/11/16 下午4:04, Nikolay Borisov wrote:
> 
> 
> On 16.11.18 г. 9:54 ч., Qu Wenruo wrote:
>> Although most fallthrough case is pretty obvious, we still need to teach
>> the dumb compiler that it's an explicit fallthrough.
>>
>> Also reformat the code to use common indent.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
> 
> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> 
> Is this attribute dependent on a particular compiler version? Does
> gcc4.4 for example support it?

IIRC unsupported attribute should just be ignored by the compiler, so it
shouldn't cause any problem.

However I'm not 100% perfect sure, as my distribution doesn't provide
older gcc.

Thanks,
Qu

> 
>> ---
>>  utils.c | 30 ++++++++++++++++++++++--------
>>  1 file changed, 22 insertions(+), 8 deletions(-)
>>
>> diff --git a/utils.c b/utils.c
>> index a310300829eb..b274f46fdd9d 100644
>> --- a/utils.c
>> +++ b/utils.c
>> @@ -1134,15 +1134,25 @@ int pretty_size_snprintf(u64 size, char *str, size_t str_size, unsigned unit_mod
>>  	num_divs = 0;
>>  	last_size = size;
>>  	switch (unit_mode & UNITS_MODE_MASK) {
>> -	case UNITS_TBYTES: base *= mult; num_divs++;
>> -	case UNITS_GBYTES: base *= mult; num_divs++;
>> -	case UNITS_MBYTES: base *= mult; num_divs++;
>> -	case UNITS_KBYTES: num_divs++;
>> -			   break;
>> +	case UNITS_TBYTES:
>> +		base *= mult;
>> +		num_divs++;
>> +		__attribute__ ((fallthrough));
>> +	case UNITS_GBYTES:
>> +		base *= mult;
>> +		num_divs++;
>> +		__attribute__ ((fallthrough));
>> +	case UNITS_MBYTES:
>> +		base *= mult;
>> +		num_divs++;
>> +		__attribute__ ((fallthrough));
>> +	case UNITS_KBYTES:
>> +		num_divs++;
>> +		break;
>>  	case UNITS_BYTES:
>> -			   base = 1;
>> -			   num_divs = 0;
>> -			   break;
>> +		base = 1;
>> +		num_divs = 0;
>> +		break;
>>  	default:
>>  		if (negative) {
>>  			s64 ssize = (s64)size;
>> @@ -1907,13 +1917,17 @@ int test_num_disk_vs_raid(u64 metadata_profile, u64 data_profile,
>>  	default:
>>  	case 4:
>>  		allowed |= BTRFS_BLOCK_GROUP_RAID10;
>> +		__attribute__ ((fallthrough));
>>  	case 3:
>>  		allowed |= BTRFS_BLOCK_GROUP_RAID6;
>> +		__attribute__ ((fallthrough));
>>  	case 2:
>>  		allowed |= BTRFS_BLOCK_GROUP_RAID0 | BTRFS_BLOCK_GROUP_RAID1 |
>>  			BTRFS_BLOCK_GROUP_RAID5;
>> +		__attribute__ ((fallthrough));
>>  	case 1:
>>  		allowed |= BTRFS_BLOCK_GROUP_DUP;
>> +		__attribute__ ((fallthrough));
>>  	}
>>  
>>  	if (dev_cnt > 1 && profile & BTRFS_BLOCK_GROUP_DUP) {
>>

Re: [PATCH 7/9] btrfs-progs: Fix Wmaybe-uninitialized warning

[Nikolay Borisov]

On 16.11.18 г. 9:54 ч., Qu Wenruo wrote:
> The only location is the following code:
> 
> 	int level = path->lowest_level + 1;
>  	BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
>  	while(level < BTRFS_MAX_LEVEL) {
> 		slot = path->slots[level] + 1;
> 		...
> 	}
> 	path->slots[level] = slot;
> 
> Again, it's the stupid compiler needs some hint for the fact that
> we will always enter the while loop for at least once, thus @slot should
> always be initialized.
> 
> Fix it by assign level after the BUG_ON(), so the stupid compiler knows
> we will always go into the while loop.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

I don't get this warning on gcc 7.3. Also from your changelog it's not
really clear what the compiler is confused about.

Codewise lgtm:

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  ctree.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/ctree.c b/ctree.c
> index 46e2ccedc0bf..9c9cb0dfdbf2 100644
> --- a/ctree.c
> +++ b/ctree.c
> @@ -2961,11 +2961,12 @@ int btrfs_next_sibling_tree_block(struct btrfs_fs_info *fs_info,
>  				  struct btrfs_path *path)
>  {
>  	int slot;
> -	int level = path->lowest_level + 1;
> +	int level;
>  	struct extent_buffer *c;
>  	struct extent_buffer *next = NULL;
>  
>  	BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
> +	level = path->lowest_level + 1;
>  	while(level < BTRFS_MAX_LEVEL) {
>  		if (!path->nodes[level])
>  			return 1;
> 

Re: [PATCH v1.1 9/9] btrfs-progs: Cleanup warning reported by -Wmissing-prototypes

[Nikolay Borisov]

On 16.11.18 г. 10:04 ч., Qu Wenruo wrote:
> The following missing prototypes will be fixed:
> 1)  btrfs.c::handle_special_globals()
> 2)  check/mode-lowmem.c::repair_ternary_lowmem()
> 3)  extent-tree.c::btrfs_search_overlap_extent()
>     Above 3 can be fixed by making them static
> 
> 4)  utils.c::btrfs_check_nodesize()
>     Fixed by moving it to fsfeatures.c
> 
> 5)  chunk-recover.c::btrfs_recover_chunk_tree()
> 6)  super-recover.c::btrfs_recover_superblocks()
>     Fixed by moving the declaration from cmds-rescue.c to rescue.h
> 
> 7)  utils-lib.c::arg_strtou64()
> 8)  utils-lib.c::lookup_path_rootid()
>     Fixed by include "utils.h"
> 
> 9)  free-space-tree.c::set_free_space_tree_thresholds()
>     Fixed by deleting it, as there is no user.
> 
> 10) free-space-tree.c::convert_free_space_to_bitmaps()
> 11) free-space-tree.c::convert_free_space_to_extents()
> 12) free-space-tree.c::__remove_from_free_space_tree()
> 13) free-space-tree.c::__add_to_free_space_tree()
> 14) free-space-tree.c::btrfs_create_tree()
>     Fixed by making them static.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>


> ---
> changelog:
> v1.1
>    Update the subject, as we free space tree will also be cleaned up.
> ---
>  btrfs.c             |  2 +-
>  check/mode-lowmem.c |  6 ++---
>  chunk-recover.c     |  1 +
>  cmds-rescue.c       |  4 +--
>  extent-tree.c       |  2 +-
>  free-space-tree.c   | 59 ++++++++++++---------------------------------
>  fsfeatures.c        | 22 +++++++++++++++++
>  rescue.h            | 14 +++++++++++
>  super-recover.c     |  1 +
>  utils-lib.c         |  1 +
>  utils.c             | 23 ------------------
>  11 files changed, 60 insertions(+), 75 deletions(-)
>  create mode 100644 rescue.h
> 
> diff --git a/btrfs.c b/btrfs.c
> index 2d39f2ced3e8..78c468d2e050 100644
> --- a/btrfs.c
> +++ b/btrfs.c
> @@ -210,7 +210,7 @@ static int handle_global_options(int argc, char **argv)
>  	return shift;
>  }
>  
> -void handle_special_globals(int shift, int argc, char **argv)
> +static void handle_special_globals(int shift, int argc, char **argv)
>  {
>  	int has_help = 0;
>  	int has_full = 0;
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 14bbc9ee6cb6..f56b5e8d45dc 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -953,9 +953,9 @@ out:
>   * returns 0 means success.
>   * returns not 0 means on error;
>   */
> -int repair_ternary_lowmem(struct btrfs_root *root, u64 dir_ino, u64 ino,
> -			  u64 index, char *name, int name_len, u8 filetype,
> -			  int err)
> +static int repair_ternary_lowmem(struct btrfs_root *root, u64 dir_ino, u64 ino,
> +				 u64 index, char *name, int name_len,
> +				 u8 filetype, int err)
>  {
>  	struct btrfs_trans_handle *trans;
>  	int stage = 0;
> diff --git a/chunk-recover.c b/chunk-recover.c
> index 1d30db51d8ed..1e554b8e8750 100644
> --- a/chunk-recover.c
> +++ b/chunk-recover.c
> @@ -40,6 +40,7 @@
>  #include "utils.h"
>  #include "btrfsck.h"
>  #include "commands.h"
> +#include "rescue.h"
>  
>  struct recover_control {
>  	int verbose;
> diff --git a/cmds-rescue.c b/cmds-rescue.c
> index 2bc50c0841ed..36e9e1277e40 100644
> --- a/cmds-rescue.c
> +++ b/cmds-rescue.c
> @@ -26,15 +26,13 @@
>  #include "commands.h"
>  #include "utils.h"
>  #include "help.h"
> +#include "rescue.h"
>  
>  static const char * const rescue_cmd_group_usage[] = {
>  	"btrfs rescue <command> [options] <path>",
>  	NULL
>  };
>  
> -int btrfs_recover_chunk_tree(const char *path, int verbose, int yes);
> -int btrfs_recover_superblocks(const char *path, int verbose, int yes);
> -
>  static const char * const cmd_rescue_chunk_recover_usage[] = {
>  	"btrfs rescue chunk-recover [options] <device>",
>  	"Recover the chunk tree by scanning the devices one by one.",
> diff --git a/extent-tree.c b/extent-tree.c
> index cd98633992ac..8c9cdeff3b02 100644
> --- a/extent-tree.c
> +++ b/extent-tree.c
> @@ -3749,7 +3749,7 @@ static void __get_extent_size(struct btrfs_root *root, struct btrfs_path *path,
>   * Return >0 for not found.
>   * Return <0 for err
>   */
> -int btrfs_search_overlap_extent(struct btrfs_root *root,
> +static int btrfs_search_overlap_extent(struct btrfs_root *root,
>  				struct btrfs_path *path, u64 bytenr, u64 len)
>  {
>  	struct btrfs_key key;
> diff --git a/free-space-tree.c b/free-space-tree.c
> index 6641cdfa42ba..af141e6e611a 100644
> --- a/free-space-tree.c
> +++ b/free-space-tree.c
> @@ -24,35 +24,6 @@
>  #include "bitops.h"
>  #include "internal.h"
>  
> -void set_free_space_tree_thresholds(struct btrfs_block_group_cache *cache,
> -				    u64 sectorsize)
> -{
> -	u32 bitmap_range;
> -	size_t bitmap_size;
> -	u64 num_bitmaps, total_bitmap_size;
> -
> -	/*
> -	 * We convert to bitmaps when the disk space required for using extents
> -	 * exceeds that required for using bitmaps.
> -	 */
> -	bitmap_range = sectorsize * BTRFS_FREE_SPACE_BITMAP_BITS;
> -	num_bitmaps = div_u64(cache->key.offset + bitmap_range - 1,
> -			      bitmap_range);
> -	bitmap_size = sizeof(struct btrfs_item) + BTRFS_FREE_SPACE_BITMAP_SIZE;
> -	total_bitmap_size = num_bitmaps * bitmap_size;
> -	cache->bitmap_high_thresh = div_u64(total_bitmap_size,
> -					    sizeof(struct btrfs_item));
> -
> -	/*
> -	 * We allow for a small buffer between the high threshold and low
> -	 * threshold to avoid thrashing back and forth between the two formats.
> -	 */
> -	if (cache->bitmap_high_thresh > 100)
> -		cache->bitmap_low_thresh = cache->bitmap_high_thresh - 100;
> -	else
> -		cache->bitmap_low_thresh = 0;
> -}
> -
>  static struct btrfs_free_space_info *
>  search_free_space_info(struct btrfs_trans_handle *trans,
>  		       struct btrfs_fs_info *fs_info,
> @@ -202,9 +173,9 @@ static void le_bitmap_set(unsigned long *map, unsigned int start, int len)
>  	}
>  }
>  
> -int convert_free_space_to_bitmaps(struct btrfs_trans_handle *trans,
> -				  struct btrfs_block_group_cache *block_group,
> -				  struct btrfs_path *path)
> +static int convert_free_space_to_bitmaps(struct btrfs_trans_handle *trans,
> +				struct btrfs_block_group_cache *block_group,
> +				struct btrfs_path *path)
>  {
>  	struct btrfs_fs_info *fs_info = trans->fs_info;
>  	struct btrfs_root *root = fs_info->free_space_root;
> @@ -341,9 +312,9 @@ out:
>  	return ret;
>  }
>  
> -int convert_free_space_to_extents(struct btrfs_trans_handle *trans,
> -				  struct btrfs_block_group_cache *block_group,
> -				  struct btrfs_path *path)
> +static int convert_free_space_to_extents(struct btrfs_trans_handle *trans,
> +				struct btrfs_block_group_cache *block_group,
> +				struct btrfs_path *path)
>  {
>  	struct btrfs_fs_info *fs_info = trans->fs_info;
>  	struct btrfs_root *root = fs_info->free_space_root;
> @@ -780,9 +751,9 @@ out:
>  	return ret;
>  }
>  
> -int __remove_from_free_space_tree(struct btrfs_trans_handle *trans,
> -				  struct btrfs_block_group_cache *block_group,
> -				  struct btrfs_path *path, u64 start, u64 size)
> +static int __remove_from_free_space_tree(struct btrfs_trans_handle *trans,
> +				struct btrfs_block_group_cache *block_group,
> +				struct btrfs_path *path, u64 start, u64 size)
>  {
>  	struct btrfs_free_space_info *info;
>  	u32 flags;
> @@ -960,9 +931,9 @@ out:
>  	return ret;
>  }
>  
> -int __add_to_free_space_tree(struct btrfs_trans_handle *trans,
> -			     struct btrfs_block_group_cache *block_group,
> -			     struct btrfs_path *path, u64 start, u64 size)
> +static int __add_to_free_space_tree(struct btrfs_trans_handle *trans,
> +				struct btrfs_block_group_cache *block_group,
> +				struct btrfs_path *path, u64 start, u64 size)
>  {
>  	struct btrfs_fs_info *fs_info = trans->fs_info;
>  	struct btrfs_free_space_info *info;
> @@ -1420,9 +1391,9 @@ out:
>  	return ret;
>  }
>  
> -struct btrfs_root *btrfs_create_tree(struct btrfs_trans_handle *trans,
> -				     struct btrfs_fs_info *fs_info,
> -				     u64 objectid)
> +static struct btrfs_root *btrfs_create_tree(struct btrfs_trans_handle *trans,
> +					    struct btrfs_fs_info *fs_info,
> +					    u64 objectid)
>  {
>  	struct extent_buffer *leaf;
>  	struct btrfs_root *tree_root = fs_info->tree_root;
> diff --git a/fsfeatures.c b/fsfeatures.c
> index 7d85d60f1277..201d0f576688 100644
> --- a/fsfeatures.c
> +++ b/fsfeatures.c
> @@ -225,3 +225,25 @@ u32 get_running_kernel_version(void)
>  	return version;
>  }
>  
> +int btrfs_check_nodesize(u32 nodesize, u32 sectorsize, u64 features)
> +{
> +	if (nodesize < sectorsize) {
> +		error("illegal nodesize %u (smaller than %u)",
> +				nodesize, sectorsize);
> +		return -1;
> +	} else if (nodesize > BTRFS_MAX_METADATA_BLOCKSIZE) {
> +		error("illegal nodesize %u (larger than %u)",
> +			nodesize, BTRFS_MAX_METADATA_BLOCKSIZE);
> +		return -1;
> +	} else if (nodesize & (sectorsize - 1)) {
> +		error("illegal nodesize %u (not aligned to %u)",
> +			nodesize, sectorsize);
> +		return -1;
> +	} else if (features & BTRFS_FEATURE_INCOMPAT_MIXED_GROUPS &&
> +		   nodesize != sectorsize) {
> +		error("illegal nodesize %u (not equal to %u for mixed block group)",
> +			nodesize, sectorsize);
> +		return -1;
> +	}
> +	return 0;
> +}
> diff --git a/rescue.h b/rescue.h
> new file mode 100644
> index 000000000000..c9f6e7b80b16
> --- /dev/null
> +++ b/rescue.h
> @@ -0,0 +1,14 @@
> +/*
> + * Copyright (C) 2018 SUSE.  All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public
> + * License v2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + */
> +int btrfs_recover_superblocks(const char *path, int verbose, int yes);
> +int btrfs_recover_chunk_tree(const char *path, int verbose, int yes);
> diff --git a/super-recover.c b/super-recover.c
> index 86b3df9867dc..a1af71786034 100644
> --- a/super-recover.c
> +++ b/super-recover.c
> @@ -34,6 +34,7 @@
>  #include "crc32c.h"
>  #include "volumes.h"
>  #include "commands.h"
> +#include "rescue.h"
>  
>  struct btrfs_recover_superblock {
>  	struct btrfs_fs_devices *fs_devices;
> diff --git a/utils-lib.c b/utils-lib.c
> index 044f93fc4446..5bb89f2f1a8d 100644
> --- a/utils-lib.c
> +++ b/utils-lib.c
> @@ -1,4 +1,5 @@
>  #include "kerncompat.h"
> +#include "utils.h"
>  #include <unistd.h>
>  #include <stdlib.h>
>  #include <limits.h>
> diff --git a/utils.c b/utils.c
> index b274f46fdd9d..a7e34b804551 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -2266,29 +2266,6 @@ int btrfs_tree_search2_ioctl_supported(int fd)
>  	return ret;
>  }
>  
> -int btrfs_check_nodesize(u32 nodesize, u32 sectorsize, u64 features)
> -{
> -	if (nodesize < sectorsize) {
> -		error("illegal nodesize %u (smaller than %u)",
> -				nodesize, sectorsize);
> -		return -1;
> -	} else if (nodesize > BTRFS_MAX_METADATA_BLOCKSIZE) {
> -		error("illegal nodesize %u (larger than %u)",
> -			nodesize, BTRFS_MAX_METADATA_BLOCKSIZE);
> -		return -1;
> -	} else if (nodesize & (sectorsize - 1)) {
> -		error("illegal nodesize %u (not aligned to %u)",
> -			nodesize, sectorsize);
> -		return -1;
> -	} else if (features & BTRFS_FEATURE_INCOMPAT_MIXED_GROUPS &&
> -		   nodesize != sectorsize) {
> -		error("illegal nodesize %u (not equal to %u for mixed block group)",
> -			nodesize, sectorsize);
> -		return -1;
> -	}
> -	return 0;
> -}
> -
>  /*
>   * Copy a path argument from SRC to DEST and check the SRC length if it's at
>   * most PATH_MAX and fits into DEST. DESTLEN is supposed to be exact size of
> 

Re: [PATCH 7/9] btrfs-progs: Fix Wmaybe-uninitialized warning

[Qu Wenruo]

On 2018/11/16 下午4:13, Nikolay Borisov wrote:
> 
> 
> On 16.11.18 г. 9:54 ч., Qu Wenruo wrote:
>> The only location is the following code:
>>
>> 	int level = path->lowest_level + 1;
>>  	BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
>>  	while(level < BTRFS_MAX_LEVEL) {
>> 		slot = path->slots[level] + 1;
>> 		...
>> 	}
>> 	path->slots[level] = slot;
>>
>> Again, it's the stupid compiler needs some hint for the fact that
>> we will always enter the while loop for at least once, thus @slot should
>> always be initialized.
>>
>> Fix it by assign level after the BUG_ON(), so the stupid compiler knows
>> we will always go into the while loop.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
> 
> I don't get this warning on gcc 7.3. Also from your changelog it's not
> really clear what the compiler is confused about.

My gcc 8.2.1 reports it with "make W=1"

I'll just update the commit message with the original report:

  ctree.c: In function 'btrfs_next_sibling_tree_block':
  ctree.c:2990:21: warning: 'slot' may be used uninitialized in this
function [-Wmaybe-uninitialized]
    path->slots[level] = slot;
    ~~~~~~~~~~~~~~~~~~~^~~~~~

Thanks,
Qu

> 
> Codewise lgtm:
> 
> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> 
>> ---
>>  ctree.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/ctree.c b/ctree.c
>> index 46e2ccedc0bf..9c9cb0dfdbf2 100644
>> --- a/ctree.c
>> +++ b/ctree.c
>> @@ -2961,11 +2961,12 @@ int btrfs_next_sibling_tree_block(struct btrfs_fs_info *fs_info,
>>  				  struct btrfs_path *path)
>>  {
>>  	int slot;
>> -	int level = path->lowest_level + 1;
>> +	int level;
>>  	struct extent_buffer *c;
>>  	struct extent_buffer *next = NULL;
>>  
>>  	BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
>> +	level = path->lowest_level + 1;
>>  	while(level < BTRFS_MAX_LEVEL) {
>>  		if (!path->nodes[level])
>>  			return 1;
>>

[PATCH v1.1 7/9] btrfs-progs: Fix Wmaybe-uninitialized warning

[Qu Wenruo]

GCC 8.2.1 will report the following warning with "make W=1":

  ctree.c: In function 'btrfs_next_sibling_tree_block':
  ctree.c:2990:21: warning: 'slot' may be used uninitialized in this function [-Wmaybe-uninitialized]
    path->slots[level] = slot;
    ~~~~~~~~~~~~~~~~~~~^~~~~~

The culprit is the following code:

	int slot;		<< Not initialized
	int level = path->lowest_level + 1;
 	BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
 	while(level < BTRFS_MAX_LEVEL) {
		slot = path->slots[level] + 1;
		^^^^^^ but we initialize @slot here.
		...
	}
	path->slots[level] = slot;

Again, it's the stupid compiler needs some hint for the fact that
we will always enter the while loop for at least once, thus @slot should
always be initialized.

Fix it by assign level after the BUG_ON(), so the stupid compiler knows
we will always go into the while loop.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
changelog:
v1.1:
   Better commit message, with the original warning report and
   how compiler thinks this could be a problem.
---
 ctree.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ctree.c b/ctree.c
index 46e2ccedc0bf..9c9cb0dfdbf2 100644
--- a/ctree.c
+++ b/ctree.c
@@ -2961,11 +2961,12 @@ int btrfs_next_sibling_tree_block(struct btrfs_fs_info *fs_info,
 				  struct btrfs_path *path)
 {
 	int slot;
-	int level = path->lowest_level + 1;
+	int level;
 	struct extent_buffer *c;
 	struct extent_buffer *next = NULL;
 
 	BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
+	level = path->lowest_level + 1;
 	while(level < BTRFS_MAX_LEVEL) {
 		if (!path->nodes[level])
 			return 1;
-- 
2.19.1

Re: [PATCH v1.1 7/9] btrfs-progs: Fix Wmaybe-uninitialized warning

[Nikolay Borisov]

On 16.11.18 г. 10:22 ч., Qu Wenruo wrote:
> GCC 8.2.1 will report the following warning with "make W=1":
> 
>   ctree.c: In function 'btrfs_next_sibling_tree_block':
>   ctree.c:2990:21: warning: 'slot' may be used uninitialized in this function [-Wmaybe-uninitialized]
>     path->slots[level] = slot;
>     ~~~~~~~~~~~~~~~~~~~^~~~~~
> 
> The culprit is the following code:
> 
> 	int slot;		<< Not initialized
> 	int level = path->lowest_level + 1;
>  	BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
>  	while(level < BTRFS_MAX_LEVEL) {
> 		slot = path->slots[level] + 1;
> 		^^^^^^ but we initialize @slot here.
> 		...
> 	}
> 	path->slots[level] = slot;
> 
> Again, it's the stupid compiler needs some hint for the fact that
> we will always enter the while loop for at least once, thus @slot should
> always be initialized.
> 
> Fix it by assign level after the BUG_ON(), so the stupid compiler knows
> we will always go into the while loop.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Much better, thank you:

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
> changelog:
> v1.1:
>    Better commit message, with the original warning report and
>    how compiler thinks this could be a problem.
> ---
>  ctree.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/ctree.c b/ctree.c
> index 46e2ccedc0bf..9c9cb0dfdbf2 100644
> --- a/ctree.c
> +++ b/ctree.c
> @@ -2961,11 +2961,12 @@ int btrfs_next_sibling_tree_block(struct btrfs_fs_info *fs_info,
>  				  struct btrfs_path *path)
>  {
>  	int slot;
> -	int level = path->lowest_level + 1;
> +	int level;
>  	struct extent_buffer *c;
>  	struct extent_buffer *next = NULL;
>  
>  	BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
> +	level = path->lowest_level + 1;
>  	while(level < BTRFS_MAX_LEVEL) {
>  		if (!path->nodes[level])
>  			return 1;
> 

Re: [PATCH 1/9] btrfs-progs: Makefile.extrawarn: Import cc-disable-warning

[David Sterba]

On Fri, Nov 16, 2018 at 03:54:18PM +0800, Qu Wenruo wrote:
> We imported cc-option but forgot to import cc-disable-warning.
> 
> Fixes: b556a992c3ad ("btrfs-progs: build: allow to build with various compiler warnings")
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  Makefile.extrawarn | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Makefile.extrawarn b/Makefile.extrawarn
> index 1f4bda94a167..5849036fd166 100644
> --- a/Makefile.extrawarn
> +++ b/Makefile.extrawarn
> @@ -19,6 +19,12 @@ try-run = $(shell set -e;               \
>   cc-option = $(call try-run,\
>           $(CC) $(CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
>  
> +# cc-disable-warning
> +# Usage: cflags-y += $(call cc-disable-warning,unused-but-set-variable)
> +cc-disable-warning = $(call try-run,\
> +	$(CC) -Werror $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1)))

btrfs-progs don't use KBUILD_CPPFLAGS nor CC_OPTION_CFLAGS so this needs
to be properly ported from kernel.

Re: [PATCH 2/9] btrfs-progs: fix gcc8 default build warning caused by '-Wformat-truncation'

[David Sterba]

On Fri, Nov 16, 2018 at 03:54:19PM +0800, Qu Wenruo wrote:
> From: Su Yanjun <suyj.fnst@cn.fujitsu.com>
> 
> When using gcc8 compiles utils.c, it complains as below:
> 
> utils.c:852:45: warning: '%s' directive output may be truncated writing
> up to 4095 bytes into a region of size 4084 [-Wformat-truncation=]
>    snprintf(path, sizeof(path), "/dev/mapper/%s", name);
>                                              ^~   ~~~~
> In file included from /usr/include/stdio.h:873,
>                  from utils.c:20:
> /usr/include/bits/stdio2.h:67:10: note: '__builtin___snprintf_chk'
> output between 13 and 4108 bytes into a destination of size 4096
>    return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
>           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>         __bos (__s), __fmt, __va_arg_pack ());
>         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> This isn't a type of warning we care about, particularly when PATH_MAX
> is much less than either.
> 
> Using the GCC option -Wno-format-truncation to disable this for default
> build.
> 
> Signed-off-by: Su Yanjun <suyj.fnst@cn.fujitsu.com>
> [Use cc-disable-warning to fix the not working CFLAGS setting in configure.ac]
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  Makefile           | 1 +
>  Makefile.extrawarn | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index f4ab14ea74c8..252299f8869f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -49,6 +49,7 @@ CSCOPE_CMD := cscope -u -b -c -q
>  include Makefile.extrawarn
>  
>  EXTRA_CFLAGS :=
> +EXTRA_CFLAGS += $(call cc-disable-warning, format-truncation)

Please don't touch EXTRA_CFLAGS, this is for users who want to override
any defaults that are set by build. This should go to CFLAGS.

>  EXTRA_LDFLAGS :=
>  
>  DEBUG_CFLAGS_DEFAULT = -O0 -U_FORTIFY_SOURCE -ggdb3
> diff --git a/Makefile.extrawarn b/Makefile.extrawarn
> index 5849036fd166..bbb2d5173846 100644
> --- a/Makefile.extrawarn
> +++ b/Makefile.extrawarn
> @@ -53,6 +53,7 @@ warning-1 += -Wold-style-definition
>  warning-1 += $(call cc-option, -Wmissing-include-dirs)
>  warning-1 += $(call cc-option, -Wunused-but-set-variable)
>  warning-1 += $(call cc-disable-warning, missing-field-initializers)
> +warning-1 += $(call cc-disable-warning, format-truncation)

It's ok to disable for W=1 but eg. the W=3 level could take all
previously disabled warnings.

>  
>  warning-2 := -Waggregate-return
>  warning-2 += -Wcast-align
> -- 
> 2.19.1

Re: [PATCH v1.1 3/9] btrfs-progs: Makefile.extrawarn: Don't warn on sign compare

[David Sterba]

On Fri, Nov 16, 2018 at 04:00:40PM +0800, Qu Wenruo wrote:
> Under most case, we are just using 'int' for 'unsigned int', and doesn't
> care about the sign.
> 
> The Wsign-compare is causing tons of false alerts.
> Suppressing it would make W=1 less noisy.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> changelog:
> v1.1:
>    Use cc-disable-warning to provide much better compatibility against
>    older compiler
> ---
>  Makefile.extrawarn | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Makefile.extrawarn b/Makefile.extrawarn
> index bbb2d5173846..736bee82759f 100644
> --- a/Makefile.extrawarn
> +++ b/Makefile.extrawarn
> @@ -54,6 +54,7 @@ warning-1 += $(call cc-option, -Wmissing-include-dirs)
>  warning-1 += $(call cc-option, -Wunused-but-set-variable)
>  warning-1 += $(call cc-disable-warning, missing-field-initializers)
>  warning-1 += $(call cc-disable-warning, format-truncation)
> +warning-1 += $(call cc-disable-warning, sign-compare)

Please enable it for W=3

Re: [PATCH 7/9] btrfs-progs: Fix Wmaybe-uninitialized warning

[David Sterba]

On Fri, Nov 16, 2018 at 03:54:24PM +0800, Qu Wenruo wrote:
> The only location is the following code:
> 
> 	int level = path->lowest_level + 1;
>  	BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
>  	while(level < BTRFS_MAX_LEVEL) {
> 		slot = path->slots[level] + 1;
> 		...
> 	}
> 	path->slots[level] = slot;
> 
> Again, it's the stupid compiler needs some hint for the fact that
> we will always enter the while loop for at least once, thus @slot should
> always be initialized.

Harsh words for the compiler, and I say not deserved. The same code
pasted to kernel a built with the same version does not report the
warning, so it's apparently a missing annotation of BUG_ON in
btrfs-progs that does not give the right hint.

Re: [PATCH 2/9] btrfs-progs: fix gcc8 default build warning caused by '-Wformat-truncation'

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 2883 bytes --]



On 2018/12/4 下午7:10, David Sterba wrote:
> On Fri, Nov 16, 2018 at 03:54:19PM +0800, Qu Wenruo wrote:
>> From: Su Yanjun <suyj.fnst@cn.fujitsu.com>
>>
>> When using gcc8 compiles utils.c, it complains as below:
>>
>> utils.c:852:45: warning: '%s' directive output may be truncated writing
>> up to 4095 bytes into a region of size 4084 [-Wformat-truncation=]
>>    snprintf(path, sizeof(path), "/dev/mapper/%s", name);
>>                                              ^~   ~~~~
>> In file included from /usr/include/stdio.h:873,
>>                  from utils.c:20:
>> /usr/include/bits/stdio2.h:67:10: note: '__builtin___snprintf_chk'
>> output between 13 and 4108 bytes into a destination of size 4096
>>    return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
>>           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>         __bos (__s), __fmt, __va_arg_pack ());
>>         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> This isn't a type of warning we care about, particularly when PATH_MAX
>> is much less than either.
>>
>> Using the GCC option -Wno-format-truncation to disable this for default
>> build.
>>
>> Signed-off-by: Su Yanjun <suyj.fnst@cn.fujitsu.com>
>> [Use cc-disable-warning to fix the not working CFLAGS setting in configure.ac]
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>  Makefile           | 1 +
>>  Makefile.extrawarn | 1 +
>>  2 files changed, 2 insertions(+)
>>
>> diff --git a/Makefile b/Makefile
>> index f4ab14ea74c8..252299f8869f 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -49,6 +49,7 @@ CSCOPE_CMD := cscope -u -b -c -q
>>  include Makefile.extrawarn
>>  
>>  EXTRA_CFLAGS :=
>> +EXTRA_CFLAGS += $(call cc-disable-warning, format-truncation)
> 
> Please don't touch EXTRA_CFLAGS, this is for users who want to override
> any defaults that are set by build. This should go to CFLAGS.
> 
>>  EXTRA_LDFLAGS :=
>>  
>>  DEBUG_CFLAGS_DEFAULT = -O0 -U_FORTIFY_SOURCE -ggdb3
>> diff --git a/Makefile.extrawarn b/Makefile.extrawarn
>> index 5849036fd166..bbb2d5173846 100644
>> --- a/Makefile.extrawarn
>> +++ b/Makefile.extrawarn
>> @@ -53,6 +53,7 @@ warning-1 += -Wold-style-definition
>>  warning-1 += $(call cc-option, -Wmissing-include-dirs)
>>  warning-1 += $(call cc-option, -Wunused-but-set-variable)
>>  warning-1 += $(call cc-disable-warning, missing-field-initializers)
>> +warning-1 += $(call cc-disable-warning, format-truncation)
> 
> It's ok to disable for W=1 but eg. the W=3 level could take all
> previously disabled warnings.

Any guide on the warning disabling?

IMHO the format-truncation used in any snprintf()-like functions are
really more or less expected, just like missing-field-initializers.

THanks,
Qu

> 
>>  
>>  warning-2 := -Waggregate-return
>>  warning-2 += -Wcast-align
>> -- 
>> 2.19.1


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1.1 9/9] btrfs-progs: Cleanup warning reported by -Wmissing-prototypes

[David Sterba]

On Fri, Nov 16, 2018 at 04:04:51PM +0800, Qu Wenruo wrote:
> The following missing prototypes will be fixed:
> 1)  btrfs.c::handle_special_globals()
> 2)  check/mode-lowmem.c::repair_ternary_lowmem()
> 3)  extent-tree.c::btrfs_search_overlap_extent()
>     Above 3 can be fixed by making them static
> 
> 4)  utils.c::btrfs_check_nodesize()
>     Fixed by moving it to fsfeatures.c
> 
> 5)  chunk-recover.c::btrfs_recover_chunk_tree()
> 6)  super-recover.c::btrfs_recover_superblocks()
>     Fixed by moving the declaration from cmds-rescue.c to rescue.h
> 
> 7)  utils-lib.c::arg_strtou64()
> 8)  utils-lib.c::lookup_path_rootid()
>     Fixed by include "utils.h"
> 
> 9)  free-space-tree.c::set_free_space_tree_thresholds()
>     Fixed by deleting it, as there is no user.
> 
> 10) free-space-tree.c::convert_free_space_to_bitmaps()
> 11) free-space-tree.c::convert_free_space_to_extents()
> 12) free-space-tree.c::__remove_from_free_space_tree()
> 13) free-space-tree.c::__add_to_free_space_tree()
> 14) free-space-tree.c::btrfs_create_tree()
>     Fixed by making them static.

Please split this to more patches grouped by the type of change.

> --- /dev/null
> +++ b/rescue.h
> @@ -0,0 +1,14 @@
> +/*
> + * Copyright (C) 2018 SUSE.  All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public
> + * License v2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + */

Missing ifdef to prevent multiple inclusion

> +int btrfs_recover_superblocks(const char *path, int verbose, int yes);
> +int btrfs_recover_chunk_tree(const char *path, int verbose, int yes);

Re: [PATCH 7/9] btrfs-progs: Fix Wmaybe-uninitialized warning

[Adam Borowski]

On Tue, Dec 04, 2018 at 01:17:04PM +0100, David Sterba wrote:
> On Fri, Nov 16, 2018 at 03:54:24PM +0800, Qu Wenruo wrote:
> > The only location is the following code:
> > 
> > 	int level = path->lowest_level + 1;
> >  	BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
> >  	while(level < BTRFS_MAX_LEVEL) {
> > 		slot = path->slots[level] + 1;
> > 		...
> > 	}
> > 	path->slots[level] = slot;
> > 
> > Again, it's the stupid compiler needs some hint for the fact that
> > we will always enter the while loop for at least once, thus @slot should
> > always be initialized.
> 
> Harsh words for the compiler, and I say not deserved. The same code
> pasted to kernel a built with the same version does not report the
> warning, so it's apparently a missing annotation of BUG_ON in
> btrfs-progs that does not give the right hint.

It's be nice if the C language provided a kind of a while loop that executes
at least once...

-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢠⠒⠀⣿⡁ Ivan was a worldly man: born in St. Petersburg, raised in
⢿⡄⠘⠷⠚⠋⠀ Petrograd, lived most of his life in Leningrad, then returned
⠈⠳⣄⠀⠀⠀⠀ to the city of his birth to die.

Re: [PATCH v1.1 9/9] btrfs-progs: Cleanup warning reported by -Wmissing-prototypes

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 2144 bytes --]



On 2018/12/4 下午8:22, David Sterba wrote:
> On Fri, Nov 16, 2018 at 04:04:51PM +0800, Qu Wenruo wrote:
>> The following missing prototypes will be fixed:
>> 1)  btrfs.c::handle_special_globals()
>> 2)  check/mode-lowmem.c::repair_ternary_lowmem()
>> 3)  extent-tree.c::btrfs_search_overlap_extent()
>>     Above 3 can be fixed by making them static
>>
>> 4)  utils.c::btrfs_check_nodesize()
>>     Fixed by moving it to fsfeatures.c
>>
>> 5)  chunk-recover.c::btrfs_recover_chunk_tree()
>> 6)  super-recover.c::btrfs_recover_superblocks()
>>     Fixed by moving the declaration from cmds-rescue.c to rescue.h
>>
>> 7)  utils-lib.c::arg_strtou64()
>> 8)  utils-lib.c::lookup_path_rootid()
>>     Fixed by include "utils.h"
>>
>> 9)  free-space-tree.c::set_free_space_tree_thresholds()
>>     Fixed by deleting it, as there is no user.
>>
>> 10) free-space-tree.c::convert_free_space_to_bitmaps()
>> 11) free-space-tree.c::convert_free_space_to_extents()
>> 12) free-space-tree.c::__remove_from_free_space_tree()
>> 13) free-space-tree.c::__add_to_free_space_tree()
>> 14) free-space-tree.c::btrfs_create_tree()
>>     Fixed by making them static.
> 
> Please split this to more patches grouped by the type of change.

No problem, just as the numbering is already grouped.

Thanks,
Qu
> 
>> --- /dev/null
>> +++ b/rescue.h
>> @@ -0,0 +1,14 @@
>> +/*
>> + * Copyright (C) 2018 SUSE.  All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public
>> + * License v2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * General Public License for more details.
>> + */
> 
> Missing ifdef to prevent multiple inclusion
> 
>> +int btrfs_recover_superblocks(const char *path, int verbose, int yes);
>> +int btrfs_recover_chunk_tree(const char *path, int verbose, int yes);


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 7/9] btrfs-progs: Fix Wmaybe-uninitialized warning

[Nikolay Borisov]

On 4.12.18 г. 14:22 ч., Adam Borowski wrote:
> On Tue, Dec 04, 2018 at 01:17:04PM +0100, David Sterba wrote:
>> On Fri, Nov 16, 2018 at 03:54:24PM +0800, Qu Wenruo wrote:
>>> The only location is the following code:
>>>
>>> 	int level = path->lowest_level + 1;
>>>  	BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
>>>  	while(level < BTRFS_MAX_LEVEL) {
>>> 		slot = path->slots[level] + 1;
>>> 		...
>>> 	}
>>> 	path->slots[level] = slot;
>>>
>>> Again, it's the stupid compiler needs some hint for the fact that
>>> we will always enter the while loop for at least once, thus @slot should
>>> always be initialized.
>>
>> Harsh words for the compiler, and I say not deserved. The same code
>> pasted to kernel a built with the same version does not report the
>> warning, so it's apparently a missing annotation of BUG_ON in
>> btrfs-progs that does not give the right hint.
> 
> It's be nice if the C language provided a kind of a while loop that executes
> at least once...

But it does, it's called:

do {

} while()

> 

Re: [PATCH v1.1 9/9] btrfs-progs: Cleanup warning reported by -Wmissing-prototypes

[David Sterba]

On Tue, Dec 04, 2018 at 08:24:38PM +0800, Qu Wenruo wrote:
> 
> 
> On 2018/12/4 下午8:22, David Sterba wrote:
> > On Fri, Nov 16, 2018 at 04:04:51PM +0800, Qu Wenruo wrote:
> >> The following missing prototypes will be fixed:
> >> 1)  btrfs.c::handle_special_globals()
> >> 2)  check/mode-lowmem.c::repair_ternary_lowmem()
> >> 3)  extent-tree.c::btrfs_search_overlap_extent()
> >>     Above 3 can be fixed by making them static
> >>
> >> 4)  utils.c::btrfs_check_nodesize()
> >>     Fixed by moving it to fsfeatures.c
> >>
> >> 5)  chunk-recover.c::btrfs_recover_chunk_tree()
> >> 6)  super-recover.c::btrfs_recover_superblocks()
> >>     Fixed by moving the declaration from cmds-rescue.c to rescue.h
> >>
> >> 7)  utils-lib.c::arg_strtou64()
> >> 8)  utils-lib.c::lookup_path_rootid()
> >>     Fixed by include "utils.h"
> >>
> >> 9)  free-space-tree.c::set_free_space_tree_thresholds()
> >>     Fixed by deleting it, as there is no user.
> >>
> >> 10) free-space-tree.c::convert_free_space_to_bitmaps()
> >> 11) free-space-tree.c::convert_free_space_to_extents()
> >> 12) free-space-tree.c::__remove_from_free_space_tree()
> >> 13) free-space-tree.c::__add_to_free_space_tree()
> >> 14) free-space-tree.c::btrfs_create_tree()
> >>     Fixed by making them static.
> > 
> > Please split this to more patches grouped by the type of change.
> 
> No problem, just as the numbering is already grouped.

Note that 1-3 and 10-14 are the same group.

Re: [PATCH 7/9] btrfs-progs: Fix Wmaybe-uninitialized warning

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 1120 bytes --]



On 2018/12/4 下午8:17, David Sterba wrote:
> On Fri, Nov 16, 2018 at 03:54:24PM +0800, Qu Wenruo wrote:
>> The only location is the following code:
>>
>> 	int level = path->lowest_level + 1;
>>  	BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
>>  	while(level < BTRFS_MAX_LEVEL) {
>> 		slot = path->slots[level] + 1;
>> 		...
>> 	}
>> 	path->slots[level] = slot;
>>
>> Again, it's the stupid compiler needs some hint for the fact that
>> we will always enter the while loop for at least once, thus @slot should
>> always be initialized.
> 
> Harsh words for the compiler, and I say not deserved. The same code
> pasted to kernel a built with the same version does not report the
> warning, so it's apparently a missing annotation of BUG_ON in
> btrfs-progs that does not give the right hint.
> 
Well, in fact after the recent gcc8 updates (god knows how many versions
gcc8 get updated in Arch after the patchset), it doesn't report this
error anymore.

But your idea on the BUG_ON() lacking noreturn attribute makes sense.

I'll just add some hint for kerncompact.

Thanks,
Qu


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]