Re: [RFC PATCH v8 06/10] hw/vmapple: aes: convert MAX_LEN to a #define

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Sep 16, 2026 at 12:20:33PM +0100, Peter Maydell wrote:
> On Wed, 16 Sept 2026 at 11:00, Mohamed Mediouni
> <mohamed@unpredictable.fr> wrote:
> >
> > Working around this compiler warning:
> >
> > ../hw/vmapple/aes.c:257:14: warning: variable length array folded to constant array as an extension [-Wgnu-folding-constant]
> >   257 |     char hex[MAX_LEN * 2 + 1] = "";
> >       |              ^~~~~~~~~~~~~~~
> > 1 warning generated.
> 
> I'm surprised this wasn't already caught by our '-Wvla' setting:
> we don't want to have variable length arrays at all.

The -Wgnu-folding-constant warning seems to be saying that
GCC will always evaluate this expresson at compile time
eliminating the variable length array. Presumably that
expansion happens before -Wvla gets to see the code.

IOW, this warning is saying that we're not portable to compilers
other than GCC/CLang, which is just fine by QEMU's intent.

If we don't want to change the code, it looks valid to add
-Wno-gnu-folding-constant, since we dont care about
portability to other compilers.

> 
> > Signed-off-by: Mohamed Mediouni <mohamed@unpredictable.fr>
> > ---
> >  hw/vmapple/aes.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/vmapple/aes.c b/hw/vmapple/aes.c
> > index 553e688adb..9d062d5a7f 100644
> > --- a/hw/vmapple/aes.c
> > +++ b/hw/vmapple/aes.c
> > @@ -253,7 +253,7 @@ static bool cmd_iv(AESState *s)
> >
> >  static void dump_data(const char *desc, const void *p, size_t len)
> >  {
> > -    static const size_t MAX_LEN = 0x1000;
> > +#define MAX_LEN  0x1000
> >      char hex[MAX_LEN * 2 + 1] = "";
> >
> >      if (len > MAX_LEN) {
> > @@ -262,6 +262,7 @@ static void dump_data(const char *desc, const void *p, size_t len)
> >
> >      qemu_hexdump_to_buffer(hex, sizeof(hex), p, len);
> >      trace_aes_dump_data(desc, hex);
> > +#undef MAX_LEN
> >  }
> 
> MAX_LEN is an awkwardly generic name. I would define it outside
> the function, something like
> 
> /* Maximum amount of AES data to dump in trace events */
> #define MAX_AES_DUMP_DATA_LEN 0x1000
> 
> Then you don't need to #undef it as there's no risk of a clash.
> 
> (Really this function ought not to do the binary-to-hex
> conversion of all the data unless the trace event is actually
> enabled, which docs/devel/tracing.rst says you can do, and
> it would maybe be nice to emit at least some truncated data
> in the "more than 1K of data" case, but let's stick to fixing
> the compile issues for the moment.)
> 
> thanks
> -- PMM
> 

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|