On 05/11/2024 3:05 pm, Anthony PERARD wrote:
> On Mon, Oct 28, 2024 at 12:03:59PM +0000, Andrew Cooper wrote:
>> On 28/10/2024 11:48 am, Roger Pau Monne wrote:
>>> Clang 19 complains with the following error when building libxl:
>>>
>>> libxl_utils.c:48:15: error: variable length array folded to constant array
>>> as an extension [-Werror,-Wgnu-folding-constant]
>>> 48 | char path[strlen("/local/domain") + 12];
>>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>
>>> Replace the usage of strlen() with ARRAY_SIZE(), which allows the literal
>>> string length to be known at build time. Note ARRAY_SIZE() accounts for the
>>> NUL terminator while strlen() didn't, hence subtract 1 from the total size
>>> calculation.
>>>
>>> Signed-off-by: Roger Pau Monné <[email protected]>
>>> ---
>>> tools/libs/light/libxl_utils.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/libs/light/libxl_utils.c b/tools/libs/light/libxl_utils.c
>>> index 10398a6c8611..b3f5e751cc3f 100644
>>> --- a/tools/libs/light/libxl_utils.c
>>> +++ b/tools/libs/light/libxl_utils.c
>>> @@ -45,7 +45,7 @@ unsigned long libxl_get_required_shadow_memory(unsigned
>>> long maxmem_kb, unsigned
>>> char *libxl_domid_to_name(libxl_ctx *ctx, uint32_t domid)
>>> {
>>> unsigned int len;
>>> - char path[strlen("/local/domain") + 12];
>>> + char path[ARRAY_SIZE("/local/domain") + 11];
>>> char *s;
>>>
>>> snprintf(path, sizeof(path), "/local/domain/%d/name", domid);
>>> @@ -141,7 +141,7 @@ int libxl_cpupool_qualifier_to_cpupoolid(libxl_ctx
>>> *ctx, const char *p,
>>> char *libxl_cpupoolid_to_name(libxl_ctx *ctx, uint32_t poolid)
>>> {
>>> unsigned int len;
>>> - char path[strlen("/local/pool") + 12];
>>> + char path[ARRAY_SIZE("/local/pool") + 11];
>>> char *s;
>>>
>>> snprintf(path, sizeof(path), "/local/pool/%d/name", poolid);
>> Acked-by: Andrew Cooper <[email protected]>
>>
>> Although I have a minor preference for sizeof() as suggested by Frediano.
> I have a preference for sizeof() too, we even used it this way (more or
> less) in libxl before, for `eom` here:
>
> https://elixir.bootlin.com/xen/v4.19.0/source/tools/libs/light/libxl_qmp.c#L1608
>
> I was a bit supprised by the use of ARRAY_SIZE on a string literal but
> it's just an array of char :-).
>
> For the patch, with sizeof() or ARRAY_SIZE():
> Acked-by: Anthony PERARD <[email protected]>
That's 3 for sizeof(), one ambivalent, and one for ARRAY_SIZE(), with
the maintainer (who gets the majority say) already in the largest group
already.
I've adjusted and committed.
~Andrew
