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