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

[Markus Armbruster]

Marc-André Lureau <address@hidden> writes:

> 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.

Beware of prerequisites escalating into "boil the ocean".

Don't get me wrong, I *want* the qapi monolith broken up.  I also think
applying conditionals everywhere would be neat, and save us quite a few
silly stubs.  But we need to identify intermedeate steps that are in
reasonably easy reach.

Applying conditionals just to qmp-introspect.c could be such a step.  It
solves the "can't introspect whether compile-time conditional stuff is
available" problem.  It may not be the simplest solution for that
problem, but it should be a stepping stone towards solving other
problems.

On breaking up the monolith, just to avoid misunderstandings: I don't
think we should split the schema into several independent schemas.
Instead, we should have the QAPI generators generate split output.

An obvious and stupid split is one set of files per .json, where the
headers include each other exactly like the .json do.  By itself not
enough to solve the poisoned symbol problem.  Could become enough if we
manage to move target-dependent stuff to separate .json.

We might need a smarter split.

Running a full generator once per generated file has always been stupid
and slow, and the more files we generate, the stupider and slower it'll
become.  There's no reason a single generator run can't generate all the
files.  Requires a bit of Make wizardry, though.