Linux-BTRFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH next 0/2] btrfs: Fix build warnings for arm
@ 2019-11-08 21:38 Andreas Färber
  2019-11-08 21:38 ` [PATCH next 1/2] btrfs: tree-checker: Fix error format string Andreas Färber
  2019-11-08 21:38 ` [PATCH next 2/2] btrfs: extent-tree: " Andreas Färber
  0 siblings, 2 replies; 4+ messages in thread
From: Andreas Färber @ 2019-11-08 21:38 UTC (permalink / raw)
  To: Qu Wenruo, David Sterba
  Cc: Johannes Thumshirn, Chris Mason, Josef Bacik, linux-btrfs,
	linux-arm-kernel, linux-kernel

Hello Wenruo and Dave,

This mini-series fixes two build warnings found while cross-compiling for arm,
using openSUSE's cross-arm-binutils and cross-arm-none-gcc9 packages.

Replacing BUG_ON() with btrfs_crit() error handling is noble work, but please
be careful not to hardcode format specifiers for x86_64's size_t.

https://www.kernel.org/doc/Documentation/printk-formats.txt

In one case it could've been noticed during review, in another it was hidden
through a macro and would've only been found through compile-testing.
Probably a 32-bit i386 build would do; otherwise ARCH=arm multi_v7_defconfig
plus CONFIG_BTRFS_FS should reproduce.

It's around for maybe three weeks, so I wonder why kbuild bot didn't catch it.

Cheers,
Andreas

Andreas Färber (2):
  btrfs: tree-checker: Fix error format string
  btrfs: extent-tree: Fix error format string

 fs/btrfs/extent-tree.c  | 2 +-
 fs/btrfs/tree-checker.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
2.16.4


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH next 1/2] btrfs: tree-checker: Fix error format string
  2019-11-08 21:38 [PATCH next 0/2] btrfs: Fix build warnings for arm Andreas Färber
@ 2019-11-08 21:38 ` Andreas Färber
  2019-11-11 18:31   ` David Sterba
  2019-11-08 21:38 ` [PATCH next 2/2] btrfs: extent-tree: " Andreas Färber
  1 sibling, 1 reply; 4+ messages in thread
From: Andreas Färber @ 2019-11-08 21:38 UTC (permalink / raw)
  To: Qu Wenruo, David Sterba
  Cc: Johannes Thumshirn, Chris Mason, Josef Bacik, linux-btrfs,
	linux-arm-kernel, linux-kernel

From: Andreas Färber <afaerber@suse.com>

Argument BTRFS_FILE_EXTENT_INLINE_DATA_START is defined as offsetof(),
which returns type size_t, so we need %zu instead of %lu.

This fixes a build warning on 32-bit arm:

  ../fs/btrfs/tree-checker.c: In function 'check_extent_data_item':
  ../fs/btrfs/tree-checker.c:230:43: warning: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'unsigned int' [-Wformat=]
    230 |     "invalid item size, have %u expect [%lu, %u)",
        |                                         ~~^
        |                                           |
        |                                           long unsigned int
        |                                         %u

Fixes: a31ccb4b7ba2 ("btrfs: tree-checker: Check item size before reading file extent type")
Cc: Qu Wenruo <wqu@suse.com>
Cc: David Sterba <dsterba@suse.com>
Signed-off-by: Andreas Färber <afaerber@suse.com>
---
 fs/btrfs/tree-checker.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 493d4d9e0f79..092b8ece36d7 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -227,7 +227,7 @@ static int check_extent_data_item(struct extent_buffer *leaf,
 	 */
 	if (item_size < BTRFS_FILE_EXTENT_INLINE_DATA_START) {
 		file_extent_err(leaf, slot,
-				"invalid item size, have %u expect [%lu, %u)",
+				"invalid item size, have %u expect [%zu, %u)",
 				item_size, BTRFS_FILE_EXTENT_INLINE_DATA_START,
 				SZ_4K);
 		return -EUCLEAN;
-- 
2.16.4


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH next 2/2] btrfs: extent-tree: Fix error format string
  2019-11-08 21:38 [PATCH next 0/2] btrfs: Fix build warnings for arm Andreas Färber
  2019-11-08 21:38 ` [PATCH next 1/2] btrfs: tree-checker: Fix error format string Andreas Färber
