Re: [PATCH v6 01/11] hw/qdev-properties-system: Introduce EndianMode QAPI enum

[Daniel P . Berrangé]

On Wed, Feb 12, 2025 at 01:02:18PM +0100, Philippe Mathieu-Daudé wrote:
> On 12/2/25 12:43, Philippe Mathieu-Daudé wrote:
> > On 12/2/25 12:37, Thomas Huth wrote:
> > > On 12/02/2025 12.24, Philippe Mathieu-Daudé wrote:
> > > > Introduce the EndianMode type and the DEFINE_PROP_ENDIAN() macros.
> > > > Endianness can be BIG, LITTLE or unspecified (default).
> > > > 
> > > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > > ---
> > > >   qapi/common.json                    | 16 ++++++++++++++++
> > > >   include/hw/qdev-properties-system.h |  7 +++++++
> > > >   hw/core/qdev-properties-system.c    | 11 +++++++++++
> > > >   3 files changed, 34 insertions(+)
> > > > 
> > > > diff --git a/qapi/common.json b/qapi/common.json
> > > > index 6ffc7a37890..217feaaf683 100644
> > > > --- a/qapi/common.json
> > > > +++ b/qapi/common.json
> > > > @@ -212,3 +212,19 @@
> > > >   ##
> > > >   { 'struct': 'HumanReadableText',
> > > >     'data': { 'human-readable-text': 'str' } }
> > > > +
> > > > +##
> > > > +# @EndianMode:
> > > > +#
> > > > +# An enumeration of three options: little, big, and unspecified
> > > > +#
> > > > +# @little: Little endianness
> > > > +#
> > > > +# @big: Big endianness
> > > > +#
> > > > +# @unspecified: Endianness not specified
> > > > +#
> > > > +# Since: 10.0
> > > > +##
> > > > +{ 'enum': 'EndianMode',
> > > > +  'data': [ 'little', 'big', 'unspecified' ] }
> > > 
> > > Should 'unspecified' come first? ... so that it gets the value 0,
> > > i.e. when someone forgets to properly initialize a related variable,
> > > the chances are higher that it ends up as "unspecified" than as
> > > "little" ?
> 
> BTW I'm not sure QAPI guaranty enums are following an order
> (at least, as in C, I wouldn't rely on that assumption).

If we don't document a guaranteed order IMHO we should, mostly just for the
sake for guaranteeing exactly what will be the 0 value . It is pretty common
to want a particular enum constant to be special default for the 0 value.
It allows enums to be retrofitted into existing code, with confidence that
any code forgetting to initialize a variable/field will get the special
default. Missed initialization is relatively common as a C bug, and we
use -ftrivial-auto-var-init=zero to give well defined (usually safe)
semantics.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|