Re: [PATCH] btrfs: avoid using fixed char array size for tree names

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

在 2024/7/20 09:23, David Sterba 写道:
> On Fri, Jul 19, 2024 at 02:20:39PM +0930, Qu Wenruo wrote:
>> [BUG]
>> There is a bug report that using the latest trunk GCC, btrfs would cause
>> unterminated-string-initialization warning:
>>
>>    linux-6.6/fs/btrfs/print-tree.c:29:49: error: initializer-string for array of ‘char’ is too long [-Werror=unterminated-string-initialization]
>>     29 |         { BTRFS_BLOCK_GROUP_TREE_OBJECTID,      "BLOCK_GROUP_TREE"      },
>>        |
>>        ^~~~~~~~~~~~~~~~~~
>>
>> [CAUSE]
>> To print tree names we have an array of root_name_map structure, which
>> uses "char name[16];" to store the name string of a tree.
>>
>> But the following trees have names exactly at 16 chars length:
>> - "BLOCK_GROUP_TREE"
>> - "RAID_STRIPE_TREE"
>>
>> This means we will have no space for the terminating '\0', and can lead
>> to unexpected access when printing the name.
>>
>> [FIX]
>> Instead of "char name[16];" use "const char *" instead.
>
> Please use a fixed size string, this avoids the indirection of one
> pointer and the actual strings.

I strongly doubt the necessary of avoiding indirection.

Just remember all of our error messages are some pointers to a ro data
section, and I see no reason why we need to bother the indirection or
whatever.

They are the cold path anyway, so is our tree names.

You can go char name[24], but without a proper macros checking the
string length, we're going to hit the same problem sooner or later.

So, I see no reason bothering extending the char size.
It's not extendable, nor safe.

> For static tables like this is a compact
> way to store it.

Nope, it's not compact at all, for shorter names we're just wasting
global ro data space.
The const char * solution is really using the minimal space.

Thanks,
Qu

> As the alignment is mandated by u64 the sizes would be
> best in multipes of 8, so 'char name[24]'.
>