Re: [Qemu-devel] [PATCH v2 28/54] qapi: do not define enumeration value explicitely

[Marc-André Lureau]

Hi

----- Original Message -----
> Marc-André Lureau <address@hidden> writes:
> 
> > ----- Original Message -----
> >> 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.
> >> 
> >> Yes, but it also risks entertaining mishaps like compiling this one
> >> 
> >>     typedef enum Color {
> >>         COLOR_WHITE,
> >> #if defined(NEED_CPU_H)
> >> #if defined(TARGET_S390X)
> >>         COLOR_BLUE,
> >> #endif /* defined(TARGET_S390X) */
> >> #endif /* defined(NEED_CPU_H) */
> >>         COLOR_BLACK,
> >>     } Color;
> >> 
> >> in s390x-code (COLOR_BLACK = 2) and in target-independent code
> >> (COLOR_BLACK = 1), then linking the two together.
> >
> > This is also true with other kind of types, like struct.
> >
> > One of the main reason why we should have schemas for target-only and the
> > NEED_CPU_H is a temporary working hack.
> 
> A temporary hack that's ugly or occasionally breaks in an obvious way is
> okay, but this is an open deathtrap.  I'm afraid we need to rethink our
> short term conditional code generation strategy.
> 
> Stupidest solution that could possibly work: apply conditionals *only*
> to qmp-introspect.c, leave everything unconditional elsewhere.  What do
> you think?

That's not what I had in mind when I started. My first 45 loc approach would 
have saved us a lot of troubles :)

I would rather fix this, and split the schema. I'll investigate.