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

["Anthony PERARD" <anthony.perard@vates.tech>]

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é <roger.pau@citrix.com>
> > ---
> >  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@citrix.com>
>
> 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 <anthony.perard@vates.tech>

Thanks,

--

Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech