Re: [Qemu-devel] [PATCH RFC v2 26/47] qapi-types: Convert to QAPISchemaVisitor, fixing flat unions

[Eric Blake]

On 07/30/2015 09:53 AM, Markus Armbruster wrote:

>> Or, we could ditch the qtypes lookup altogether, and merely create the
>> alternate enum as a non-consecutive QTYPE mapping, for one less level of
>> indirection, as in:
>>
>> typedef enum BlockdevRefKind {
>>     BLOCKDEV_REF_DEFINITION = QTYPE_QOBJECT,
> 
> QTYPE_QDICT, but I get what you mean.
> 
>>     BLOCKDEV_REF_REFERENCE = QTYPE_QSTRING,
>> };
>>
>> then rewrite visit_get_next_type() to directly return the qtype, as well
>> as rewrite the generated switch statement in visit_type_BlockdevRef() to
>> directly inspect the qtypes it cares about.  In fact, that's the
>> approach I'm currently playing with.
> 
> Hmm.  The schema currently doesn't let you control enumeration values.

For public enums. But the enum for alternate is never public - you never
send the name of the branch over the wire, so we don't need to stringize
the name anywhere.

> qapi-code-gen.txt specifies:
> 
>     The enumeration values [...] are encoded as C enum integral values
>     in generated code.  While the C code starts numbering at 0, it is
>     better to use explicit comparisons to enum values than implicit
>     comparisons to 0; the C code will also include a generated enum
>     member ending in _MAX for tracking the size of the enum, useful when
>     using common functions for converting between strings and enum
>     values.
> 
> Strictly speaking, this needn't apply to implicit enums like
> BlockdevRefKind.  But is it a good idea to make them different?

I think so, but maybe under a different name (maybe
BLOCKDEV_REF_REFERENCE_QTYPE) to make it more obvious that this is not
the usual generated enum, but special to the alternate.

> 
> Hmm, your new BlockdevRefKind is basically a subset of qtype_code with
> the members renamed.  Could we simply use qtype_code directly?

We could, except that clients that manipulate the generated struct then
have to know the qtype mapping directly; while keeping symbolic names
lets them do 'foo->type = BLOCKDEV_REF_REFERENCE; foo->reference = xyz;'
as a nice visual indicator of which union member within the struct is
being assigned according to the discriminator.

I guess I'll see how much code currently manipulates the generated
structs (I already recall from other patches in this series that
blockdev played a bit loose by  validating that the QMP was okay and
then using QDict for everything else rather than the generated struct)
and make my decision when posting my RFC patch.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature