* [PATCH 1/2] btrfs: Use NAME_MAX to replace intermediate number of BTRFS_NAME_LEN
@ 2018-09-25 0:06 Qu Wenruo
2018-09-25 0:06 ` [PATCH 2/2] btrfs: tree-checker: Avoid using max() for stack array allocation Qu Wenruo
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Qu Wenruo @ 2018-09-25 0:06 UTC (permalink / raw)
To: linux-btrfs
Since we're following the name size limit of linux, just use NAME_MAX.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/ctree.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 53af9f5253f4..5ab6d1f6e055 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -65,7 +65,7 @@ struct btrfs_ordered_sum;
* we can actually store much bigger names, but lets not confuse the rest
* of linux
*/
-#define BTRFS_NAME_LEN 255
+#define BTRFS_NAME_LEN NAME_MAX
/*
* Theoretical limit is larger, but we keep this down to a sane
--
2.19.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] btrfs: tree-checker: Avoid using max() for stack array allocation 2018-09-25 0:06 [PATCH 1/2] btrfs: Use NAME_MAX to replace intermediate number of BTRFS_NAME_LEN Qu Wenruo @ 2018-09-25 0:06 ` Qu Wenruo 2018-09-25 15:34 ` David Sterba 2018-09-25 1:03 ` [PATCH 1/2] btrfs: Use NAME_MAX to replace intermediate number of BTRFS_NAME_LEN Su Yue 2018-09-25 15:29 ` David Sterba 2 siblings, 1 reply; 6+ messages in thread From: Qu Wenruo @ 2018-09-25 0:06 UTC (permalink / raw) To: linux-btrfs Although BTRFS_NAME_LEN and XATTR_NAME_MAX is the same value (255), max(BTRFS_NAME_LEN, XATTR_NAME_MAX) should be optimized as const at runtime. However S390x' arch dependent option "-mwarn-dynamicstack" could still report it as dyanamic stack allocation. Just use BTRFS_NAME_LEN directly to avoid such false alert. Signed-off-by: Qu Wenruo <wqu@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 db835635372f..4c045609909b 100644 --- a/fs/btrfs/tree-checker.c +++ b/fs/btrfs/tree-checker.c @@ -336,7 +336,7 @@ static int check_dir_item(struct btrfs_fs_info *fs_info, */ if (key->type == BTRFS_DIR_ITEM_KEY || key->type == BTRFS_XATTR_ITEM_KEY) { - char namebuf[max(BTRFS_NAME_LEN, XATTR_NAME_MAX)]; + char namebuf[BTRFS_NAME_LEN]; read_extent_buffer(leaf, namebuf, (unsigned long)(di + 1), name_len); -- 2.19.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] btrfs: tree-checker: Avoid using max() for stack array allocation 2018-09-25 0:06 ` [PATCH 2/2] btrfs: tree-checker: Avoid using max() for stack array allocation Qu Wenruo @ 2018-09-25 15:34 ` David Sterba 2018-09-26 0:19 ` Qu Wenruo 0 siblings, 1 reply; 6+ messages in thread From: David Sterba @ 2018-09-25 15:34 UTC (permalink / raw) To: Qu Wenruo; +Cc: linux-btrfs On Tue, Sep 25, 2018 at 08:06:26AM +0800, Qu Wenruo wrote: > Although BTRFS_NAME_LEN and XATTR_NAME_MAX is the same value (255), > max(BTRFS_NAME_LEN, XATTR_NAME_MAX) should be optimized as const at > runtime. > > However S390x' arch dependent option "-mwarn-dynamicstack" could still > report it as dyanamic stack allocation. > > Just use BTRFS_NAME_LEN directly to avoid such false alert. Same reasoning as for the NAME_MAX, these are different things. > Signed-off-by: Qu Wenruo <wqu@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 db835635372f..4c045609909b 100644 > --- a/fs/btrfs/tree-checker.c > +++ b/fs/btrfs/tree-checker.c > @@ -336,7 +336,7 @@ static int check_dir_item(struct btrfs_fs_info *fs_info, > */ > if (key->type == BTRFS_DIR_ITEM_KEY || > key->type == BTRFS_XATTR_ITEM_KEY) { > - char namebuf[max(BTRFS_NAME_LEN, XATTR_NAME_MAX)]; > + char namebuf[BTRFS_NAME_LEN]; The updated implementation of max() can now handle the expression without a warning, with sufficiently new compiler so I don't think we need to fix that. Alternatively, you could use BTRFS_NAME_LEN and add a BUILD_BUG_ON(BTRFS_NAME_LEN < XATTR_NAME_MAX) with a comment why. > > read_extent_buffer(leaf, namebuf, > (unsigned long)(di + 1), name_len); > -- > 2.19.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] btrfs: tree-checker: Avoid using max() for stack array allocation 2018-09-25 15:34 ` David Sterba @ 2018-09-26 0:19 ` Qu Wenruo 0 siblings, 0 replies; 6+ messages in thread From: Qu Wenruo @ 2018-09-26 0:19 UTC (permalink / raw) To: dsterba, Qu Wenruo, linux-btrfs [-- Attachment #1.1: Type: text/plain, Size: 1762 bytes --] On 2018/9/25 下午11:34, David Sterba wrote: > On Tue, Sep 25, 2018 at 08:06:26AM +0800, Qu Wenruo wrote: >> Although BTRFS_NAME_LEN and XATTR_NAME_MAX is the same value (255), >> max(BTRFS_NAME_LEN, XATTR_NAME_MAX) should be optimized as const at >> runtime. >> >> However S390x' arch dependent option "-mwarn-dynamicstack" could still >> report it as dyanamic stack allocation. >> >> Just use BTRFS_NAME_LEN directly to avoid such false alert. > > Same reasoning as for the NAME_MAX, these are different things. > >> Signed-off-by: Qu Wenruo <wqu@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 db835635372f..4c045609909b 100644 >> --- a/fs/btrfs/tree-checker.c >> +++ b/fs/btrfs/tree-checker.c >> @@ -336,7 +336,7 @@ static int check_dir_item(struct btrfs_fs_info *fs_info, >> */ >> if (key->type == BTRFS_DIR_ITEM_KEY || >> key->type == BTRFS_XATTR_ITEM_KEY) { >> - char namebuf[max(BTRFS_NAME_LEN, XATTR_NAME_MAX)]; >> + char namebuf[BTRFS_NAME_LEN]; > > The updated implementation of max() can now handle the expression > without a warning, with sufficiently new compiler so I don't think we > need to fix that. Yes, it's mostly a workaround to make S390 happy. And if it can be fixed by kernel config/compiler, it doesn't make much sense to fix it here. So please discard these 2 patches. Thanks, Qu > > Alternatively, you could use BTRFS_NAME_LEN and add a > BUILD_BUG_ON(BTRFS_NAME_LEN < XATTR_NAME_MAX) with a comment why. > >> >> read_extent_buffer(leaf, namebuf, >> (unsigned long)(di + 1), name_len); >> -- >> 2.19.0 [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] btrfs: Use NAME_MAX to replace intermediate number of BTRFS_NAME_LEN 2018-09-25 0:06 [PATCH 1/2] btrfs: Use NAME_MAX to replace intermediate number of BTRFS_NAME_LEN Qu Wenruo 2018-09-25 0:06 ` [PATCH 2/2] btrfs: tree-checker: Avoid using max() for stack array allocation Qu Wenruo @ 2018-09-25 1:03 ` Su Yue 2018-09-25 15:29 ` David Sterba 2 siblings, 0 replies; 6+ messages in thread From: Su Yue @ 2018-09-25 1:03 UTC (permalink / raw) To: Qu Wenruo, linux-btrfs On 9/25/18 8:06 AM, Qu Wenruo wrote: > Since we're following the name size limit of linux, just use NAME_MAX. > > Signed-off-by: Qu Wenruo <wqu@suse.com> Reviewed-by: Su Yue <suy.fnst@cn.fujitsu.com> > --- > fs/btrfs/ctree.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h > index 53af9f5253f4..5ab6d1f6e055 100644 > --- a/fs/btrfs/ctree.h > +++ b/fs/btrfs/ctree.h > @@ -65,7 +65,7 @@ struct btrfs_ordered_sum; > * we can actually store much bigger names, but lets not confuse the rest > * of linux > */ > -#define BTRFS_NAME_LEN 255 > +#define BTRFS_NAME_LEN NAME_MAX > > /* > * Theoretical limit is larger, but we keep this down to a sane > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] btrfs: Use NAME_MAX to replace intermediate number of BTRFS_NAME_LEN 2018-09-25 0:06 [PATCH 1/2] btrfs: Use NAME_MAX to replace intermediate number of BTRFS_NAME_LEN Qu Wenruo 2018-09-25 0:06 ` [PATCH 2/2] btrfs: tree-checker: Avoid using max() for stack array allocation Qu Wenruo 2018-09-25 1:03 ` [PATCH 1/2] btrfs: Use NAME_MAX to replace intermediate number of BTRFS_NAME_LEN Su Yue @ 2018-09-25 15:29 ` David Sterba 2 siblings, 0 replies; 6+ messages in thread From: David Sterba @ 2018-09-25 15:29 UTC (permalink / raw) To: Qu Wenruo; +Cc: linux-btrfs On Tue, Sep 25, 2018 at 08:06:25AM +0800, Qu Wenruo wrote: > Since we're following the name size limit of linux, just use NAME_MAX. > > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > fs/btrfs/ctree.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h > index 53af9f5253f4..5ab6d1f6e055 100644 > --- a/fs/btrfs/ctree.h > +++ b/fs/btrfs/ctree.h > @@ -65,7 +65,7 @@ struct btrfs_ordered_sum; > * we can actually store much bigger names, but lets not confuse the rest > * of linux > */ > -#define BTRFS_NAME_LEN 255 > +#define BTRFS_NAME_LEN NAME_MAX While the values are the same, the symbolic names have a slightly different meaning. NAME_MAX is from the public API, BTRFS_NAME_LEN is defined as btrfs limit, and de facto part of the on-disk format. These are independent, although compatible for all practical purposes. I would not conflate the two in the define, the comment could be updated to document that better though. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-09-26 0:20 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-09-25 0:06 [PATCH 1/2] btrfs: Use NAME_MAX to replace intermediate number of BTRFS_NAME_LEN Qu Wenruo 2018-09-25 0:06 ` [PATCH 2/2] btrfs: tree-checker: Avoid using max() for stack array allocation Qu Wenruo 2018-09-25 15:34 ` David Sterba 2018-09-26 0:19 ` Qu Wenruo 2018-09-25 1:03 ` [PATCH 1/2] btrfs: Use NAME_MAX to replace intermediate number of BTRFS_NAME_LEN Su Yue 2018-09-25 15:29 ` David Sterba
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).