Re: [PATCH 2/2] btrfs: tree-checker: Avoid using max() for stack array allocation

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

[-- 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 --]