[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH RFC v2 26/47] qapi-types: Convert to QAPISchemaV

From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH RFC v2 26/47] qapi-types: Convert to QAPISchemaVisitor, fixing flat unions
Date: Thu, 30 Jul 2015 10:36:31 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0

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 {
> QTYPE_QDICT, but I get what you mean.
>> };
>> 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

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]