Re: [PATCH] tools/libxl: remove usage of VLA arrays

[Frediano Ziglio <frediano.ziglio@xxxxxxxxx>]

On Mon, Oct 28, 2024 at 11:48 AM Roger Pau Monne <roger.pau@xxxxxxxxxx> 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é <roger.pau@xxxxxxxxxx>
> ---
>  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);

Minor: why not sizeof instead of ARRAY_SIZE ?

Not a regression, but looking at final strings and size added, it
looks like the numbers should be more uint16_t than uint32_t.
Maybe we want something like

    char path[sizeof("/local/pool//name") + 12];

BTW,

Reviewed-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxx>

Frediano