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

[Frediano Ziglio <frediano.ziglio@xxxxxxxxx>]

On Mon, Oct 28, 2024 at 12:48 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 28.10.2024 13:03, 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é <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);
> >
> > Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> >
> > Although I have a minor preference for sizeof() as suggested by Frediano.
> >
> > Can fix on commit, if you're happy?
>
> Please can we stick to ARRAY_SIZE() when it comes to strings? It's the
> same as sizeof() when the base type is char, but the difference becomes
> relevant if the base type was e.g. wchar_t.
>
> Jan
>

But "literal" is not a wide string, and the type is "char" which is
not wide too.

BTW, both me and Andrew are not strong about.

Frediano