* [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; 6+ 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] 6+ 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; 6+ 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 related [flat|nested] 6+ 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 2019-11-26 10:36 ` Geert Uytterhoeven 0 siblings, 1 reply; 6+ 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] 6+ messages in thread
* Re: [PATCH next 1/2] btrfs: tree-checker: Fix error format string 2019-11-11 18:31 ` David Sterba @ 2019-11-26 10:36 ` Geert Uytterhoeven 2019-11-26 15:44 ` David Sterba 0 siblings, 1 reply; 6+ messages in thread From: Geert Uytterhoeven @ 2019-11-26 10:36 UTC (permalink / raw) To: David Sterba, Andreas Färber, Qu Wenruo, David Sterba, Johannes Thumshirn, Chris Mason, Josef Bacik, linux-btrfs, Linux ARM, Linux Kernel Mailing List Hi David, On Mon, Nov 11, 2019 at 7:32 PM David Sterba <dsterba@suse.cz> wrote: > 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. Yep. On 64-bit, size_t _is_ unsigned long. So you have to compile for both 32-bit and 64-bit. Acked-by: Geert Uytterhoeven <geert@linux-m68k.org> > > 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 It indeed is not: Fixes: 153a6d299956983d ("btrfs: tree-checker: Check item size before reading file extent type") > and I'll fold the change to to the patch. Thanks. Apparently that was forgotten, and now the issue is part of Linus' tree. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH next 1/2] btrfs: tree-checker: Fix error format string 2019-11-26 10:36 ` Geert Uytterhoeven @ 2019-11-26 15:44 ` David Sterba 0 siblings, 0 replies; 6+ messages in thread From: David Sterba @ 2019-11-26 15:44 UTC (permalink / raw) To: Geert Uytterhoeven Cc: David Sterba, Andreas Färber, Qu Wenruo, David Sterba, Johannes Thumshirn, Chris Mason, Josef Bacik, linux-btrfs, Linux ARM, Linux Kernel Mailing List On Tue, Nov 26, 2019 at 11:36:58AM +0100, Geert Uytterhoeven wrote: > > > 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 > > It indeed is not: > Fixes: 153a6d299956983d ("btrfs: tree-checker: Check item size before > reading file extent type") > > > and I'll fold the change to to the patch. Thanks. > > Apparently that was forgotten, and now the issue is part of Linus' tree. Mistake on my side so I'll apply the full fixup patch, thanks for noticing. ^ permalink raw reply [flat|nested] 6+ 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; 6+ 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 related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-11-26 15:44 UTC | newest] Thread overview: 6+ 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-26 10:36 ` Geert Uytterhoeven 2019-11-26 15:44 ` David Sterba 2019-11-08 21:38 ` [PATCH next 2/2] btrfs: extent-tree: " Andreas Färber
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).