btrfs-progs: Make W=1 clean (no "again")

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 <[email protected]>

---
 Makefile.extrawarn | 6 ++++++
 1 file changed, 6 insertions(+)

Makefile.extrawarn +0 -0

index 1f4bda94a167..5849036fd166 100644

From: Su Yanjun <[email protected]>

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 <[email protected]>

[Use cc-disable-warning to fix the not working CFLAGS setting in configure.ac]

Signed-off-by Qu Wenruo <[email protected]>

---
 Makefile           | 1 +
 Makefile.extrawarn | 1 +
 2 files changed, 2 insertions(+)

Makefile +0 -0

index f4ab14ea74c8..252299f8869f 100644

Makefile.extrawarn +0 -0

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

Signature · 1 line

2.19.1

Quoted 36 lines · Qu Wenruo

From: Su Yanjun <[email protected]>

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 <[email protected]>

[Use cc-disable-warning to fix the not working CFLAGS setting in configure.ac]

Signed-off-by Qu Wenruo <[email protected]>

---
 Makefile           | 1 +
 Makefile.extrawarn | 1 +
 2 files changed, 2 insertions(+)

Makefile +0 -0

index f4ab14ea74c8..252299f8869f 100644

Makefile CSCOPE_CMD := cscope -u -b -c -q · line 52

Quoted 4 lines

Makefile +1 -0

@@ -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.

Quoted 2 lines

Makefile +0 -0

 EXTRA_LDFLAGS :=
 

Makefile.extrawarn warning-1 += -Wold-style-definition · line 56

Quoted 10 lines

Makefile +0 -0

 DEBUG_CFLAGS_DEFAULT = -O0 -U_FORTIFY_SOURCE -ggdb3

Makefile.extrawarn +1 -0

index 5849036fd166..bbb2d5173846 100644
@@ -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.

Quoted 6 lines

Makefile.extrawarn +0 -0

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

Signature · 1 line

2.19.1

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 <[email protected]>

---
changelog:
v1.1:
Use cc-disable-warning to provide much better compatibility against older compiler

---
 Makefile.extrawarn | 1 +
 1 file changed, 1 insertion(+)

Makefile.extrawarn +0 -0

index bbb2d5173846..736bee82759f 100644

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 <[email protected]>

---
 messages.h | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

messages.h +10 -5

index ec7d93381a36..16f650d19a4b 100644
@@ -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)
 

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 <[email protected]>

---
 utils.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

utils.c +22 -8

index a310300829eb..b274f46fdd9d 100644
@@ -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) {

Quoted 3 lines · Qu Wenruo

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

Quoted 3 lines

Also reformat the code to use common indent.

Signed-off-by Qu Wenruo <[email protected]>

Reviewed-by Nikolay Borisov <[email protected]>

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

Quoted 62 lines

---
 utils.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

utils.c +22 -8

index a310300829eb..b274f46fdd9d 100644
@@ -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) {

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

Signed-off-by Qu Wenruo <[email protected]>

---
 string-table.c | 1 +
 1 file changed, 1 insertion(+)

string-table.c +1 -0

index 95833768960d..455285702d51 100644
@@ -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)
 {

The only location is the following code:

	int level = path->lowest_level + 1;

ctree.c +0 -0

 	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 <[email protected]>

---
 ctree.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

ctree.c +2 -1

index 46e2ccedc0bf..9c9cb0dfdbf2 100644
@@ -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;

Quoted 14 lines · Qu Wenruo

The only location is the following code:

	int level = path->lowest_level + 1;

ctree.c +0 -0

 	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.

Quoted 4 lines

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 <[email protected]>

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 <[email protected]>

Quoted 23 lines

---
 ctree.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

ctree.c +2 -1

index 46e2ccedc0bf..9c9cb0dfdbf2 100644
@@ -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;

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;

ctree.c +0 -0

 	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 <[email protected]>

---
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(-)

ctree.c +2 -1

index 46e2ccedc0bf..9c9cb0dfdbf2 100644
@@ -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;

The only location is the following code:

	int level = path->lowest_level + 1;

ctree.c +0 -0

 	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.

Quoted 10 lines · Qu Wenruo

The only location is the following code:

	int level = path->lowest_level + 1;

ctree.c +0 -0

 	BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
 	while(level < BTRFS_MAX_LEVEL) {

		slot = path->slots[level] + 1;
		...
	}
	path->slots[level] = slot;

Quoted 3 lines

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.

Quoted 15 lines · David Sterba

Quoted 13 lines · Qu Wenruo

The only location is the following code:

	int level = path->lowest_level + 1;

ctree.c +0 -0

 	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.

Quoted 4 lines

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...

Quoted 10 lines · Qu Wenruo

The only location is the following code:

	int level = path->lowest_level + 1;

ctree.c +0 -0

 	BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
 	while(level < BTRFS_MAX_LEVEL) {

		slot = path->slots[level] + 1;
		...
	}
	path->slots[level] = slot;

Quoted 3 lines

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.

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) {

send-stream.c +0 -0

 			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 <[email protected]>

---
 send-stream.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

send-stream.c +1 -2

index 3b8e39c9486a..25461e92c37b 100644
@@ -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;

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 <[email protected]>

---
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

btrfs.c +1 -1

index 2d39f2ced3e8..78c468d2e050 100644
@@ -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;

check/mode-lowmem.c +3 -3

index 14bbc9ee6cb6..f56b5e8d45dc 100644
@@ -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;

chunk-recover.c +1 -0

index 1d30db51d8ed..1e554b8e8750 100644
@@ -40,6 +40,7 @@
 #include "utils.h"
 #include "btrfsck.h"
 #include "commands.h"
+#include "rescue.h"
 
 struct recover_control {
 	int verbose;

cmds-rescue.c +1 -3

index 2bc50c0841ed..36e9e1277e40 100644
@@ -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.",

extent-tree.c +1 -1

index cd98633992ac..8c9cdeff3b02 100644
@@ -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;

free-space-tree.c +15 -44

index 6641cdfa42ba..af141e6e611a 100644
@@ -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;

fsfeatures.c +22 -0

index 7d85d60f1277..201d0f576688 100644
@@ -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;
+}

rescue.h new file +14 -0

new file mode 100644
index 000000000000..c9f6e7b80b16
@@ -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);

super-recover.c +1 -0

index 86b3df9867dc..a1af71786034 100644
@@ -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;

utils-lib.c +1 -0

index 044f93fc4446..5bb89f2f1a8d 100644
@@ -1,4 +1,5 @@
 #include "kerncompat.h"
+#include "utils.h"
 #include <unistd.h>
 #include <stdlib.h>
 #include <limits.h>

utils.c +0 -23

index b274f46fdd9d..a7e34b804551 100644
@@ -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

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.

rescue.h new file +2 -0

@@ -0,0 +1,14 @@
+/*
+ * Copyright (C) 2018 SUSE.  All rights reserved.

rescue.h new file +2 -0

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

Quoted 20 lines · 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.

Quoted 6 lines

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.

Quoted 6 lines

rescue.h new file +2 -0

@@ -0,0 +1,14 @@
+/*
+ * Copyright (C) 2018 SUSE.  All rights reserved.

rescue.h line 12

Quoted 10 lines

rescue.h new file +10 -0

+ *
+ * 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

Quoted 2 lines

rescue.h new file +2 -0

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

Quoted 28 lines · David Sterba

Quoted 26 lines · 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.

Quoted 1 line

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

No problem, just as the numbering is already grouped.