Re: [Qemu-devel] [PATCH for-4.0 v7 02/27] qapi: do not define enumeration value explicitly

[Marc-André Lureau]

Hi

On Wed, Dec 12, 2018 at 12:52 PM Markus Armbruster <address@hidden> wrote:
>
> Marc-André Lureau <address@hidden> writes:
>
> > The C standard has the initial value at 0 and the subsequent values
> > incremented by 1. No need to set this explicitely.
> >
> > This will prevent from artificial "gaps" when compiling out some enum
> > values and having unnecessarily large MAX values & enums arrays, or
> > simplifying iterating over valid enum values.
>
> Should mention that compiling out will only become possible later in
> this series.
>
> > Whenever config-host.h is changed, all the enum/types are recompiled.
>
> This soundness argument is incomplete.  Yes, our coding conventions
> ensure everything gets recompiled when config-host.h changes.  But
> nothing stops people from using 'if' conditions that depend on more than
> just config-host.h.
>
> What about this:
>
>   qapi: Do not define enumeration value explicitly
>
>   The generated C enumeration types explicitly set the enumeration
>   constants to 0, 1, 2, ...  That's exactly what you get when you don't
>   supply values.
>
>   Drop the explicit values.  No change now, but it will avoid gaps in
>   the values when we later add support for 'if' conditions.  Avoiding
>   such gaps will save us the trouble of changing the ENUM_lookup[]
>   tables to work without a sentinel.
>
>   We'll have to take care to ensure the headers required by the 'if'
>   conditions get always included before the generated QAPI code.
>   Fortunately, our convention to include "qemu/osdep.h" first in any .c
>   ensures that's the case for our CONFIG_FOO macros.
>
> > Signed-off-by: Marc-André Lureau <address@hidden>
>
> With an improved commit message:
> Reviewed-by: Markus Armbruster <address@hidden>

ack, thanks!