@ 2019-11-08 21:38 ` " Andreas Färber
  1 sibling, 0 replies; 4+ messages in thread
From: Andreas Färber @ 2019-11-08 21:38 UTC (permalink / raw)
  To: Qu Wenruo, David Sterba
  Cc: Johannes Thumshirn, Chris Mason, Josef Bacik, linux-btrfs,
	linux-arm-kernel, linux-kernel

From: Andreas Färber <afaerber@suse.com>

sizeof() returns type size_t, thus we need %zu instead of %lu.

This fixes the following build warning on 32-bit arm:

  In file included from ../include/linux/printk.h:7,
                   from ../include/linux/kernel.h:15,
                   from ../include/asm-generic/bug.h:19,
                   from ../arch/arm/include/asm/bug.h:60,
                   from ../include/linux/bug.h:5,
                   from ../include/linux/thread_info.h:12,
                   from ../include/asm-generic/current.h:5,
                   from ./arch/arm/include/generated/asm/current.h:1,
                   from ../include/linux/sched.h:12,
                   from ../fs/btrfs/extent-tree.c:6:
  ../fs/btrfs/extent-tree.c: In function '__btrfs_free_extent':
  ../include/linux/kern_levels.h:5:18: warning: format '%lu' expects argument of type 'long unsigned int', but argument 8 has type 'unsigned int' [-Wformat=]
      5 | #define KERN_SOH "\001"  /* ASCII Start Of Header */
        |                  ^~~~~~
  ../include/linux/kern_levels.h:10:19: note: in expansion of macro 'KERN_SOH'
     10 | #define KERN_CRIT KERN_SOH "2" /* critical conditions */
        |                   ^~~~~~~~
  ../fs/btrfs/ctree.h:2986:24: note: in expansion of macro 'KERN_CRIT'
   2986 |  btrfs_printk(fs_info, KERN_CRIT fmt, ##args)
        |                        ^~~~~~~~~
  ../fs/btrfs/extent-tree.c:3207:4: note: in expansion of macro 'btrfs_crit'
   3207 |    btrfs_crit(info,
        |    ^~~~~~~~~~
  ../fs/btrfs/extent-tree.c:3208:83: note: format string is defined here
   3208 | "invalid extent item size for key (%llu, %u, %llu) owner %llu, has %u expect >= %lu",
        |                                                                                 ~~^
        |                                                                                   |
        |                                                                                   long unsigned int
        |                                                                                 %u

Fixes: 0c171e9095e4 ("btrfs: extent-tree: Kill BUG_ON() in __btrfs_free_extent() and do better comment")
Cc: Qu Wenruo <wqu@suse.com>
Cc: David Sterba <dsterba@suse.com>
Signed-off-by: Andreas Färber <afaerber@suse.com>
---
 fs/btrfs/extent-tree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 7c7a3e30e917..631c9743ddc7 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3205,7 +3205,7 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
 		struct btrfs_tree_block_info *bi;
 		if (unlikely(item_size < sizeof(*ei) + sizeof(*bi))) {
 			btrfs_crit(info,
-"invalid extent item size for key (%llu, %u, %llu) owner %llu, has %u expect >= %lu",
+"invalid extent item size for key (%llu, %u, %llu) owner %llu, has %u expect >= %zu",
 				   key.objectid, key.type, key.offset,
 				   owner_objectid, item_size,
 				   sizeof(*ei) + sizeof(*bi));
-- 
2.16.4


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH next 1/2] btrfs: tree-checker: Fix error format string
  2019-11-08 21:38 ` [PATCH next 1/2] btrfs: tree-checker: Fix error format string Andreas Färber
@ 2019-11-11 18:31   ` David Sterba
  0 siblings, 0 replies; 4+ messages in thread
From: David Sterba @ 2019-11-11 18:31 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Qu Wenruo, David Sterba, Johannes Thumshirn, Chris Mason,
	Josef Bacik, linux-btrfs, linux-arm-kernel, linux-kernel

On Fri, Nov 08, 2019 at 10:38:52PM +0100, Andreas Färber wrote:
> From: Andreas Färber <afaerber@suse.com>
> 
> Argument BTRFS_FILE_EXTENT_INLINE_DATA_START is defined as offsetof(),
> which returns type size_t, so we need %zu instead of %lu.
> 
> This fixes a build warning on 32-bit arm:
> 
>   ../fs/btrfs/tree-checker.c: In function 'check_extent_data_item':
>   ../fs/btrfs/tree-checker.c:230:43: warning: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'unsigned int' [-Wformat=]
>     230 |     "invalid item size, have %u expect [%lu, %u)",
>         |                                         ~~^
>         |                                           |
>         |                                           long unsigned int
>         |                                         %u

Is there a gcc warning option that can catch that on 64bit too?
-Wformat=2 does not and I don't see any other of the option family to do
that. We've had fixups of the size_t printk formats and I'd like to
catch that when the patches are added to the devel branches. I can't run
32bit build check each time but this seems to be the only way so far.

> Fixes: a31ccb4b7ba2 ("btrfs: tree-checker: Check item size before reading file extent type")

As the patch is still in the devel branch, the commit id is not stable
and I'll fold the change to to the patch. Thanks.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-08 21:38 [PATCH next 0/2] btrfs: Fix build warnings for arm Andreas Färber
2019-11-08 21:38 ` [PATCH next 1/2] btrfs: tree-checker: Fix error format string Andreas Färber
2019-11-11 18:31   ` David Sterba
2019-11-08 21:38 ` [PATCH next 2/2] btrfs: extent-tree: " Andreas Färber

Linux-BTRFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-btrfs/0 linux-btrfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-btrfs linux-btrfs/ https://lore.kernel.org/linux-btrfs \
		linux-btrfs@vger.kernel.org
	public-inbox-index linux-btrfs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-btrfs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git