qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH RFC v4 10/32] qapi-types: Convert to QAPISchemaV


From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH RFC v4 10/32] qapi-types: Convert to QAPISchemaVisitor, fixing flat unions
Date: Thu, 3 Sep 2015 21:11:54 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0

On 09/03/2015 08:30 AM, Markus Armbruster wrote:
> Fixes flat unions to get the base's base members.  Test case is from
> commit 2fc0043, in qapi-schema-test.json:
> 

> 
> Signed-off-by: Markus Armbruster <address@hidden>
> Reviewed-by: Eric Blake <address@hidden>
> ---
>  docs/qapi-code-gen.txt                  |  51 +++---
>  scripts/qapi-types.py                   | 273 
> ++++++++++++++------------------
>  tests/qapi-schema/qapi-schema-test.json |   4 +-
>  3 files changed, 144 insertions(+), 184 deletions(-)

> @@ -231,39 +200,36 @@ struct %(name)s {
>      union { /* union tag is @%(c_name)s */
>          void *data;
>  ''',
> -                 c_name=c_name(discriminator or 'kind'))
> +                 c_name=c_name(variants.tag_member.name))

See my review on 30/32.  If we change tag_member to match the wire
spelling of 'type' in patch 2, then this patch needs a slight
alteration.  (Of course, I've already posted a potential followup patch,
currently for use after your series lands, that fixes the C spelling to
quit the pointless rename to 'kind'; maybe we should instead fold that
in to the early part of this series? If not, it will be one of our
earlier followups).

Here's the diff that got the bug in 30/32 fixed, along with changes to
the two other files.

diff --git i/scripts/qapi-types.py w/scripts/qapi-types.py
index 70a57c0..83fc421 100644
--- i/scripts/qapi-types.py
+++ w/scripts/qapi-types.py
@@ -139,11 +139,12 @@ struct %(c_name)s {
     # should not be any data leaks even without a data pointer.  Or, if
     # 'data' is merely added to guarantee we don't have an empty union,
     # shouldn't we enforce that at .json parse time?
+    # FIXME: Use same tag name in C as on the wire
     ret += mcgen('''
     union { /* union tag is @%(c_name)s */
         void *data;
 ''',
-                 c_name=c_name(variants.tag_member.name))
+                 c_name=c_name(variants.tag_name or 'kind'))

     for var in variants.variants:
         # TODO ugly special case for simple union


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