On 10/26/2015 04:35 PM, Eric Blake wrote: > We have two issues with our qapi union layout: > 1) Even though the QMP wire format spells the tag 'type', the > C code spells it 'kind', requiring some hacks in the generator. > 2) The C struct uses an anonymous union, which places all tag > values in the same namespace as all non-variant members. This > leads to spurious collisions if a tag value matches a QMP name.
You asked for an updated commit message (but that request was made against v10: https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg06216.html). There, you suggested "a non-variant member name" rather than "QMP name" - it works for me, but you'd want to make that change for all of 13-22/25 (since I copy-pasted the same intro to each). Or decide which ones you want to squash together. For your other comment in that mail (mentioning an example test), I think I already got close to what you asked for, but have one tweak below: > > This patch is the back end for a series that converts to a > saner qapi union layout. Now that all clients have been > converted to use 'type' and 'obj->u.value', we can drop the > temporary parallel support for 'kind' and 'obj->value'. > > Given a simple union qapi type: > > { 'union':'Foo', 'data': { 'a':'int', 'b':'bool' } } > > this is the overall effect, when compared to the state before > this series of patches: > > | struct Foo { > |- FooKind kind; > |- union { /* union tag is @kind */ > |+ FooKind type; > |+ union { /* union tag is @type */ > | void *data; > | int64_t a; > | bool b; > |- }; > |+ } u; > | }; > > There are still some further changes that can be made to the > testsuite now that there is no longer a collision between > enum tag values and QMP names, as well as adding a reservation > of 'u' to avoid a collision between QMP and our choice of union > naming, but those will be later patches. Change this paragraph to something along these lines: The testsuite still contains some examples of artificial restrictions (see flat-union-clash-type.json, for example) that are no longer technically necessary, now that there is no longer a collision between enum tag values and non-variant member names; but fixing this will be done in later patches, in part because some further changes are required to keep QAPISchema*.check() from asserting. Also, a later patch will add a reservation for the member name 'u' to avoid a collision between a user's non-variant names and our internal choice of C union name. > > Note, however, that we do not rename the generated enum, which > is still 'FooKind'. A further patch could generate implicit > enums as 'FooType', but while the generator already reserved > the '*Kind' namespace (commit 4dc2e69), there are already QMP > constructs with '*Type' naming, which means changing our > reservation namespace would have lots of churn to C code to > deal with a forced name change. > > Signed-off-by: Eric Blake <ebl...@redhat.com> > > --- -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
