* [PATCH v2 00/13] btrfs-progs: Make W=1 great (no "again")
@ 2018-12-05 6:40 Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 01/13] btrfs-progs: Makefile.extrawarn: Import cc-disable-warning Qu Wenruo
` (12 more replies)
0 siblings, 13 replies; 19+ messages in thread
From: Qu Wenruo @ 2018-12-05 6:40 UTC (permalink / raw)
To: linux-btrfs
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" great (may 'again' or not, depending on the
distribution rolling speed).
changelog:
v1.1:
- Use cc-disable-warning instead of putting -Wno-something to improve
compatibility.
- Better explaination on the BUG_ON() branch caused uninitialized
variable.
- Also cleanup free-space-tree.c
v2:
- Add reviewed-by tags, except the 7th patch, as it goes a different way
to fix in v2.
- Fix bad port of cc-disable-warning, using $CFLAGS instead of kernel
flags.
- Make sure fixed warnings still show in W=3.
- Use do {} while() loop to replace a while() {} loop, so even compiler
doesn't have enough hint for BUG_ON(), it won't report uninitialized
variable warning.
Qu Wenruo (12):
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: Fix missing-prototypes warning caused by non-static
functions
btrfs-progs: Move btrfs_check_nodesize to fsfeatures.c to fix
missing-prototypes warning
btrfs-progs: Introduce rescue.h to resolve missing-prototypes for
chunk and super rescue
btrfs-progs: Add utils.h include to solve missing-prototypes warning
btrfs-progs: free-space-tree: Remove unsued function
Su Yanjun (1):
btrfs-progs: fix gcc8 default build warning caused by
'-Wformat-truncation'
Makefile | 5 ++++
Makefile.extrawarn | 10 ++++++++
btrfs.c | 2 +-
check/mode-lowmem.c | 6 ++---
chunk-recover.c | 1 +
cmds-rescue.c | 4 +--
ctree.c | 4 +--
extent-tree.c | 2 +-
free-space-tree.c | 59 ++++++++++++---------------------------------
fsfeatures.c | 23 ++++++++++++++++++
messages.h | 15 ++++++++----
rescue.h | 21 ++++++++++++++++
send-stream.c | 3 +--
string-table.c | 1 +
super-recover.c | 1 +
utils-lib.c | 1 +
utils.c | 53 +++++++++++++++++-----------------------
17 files changed, 119 insertions(+), 92 deletions(-)
create mode 100644 rescue.h
--
2.19.2
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 01/13] btrfs-progs: Makefile.extrawarn: Import cc-disable-warning
2018-12-05 6:40 [PATCH v2 00/13] btrfs-progs: Make W=1 great (no "again") Qu Wenruo
@ 2018-12-05 6:40 ` Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 02/13] btrfs-progs: fix gcc8 default build warning caused by '-Wformat-truncation' Qu Wenruo
` (11 subsequent siblings)
12 siblings, 0 replies; 19+ messages in thread
From: Qu Wenruo @ 2018-12-05 6:40 UTC (permalink / raw)
To: linux-btrfs
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>
---
| 6 ++++++
1 file changed, 6 insertions(+)
--git a/Makefile.extrawarn b/Makefile.extrawarn
index 1f4bda94a167..18a3a860053e 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 $(CFLAGS) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1)))
+
+
# From linux.git/scripts/Makefile.extrawarn
# ==========================================================================
#
--
2.19.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 02/13] btrfs-progs: fix gcc8 default build warning caused by '-Wformat-truncation'
2018-12-05 6:40 [PATCH v2 00/13] btrfs-progs: Make W=1 great (no "again") Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 01/13] btrfs-progs: Makefile.extrawarn: Import cc-disable-warning Qu Wenruo
@ 2018-12-05 6:40 ` Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 03/13] btrfs-progs: Makefile.extrawarn: Don't warn on sign compare Qu Wenruo
` (10 subsequent siblings)
12 siblings, 0 replies; 19+ messages in thread
From: Qu Wenruo @ 2018-12-05 6:40 UTC (permalink / raw)
To: linux-btrfs; +Cc: Su Yanjun
From: Su Yanjun <suyj.fnst@cn.fujitsu.com>
When using gcc8 + glibc 2.28.5 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 calling
snprintf() we expect string to be truncated.
Using the GCC option -Wno-format-truncation to disable this for default
build and W=1 build, while still keep it for W=2/W=3 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]
[Keep the warning in W=2/W=3 build]
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
Makefile | 5 +++++
| 2 ++
2 files changed, 7 insertions(+)
diff --git a/Makefile b/Makefile
index f4ab14ea74c8..a9e57fecb6e6 100644
--- a/Makefile
+++ b/Makefile
@@ -62,6 +62,10 @@ DEBUG_LDFLAGS :=
ABSTOPDIR = $(shell pwd)
TOPDIR := .
+# Disable certain GCC 8 + glibc 2.28 warning for snprintf()
+# where string truncation for snprintf() is expected.
+DISABLE_WARNING_FLAGS := $(call cc-disable-warning, format-truncation)
+
# Common build flags
CFLAGS = $(SUBST_CFLAGS) \
$(CSTD) \
@@ -73,6 +77,7 @@ CFLAGS = $(SUBST_CFLAGS) \
-I$(TOPDIR) \
-I$(TOPDIR)/kernel-lib \
-I$(TOPDIR)/libbtrfsutil \
+ $(DISABLE_WARNING_FLAGS) \
$(EXTRAWARN_CFLAGS) \
$(DEBUG_CFLAGS_INTERNAL) \
$(EXTRA_CFLAGS)
--git a/Makefile.extrawarn b/Makefile.extrawarn
index 18a3a860053e..0c11f2450802 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
@@ -61,6 +62,7 @@ warning-2 += -Wnested-externs
warning-2 += -Wshadow
warning-2 += $(call cc-option, -Wlogical-op)
warning-2 += $(call cc-option, -Wmissing-field-initializers)
+warning-2 += $(call cc-option, -Wformat-truncation)
warning-3 := -Wbad-function-cast
warning-3 += -Wcast-qual
--
2.19.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 03/13] btrfs-progs: Makefile.extrawarn: Don't warn on sign compare
2018-12-05 6:40 [PATCH v2 00/13] btrfs-progs: Make W=1 great (no "again") Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 01/13] btrfs-progs: Makefile.extrawarn: Import cc-disable-warning Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 02/13] btrfs-progs: fix gcc8 default build warning caused by '-Wformat-truncation' Qu Wenruo
@ 2018-12-05 6:40 ` Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 04/13] btrfs-progs: Fix Wempty-body warning Qu Wenruo
` (9 subsequent siblings)
12 siblings, 0 replies; 19+ messages in thread
From: Qu Wenruo @ 2018-12-05 6:40 UTC (permalink / raw)
To: linux-btrfs
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 so we can focus on real
problem, while still allow it in W=3 build.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
| 2 ++
1 file changed, 2 insertions(+)
--git a/Makefile.extrawarn b/Makefile.extrawarn
index 0c11f2450802..9b4cace01ce4 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
@@ -74,6 +75,7 @@ warning-3 += -Wredundant-decls
warning-3 += -Wswitch-default
warning-3 += $(call cc-option, -Wpacked-bitfield-compat)
warning-3 += $(call cc-option, -Wvla)
+warning-3 += $(call cc-option, -Wsign-compare)
warning := $(warning-$(findstring 1, $(BUILD_ENABLE_EXTRA_GCC_CHECKS)))
warning += $(warning-$(findstring 2, $(BUILD_ENABLE_EXTRA_GCC_CHECKS)))
--
2.19.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 04/13] btrfs-progs: Fix Wempty-body warning
2018-12-05 6:40 [PATCH v2 00/13] btrfs-progs: Make W=1 great (no "again") Qu Wenruo
` (2 preceding siblings ...)
2018-12-05 6:40 ` [PATCH v2 03/13] btrfs-progs: Makefile.extrawarn: Don't warn on sign compare Qu Wenruo
@ 2018-12-05 6:40 ` Qu Wenruo
2018-12-13 18:59 ` David Sterba
2018-12-05 6:40 ` [PATCH v2 05/13] btrfs-progs: Fix Wimplicit-fallthrough warning Qu Wenruo
` (8 subsequent siblings)
12 siblings, 1 reply; 19+ messages in thread
From: Qu Wenruo @ 2018-12-05 6:40 UTC (permalink / raw)
To: linux-btrfs
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)
--
2.19.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 05/13] btrfs-progs: Fix Wimplicit-fallthrough warning
2018-12-05 6:40 [PATCH v2 00/13] btrfs-progs: Make W=1 great (no "again") Qu Wenruo
` (3 preceding siblings ...)
2018-12-05 6:40 ` [PATCH v2 04/13] btrfs-progs: Fix Wempty-body warning Qu Wenruo
@ 2018-12-05 6:40 ` Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 06/13] btrfs-progs: Fix Wsuggest-attribute=format warning Qu Wenruo
` (7 subsequent siblings)
12 siblings, 0 replies; 19+ messages in thread
From: Qu Wenruo @ 2018-12-05 6:40 UTC (permalink / raw)
To: linux-btrfs
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>
---
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.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 06/13] btrfs-progs: Fix Wsuggest-attribute=format warning
2018-12-05 6:40 [PATCH v2 00/13] btrfs-progs: Make W=1 great (no "again") Qu Wenruo
` (4 preceding siblings ...)
2018-12-05 6:40 ` [PATCH v2 05/13] btrfs-progs: Fix Wimplicit-fallthrough warning Qu Wenruo
@ 2018-12-05 6:40 ` Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 07/13] btrfs-progs: Fix Wmaybe-uninitialized warning Qu Wenruo
` (6 subsequent siblings)
12 siblings, 0 replies; 19+ messages in thread
From: Qu Wenruo @ 2018-12-05 6:40 UTC (permalink / raw)
To: linux-btrfs
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)
{
--
2.19.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 07/13] btrfs-progs: Fix Wmaybe-uninitialized warning
2018-12-05 6:40 [PATCH v2 00/13] btrfs-progs: Make W=1 great (no "again") Qu Wenruo
` (5 preceding siblings ...)
2018-12-05 6:40 ` [PATCH v2 06/13] btrfs-progs: Fix Wsuggest-attribute=format warning Qu Wenruo
@ 2018-12-05 6:40 ` Qu Wenruo
2018-12-05 13:40 ` David Sterba
2018-12-05 6:40 ` [PATCH v2 08/13] btrfs-progs: Fix Wtype-limits warning Qu Wenruo
` (5 subsequent siblings)
12 siblings, 1 reply; 19+ messages in thread
From: Qu Wenruo @ 2018-12-05 6:40 UTC (permalink / raw)
To: linux-btrfs
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;
It's possible that compiler doesn't get enough hint for BUG_ON() on
lowest_level + 1 >= BTRFS_MAX_LEVEL case.
Fix it by using a do {} while() loop other than while() {} loop, to
ensure we will run the loop for at least once.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
ctree.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ctree.c b/ctree.c
index 46e2ccedc0bf..867e8b60b199 100644
--- a/ctree.c
+++ b/ctree.c
@@ -2966,7 +2966,7 @@ int btrfs_next_sibling_tree_block(struct btrfs_fs_info *fs_info,
struct extent_buffer *next = NULL;
BUG_ON(path->lowest_level + 1 >= BTRFS_MAX_LEVEL);
- while(level < BTRFS_MAX_LEVEL) {
+ do {
if (!path->nodes[level])
return 1;
@@ -2986,7 +2986,7 @@ int btrfs_next_sibling_tree_block(struct btrfs_fs_info *fs_info,
if (!extent_buffer_uptodate(next))
return -EIO;
break;
- }
+ } while (level < BTRFS_MAX_LEVEL);
path->slots[level] = slot;
while(1) {
level--;
--
2.19.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 08/13] btrfs-progs: Fix Wtype-limits warning
2018-12-05 6:40 [PATCH v2 00/13] btrfs-progs: Make W=1 great (no "again") Qu Wenruo
` (6 preceding siblings ...)
2018-12-05 6:40 ` [PATCH v2 07/13] btrfs-progs: Fix Wmaybe-uninitialized warning Qu Wenruo
@ 2018-12-05 6:40 ` Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 09/13] btrfs-progs: Fix missing-prototypes warning caused by non-static functions Qu Wenruo
` (4 subsequent siblings)
12 siblings, 0 replies; 19+ messages in thread
From: Qu Wenruo @ 2018-12-05 6:40 UTC (permalink / raw)
To: linux-btrfs
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>
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;
--
2.19.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 09/13] btrfs-progs: Fix missing-prototypes warning caused by non-static functions
2018-12-05 6:40 [PATCH v2 00/13] btrfs-progs: Make W=1 great (no "again") Qu Wenruo
` (7 preceding siblings ...)
2018-12-05 6:40 ` [PATCH v2 08/13] btrfs-progs: Fix Wtype-limits warning Qu Wenruo
@ 2018-12-05 6:40 ` Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 10/13] btrfs-progs: Move btrfs_check_nodesize to fsfeatures.c to fix missing-prototypes warning Qu Wenruo
` (3 subsequent siblings)
12 siblings, 0 replies; 19+ messages in thread
From: Qu Wenruo @ 2018-12-05 6:40 UTC (permalink / raw)
To: linux-btrfs
Make the following functions static to avoid missing-prototypes warning:
- btrfs.c::handle_special_globals()
- check/mode-lowmem.c::repair_ternary_lowmem()
- extent-tree.c::btrfs_search_overlap_extent()
- free-space-tree.c::convert_free_space_to_bitmaps()
- free-space-tree.c::convert_free_space_to_extents()
- free-space-tree.c::__remove_from_free_space_tree()
- free-space-tree.c::__add_to_free_space_tree()
- free-space-tree.c::btrfs_create_tree()
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
---
btrfs.c | 2 +-
check/mode-lowmem.c | 6 +++---
extent-tree.c | 2 +-
free-space-tree.c | 30 +++++++++++++++---------------
4 files changed, 20 insertions(+), 20 deletions(-)
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/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..b3ffa90f704c 100644
--- a/free-space-tree.c
+++ b/free-space-tree.c
@@ -202,9 +202,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 +341,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 +780,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 +960,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 +1420,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;
--
2.19.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 10/13] btrfs-progs: Move btrfs_check_nodesize to fsfeatures.c to fix missing-prototypes warning
2018-12-05 6:40 [PATCH v2 00/13] btrfs-progs: Make W=1 great (no "again") Qu Wenruo
` (8 preceding siblings ...)
2018-12-05 6:40 ` [PATCH v2 09/13] btrfs-progs: Fix missing-prototypes warning caused by non-static functions Qu Wenruo
@ 2018-12-05 6:40 ` Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 11/13] btrfs-progs: Introduce rescue.h to resolve missing-prototypes for chunk and super rescue Qu Wenruo
` (2 subsequent siblings)
12 siblings, 0 replies; 19+ messages in thread
From: Qu Wenruo @ 2018-12-05 6:40 UTC (permalink / raw)
To: linux-btrfs
And fsfeatures.c is indeed a better location for that function.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
---
fsfeatures.c | 23 +++++++++++++++++++++++
utils.c | 23 -----------------------
2 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/fsfeatures.c b/fsfeatures.c
index 7d85d60f1277..13ad030870cd 100644
--- a/fsfeatures.c
+++ b/fsfeatures.c
@@ -225,3 +225,26 @@ 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/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.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 11/13] btrfs-progs: Introduce rescue.h to resolve missing-prototypes for chunk and super rescue
2018-12-05 6:40 [PATCH v2 00/13] btrfs-progs: Make W=1 great (no "again") Qu Wenruo
` (9 preceding siblings ...)
2018-12-05 6:40 ` [PATCH v2 10/13] btrfs-progs: Move btrfs_check_nodesize to fsfeatures.c to fix missing-prototypes warning Qu Wenruo
@ 2018-12-05 6:40 ` Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 12/13] btrfs-progs: Add utils.h include to solve missing-prototypes warning Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 13/13] btrfs-progs: free-space-tree: Remove unsued function Qu Wenruo
12 siblings, 0 replies; 19+ messages in thread
From: Qu Wenruo @ 2018-12-05 6:40 UTC (permalink / raw)
To: linux-btrfs
We don't have any header declaring btrfs_recover_chunk_tree() nor
btrfs_recover_superblocks(), thus W=1 gives missing-prototypes warning
on them.
Fix it by introducing a new header, rescue.h for these two functions, so
make W=1 could be much happier.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
---
chunk-recover.c | 1 +
cmds-rescue.c | 4 +---
rescue.h | 21 +++++++++++++++++++++
super-recover.c | 1 +
4 files changed, 24 insertions(+), 3 deletions(-)
create mode 100644 rescue.h
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/rescue.h b/rescue.h
new file mode 100644
index 000000000000..de486e2e2004
--- /dev/null
+++ b/rescue.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * 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.
+ */
+
+#ifndef __BTRFS_RESCUE_H__
+#define __BTRFS_RESCUE_H__
+
+int btrfs_recover_superblocks(const char *path, int verbose, int yes);
+int btrfs_recover_chunk_tree(const char *path, int verbose, int yes);
+
+#endif
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;
--
2.19.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 12/13] btrfs-progs: Add utils.h include to solve missing-prototypes warning
2018-12-05 6:40 [PATCH v2 00/13] btrfs-progs: Make W=1 great (no "again") Qu Wenruo
` (10 preceding siblings ...)
2018-12-05 6:40 ` [PATCH v2 11/13] btrfs-progs: Introduce rescue.h to resolve missing-prototypes for chunk and super rescue Qu Wenruo
@ 2018-12-05 6:40 ` Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 13/13] btrfs-progs: free-space-tree: Remove unsued function Qu Wenruo
12 siblings, 0 replies; 19+ messages in thread
From: Qu Wenruo @ 2018-12-05 6:40 UTC (permalink / raw)
To: linux-btrfs
Prototypes for arg_strtou64() and lookup_path_rootid() are included in
utils.c, resulting make W=1 warning for them.
Just include that header to make W=1 happier.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
---
utils-lib.c | 1 +
1 file changed, 1 insertion(+)
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>
--
2.19.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 13/13] btrfs-progs: free-space-tree: Remove unsued function
2018-12-05 6:40 [PATCH v2 00/13] btrfs-progs: Make W=1 great (no "again") Qu Wenruo
` (11 preceding siblings ...)
2018-12-05 6:40 ` [PATCH v2 12/13] btrfs-progs: Add utils.h include to solve missing-prototypes warning Qu Wenruo
@ 2018-12-05 6:40 ` Qu Wenruo
12 siblings, 0 replies; 19+ messages in thread
From: Qu Wenruo @ 2018-12-05 6:40 UTC (permalink / raw)
To: linux-btrfs
set_free_space_tree_thresholds() is never used, just remove it to solve
the missing-prototypes warning from make W=1.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
---
free-space-tree.c | 29 -----------------------------
1 file changed, 29 deletions(-)
diff --git a/free-space-tree.c b/free-space-tree.c
index b3ffa90f704c..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,
--
2.19.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 07/13] btrfs-progs: Fix Wmaybe-uninitialized warning
2018-12-05 6:40 ` [PATCH v2 07/13] btrfs-progs: Fix Wmaybe-uninitialized warning Qu Wenruo
@ 2018-12-05 13:40 ` David Sterba
2018-12-05 14:03 ` Qu Wenruo
0 siblings, 1 reply; 19+ messages in thread
From: David Sterba @ 2018-12-05 13:40 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Wed, Dec 05, 2018 at 02:40:12PM +0800, 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;
>
> It's possible that compiler doesn't get enough hint for BUG_ON() on
> lowest_level + 1 >= BTRFS_MAX_LEVEL case.
>
> Fix it by using a do {} while() loop other than while() {} loop, to
> ensure we will run the loop for at least once.
I was hoping that we can actually add the hint to BUG_ON so the code
does not continue if the condition is true.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 07/13] btrfs-progs: Fix Wmaybe-uninitialized warning
2018-12-05 13:40 ` David Sterba
@ 2018-12-05 14:03 ` Qu Wenruo
2019-01-15 17:42 ` David Sterba
0 siblings, 1 reply; 19+ messages in thread
From: Qu Wenruo @ 2018-12-05 14:03 UTC (permalink / raw)
To: dsterba, Qu Wenruo, linux-btrfs
[-- Attachment #1.1: Type: text/plain, Size: 1713 bytes --]
On 2018/12/5 下午9:40, David Sterba wrote:
> On Wed, Dec 05, 2018 at 02:40:12PM +0800, 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;
>>
>> It's possible that compiler doesn't get enough hint for BUG_ON() on
>> lowest_level + 1 >= BTRFS_MAX_LEVEL case.
>>
>> Fix it by using a do {} while() loop other than while() {} loop, to
>> ensure we will run the loop for at least once.
>
> I was hoping that we can actually add the hint to BUG_ON so the code
> does not continue if the condition is true.
>
I checked that method, but I'm not that confident about things like:
bugon_trace()
{
if (!val)
return;
__bugon_trace();
}
__attribute__ ((noreturn))
static inline void __bugon_trace();
This is as simple as just one extra function call, but the original
problem is just one more function call before hitting abort().
So I just give up screwing up things I'm not comfort enough to tweaking.
The current do {} while() loop is the most direct solution, if gcc one
day still gives such warning then I could say some harsh word then.
Thanks,
Qu
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 04/13] btrfs-progs: Fix Wempty-body warning
2018-12-05 6:40 ` [PATCH v2 04/13] btrfs-progs: Fix Wempty-body warning Qu Wenruo
@ 2018-12-13 18:59 ` David Sterba
2018-12-13 23:54 ` Qu Wenruo
0 siblings, 1 reply; 19+ messages in thread
From: David Sterba @ 2018-12-13 18:59 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Wed, Dec 05, 2018 at 02:40:09PM +0800, 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; \
I think the definition of PRINT_TRACE_ON_ERROR should be fixed to always
emit a statement, a "do { } while (0)" in the other case. Otherwise it's
"don't forget to enclose in { } if used", the proposed change looks
lightly safer to me. I'll update the patch, no need to resend, please
let me know if it's ok for you.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 04/13] btrfs-progs: Fix Wempty-body warning
2018-12-13 18:59 ` David Sterba
@ 2018-12-13 23:54 ` Qu Wenruo
0 siblings, 0 replies; 19+ messages in thread
From: Qu Wenruo @ 2018-12-13 23:54 UTC (permalink / raw)
To: dsterba, Qu Wenruo, linux-btrfs
[-- Attachment #1.1: Type: text/plain, Size: 1268 bytes --]
On 2018/12/14 上午2:59, David Sterba wrote:
> On Wed, Dec 05, 2018 at 02:40:09PM +0800, 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; \
>
> I think the definition of PRINT_TRACE_ON_ERROR should be fixed to always
> emit a statement, a "do { } while (0)" in the other case.
Looks much better than my braces solution.
> Otherwise it's
> "don't forget to enclose in { } if used", the proposed change looks
> lightly safer to me. I'll update the patch, no need to resend, please
> let me know if it's ok for you.
>
I'm completely OK with such better solution.
Thanks,
Qu
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 07/13] btrfs-progs: Fix Wmaybe-uninitialized warning
2018-12-05 14:03 ` Qu Wenruo
@ 2019-01-15 17:42 ` David Sterba
0 siblings, 0 replies; 19+ messages in thread
From: David Sterba @ 2019-01-15 17:42 UTC (permalink / raw)
To: Qu Wenruo; +Cc: dsterba, Qu Wenruo, linux-btrfs
On Wed, Dec 05, 2018 at 10:03:30PM +0800, Qu Wenruo wrote:
>
>
> On 2018/12/5 下午9:40, David Sterba wrote:
> > On Wed, Dec 05, 2018 at 02:40:12PM +0800, 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;
> >>
> >> It's possible that compiler doesn't get enough hint for BUG_ON() on
> >> lowest_level + 1 >= BTRFS_MAX_LEVEL case.
> >>
> >> Fix it by using a do {} while() loop other than while() {} loop, to
> >> ensure we will run the loop for at least once.
> >
> > I was hoping that we can actually add the hint to BUG_ON so the code
> > does not continue if the condition is true.
> >
> I checked that method, but I'm not that confident about things like:
>
> bugon_trace()
> {
> if (!val)
> return;
> __bugon_trace();
> }
>
> __attribute__ ((noreturn))
> static inline void __bugon_trace();
>
> This is as simple as just one extra function call, but the original
> problem is just one more function call before hitting abort().
>
> So I just give up screwing up things I'm not comfort enough to tweaking.
>
> The current do {} while() loop is the most direct solution, if gcc one
> day still gives such warning then I could say some harsh word then.
I was not able to make it work properly, patch applied so we rid of the
warning. Thanks.
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2019-01-15 17:43 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-05 6:40 [PATCH v2 00/13] btrfs-progs: Make W=1 great (no "again") Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 01/13] btrfs-progs: Makefile.extrawarn: Import cc-disable-warning Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 02/13] btrfs-progs: fix gcc8 default build warning caused by '-Wformat-truncation' Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 03/13] btrfs-progs: Makefile.extrawarn: Don't warn on sign compare Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 04/13] btrfs-progs: Fix Wempty-body warning Qu Wenruo
2018-12-13 18:59 ` David Sterba
2018-12-13 23:54 ` Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 05/13] btrfs-progs: Fix Wimplicit-fallthrough warning Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 06/13] btrfs-progs: Fix Wsuggest-attribute=format warning Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 07/13] btrfs-progs: Fix Wmaybe-uninitialized warning Qu Wenruo
2018-12-05 13:40 ` David Sterba
2018-12-05 14:03 ` Qu Wenruo
2019-01-15 17:42 ` David Sterba
2018-12-05 6:40 ` [PATCH v2 08/13] btrfs-progs: Fix Wtype-limits warning Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 09/13] btrfs-progs: Fix missing-prototypes warning caused by non-static functions Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 10/13] btrfs-progs: Move btrfs_check_nodesize to fsfeatures.c to fix missing-prototypes warning Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 11/13] btrfs-progs: Introduce rescue.h to resolve missing-prototypes for chunk and super rescue Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 12/13] btrfs-progs: Add utils.h include to solve missing-prototypes warning Qu Wenruo
2018-12-05 6:40 ` [PATCH v2 13/13] btrfs-progs: free-space-tree: Remove unsued function Qu Wenruo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).