* [PATCH] tools/libxl: remove usage of VLA arrays
@ 2024-10-28 11:48 Roger Pau Monne
2024-10-28 11:58 ` Frediano Ziglio
2024-10-28 12:03 ` Andrew Cooper
0 siblings, 2 replies; 9+ messages in thread
From: Roger Pau Monne @ 2024-10-28 11:48 UTC (permalink / raw)
To: xen-devel; +Cc: Roger Pau Monne, Anthony PERARD, Juergen Gross
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);
--
2.46.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] tools/libxl: remove usage of VLA arrays
2024-10-28 11:48 [PATCH] tools/libxl: remove usage of VLA arrays Roger Pau Monne
@ 2024-10-28 11:58 ` Frediano Ziglio
2024-10-28 12:03 ` Andrew Cooper
1 sibling, 0 replies; 9+ messages in thread
From: Frediano Ziglio @ 2024-10-28 11:58 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: xen-devel, Anthony PERARD, Juergen Gross
On Mon, Oct 28, 2024 at 11:48 AM Roger Pau Monne <roger.pau@citrix.com> 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);
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@cloud.com>
Frediano
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] tools/libxl: remove usage of VLA arrays
2024-10-28 11:48 [PATCH] tools/libxl: remove usage of VLA arrays Roger Pau Monne
2024-10-28 11:58 ` Frediano Ziglio
@ 2024-10-28 12:03 ` Andrew Cooper
2024-10-28 12:48 ` Jan Beulich
2024-11-05 15:05 ` Anthony PERARD
1 sibling, 2 replies; 9+ messages in thread
From: Andrew Cooper @ 2024-10-28 12:03 UTC (permalink / raw)
To: Roger Pau Monne, xen-devel; +Cc: Anthony PERARD, Juergen Gross
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.
Can fix on commit, if you're happy?
~Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] tools/libxl: remove usage of VLA arrays
2024-10-28 12:03 ` Andrew Cooper
@ 2024-10-28 12:48 ` Jan Beulich
2024-10-28 12:57 ` Frediano Ziglio
2024-11-05 15:05 ` Anthony PERARD
1 sibling, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2024-10-28 12:48 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Anthony PERARD, Juergen Gross, Roger Pau Monne, xen-devel
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@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.
>
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] tools/libxl: remove usage of VLA arrays
2024-10-28 12:48 ` Jan Beulich
@ 2024-10-28 12:57 ` Frediano Ziglio
2024-10-28 17:26 ` Roger Pau Monné
0 siblings, 1 reply; 9+ messages in thread
From: Frediano Ziglio @ 2024-10-28 12:57 UTC (permalink / raw)
To: Jan Beulich
Cc: Andrew Cooper, Anthony PERARD, Juergen Gross, Roger Pau Monne,
xen-devel
On Mon, Oct 28, 2024 at 12:48 PM Jan Beulich <jbeulich@suse.com> 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@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.
> >
> > 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] tools/libxl: remove usage of VLA arrays
2024-10-28 12:57 ` Frediano Ziglio
@ 2024-10-28 17:26 ` Roger Pau Monné
2024-10-29 7:42 ` Jan Beulich
0 siblings, 1 reply; 9+ messages in thread
From: Roger Pau Monné @ 2024-10-28 17:26 UTC (permalink / raw)
To: Frediano Ziglio
Cc: Jan Beulich, Andrew Cooper, Anthony PERARD, Juergen Gross,
xen-devel
On Mon, Oct 28, 2024 at 12:57:30PM +0000, Frediano Ziglio wrote:
> On Mon, Oct 28, 2024 at 12:48 PM Jan Beulich <jbeulich@suse.com> 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@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.
> > >
> > > 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.
No strong opinion either, I've assumed it was clearer to not make
implicit assumptions about the size of the string literal array
elements. I would rather like to get this committed, and Jan seems to
prefer to use ARRAY_SIZE(), so I suggest we get the patch committed
as-is.
Thanks, Roger.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] tools/libxl: remove usage of VLA arrays
2024-10-28 17:26 ` Roger Pau Monné
@ 2024-10-29 7:42 ` Jan Beulich
0 siblings, 0 replies; 9+ messages in thread
From: Jan Beulich @ 2024-10-29 7:42 UTC (permalink / raw)
To: Roger Pau Monné
Cc: Andrew Cooper, Anthony PERARD, Juergen Gross, xen-devel,
Frediano Ziglio
On 28.10.2024 18:26, Roger Pau Monné wrote:
> On Mon, Oct 28, 2024 at 12:57:30PM +0000, Frediano Ziglio wrote:
>> On Mon, Oct 28, 2024 at 12:48 PM Jan Beulich <jbeulich@suse.com> 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@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.
>>>>
>>>> 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.
>
> No strong opinion either, I've assumed it was clearer to not make
> implicit assumptions about the size of the string literal array
> elements. I would rather like to get this committed, and Jan seems to
> prefer to use ARRAY_SIZE(), so I suggest we get the patch committed
> as-is.
Well, ultimately it's Anthony's call, for still being the sole maintainer.
Jan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] tools/libxl: remove usage of VLA arrays
2024-10-28 12:03 ` Andrew Cooper
2024-10-28 12:48 ` Jan Beulich
@ 2024-11-05 15:05 ` Anthony PERARD
2024-11-05 20:19 ` Andrew Cooper
1 sibling, 1 reply; 9+ messages in thread
From: Anthony PERARD @ 2024-11-05 15:05 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Roger Pau Monne, xen-devel, Juergen Gross
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] tools/libxl: remove usage of VLA arrays
2024-11-05 15:05 ` Anthony PERARD
@ 2024-11-05 20:19 ` Andrew Cooper
0 siblings, 0 replies; 9+ messages in thread
From: Andrew Cooper @ 2024-11-05 20:19 UTC (permalink / raw)
To: Anthony PERARD; +Cc: Roger Pau Monne, xen-devel, Juergen Gross
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é <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>
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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-11-05 20:20 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-28 11:48 [PATCH] tools/libxl: remove usage of VLA arrays Roger Pau Monne
2024-10-28 11:58 ` Frediano Ziglio
2024-10-28 12:03 ` Andrew Cooper
2024-10-28 12:48 ` Jan Beulich
2024-10-28 12:57 ` Frediano Ziglio
2024-10-28 17:26 ` Roger Pau Monné
2024-10-29 7:42 ` Jan Beulich
2024-11-05 15:05 ` Anthony PERARD
2024-11-05 20:19 ` Andrew Cooper
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.