* [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 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
* 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
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).