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

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

在 2024/7/20 10:01, Qu Wenruo 写道:
>
>
> 在 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.

If you really want to dig deeper, let me compare the bytes usages of
both methods on 64bit systems:

For name[24]:
13 * (8 + 24) = 416 bytes

For const char *name:

13 * (8 + 8) + (9 + 11 + 10 + 8 + 7 + 9 + 8 + 10 + 9 + 15 + 16 + 15 +
16) + 13 = 365 bytes

Now you can see which methods wastes more space.

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