qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: armbru@redhat.com, Michael Roth <mdroth@linux.vnet.ibm.com>
Subject: [Qemu-devel] [PATCH v10 22/25] qapi: Finish converting to new qapi union layout
Date: Thu, 22 Oct 2015 23:09:55 -0600	[thread overview]
Message-ID: <1445576998-2921-23-git-send-email-eblake@redhat.com> (raw)
In-Reply-To: <1445576998-2921-1-git-send-email-eblake@redhat.com>

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.

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;
| };

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 <eblake@redhat.com>

---
v10: rebase to earlier changes, match commit wording
v9: new patch, but incorporates parts of v5 31/46 and Markus' RFC:
http://lists.gnu.org/archive/html/qemu-devel/2015-10/msg02236.html
---
 scripts/qapi-types.py | 26 +++++---------------------
 1 file changed, 5 insertions(+), 21 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 579532d..d131430 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -148,23 +148,10 @@ struct %(c_name)s {
     if base:
         ret += gen_struct_fields([], base)
     else:
-        # TODO As a hack, we emit both 'kind' and 'type'. Ultimately, we
-        # want to use only 'type', but the conversion is large enough to
-        # require staging over several commits.
-        ret += mcgen('''
-    union {
-        %(c_type)s kind;
-        %(c_type)s type;
-    };
-''',
-                     c_type=c_name(variants.tag_member.type.name))
+        ret += gen_struct_field(variants.tag_member.name,
+                                variants.tag_member.type,
+                                False)

-    # TODO As a hack, we emit the union twice, once as an anonymous union
-    # and once as a named union.  Ultimately, we want to use only the
-    # named union version (as it avoids conflicts between tag values as
-    # branch names competing with non-variant QMP names), but the conversion
-    # is large enough to require staging over several commits.
-    tmp = ''
     # FIXME: What purpose does data serve, besides preventing a union that
     # has a branch named 'data'? We use it in qapi-visit.py to decide
     # whether to bypass the switch statement if visiting the discriminator
@@ -173,7 +160,7 @@ 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?
-    tmp += mcgen('''
+    ret += mcgen('''
     union { /* union tag is @%(c_name)s */
         void *data;
 ''',
@@ -182,17 +169,14 @@ struct %(c_name)s {
     for var in variants.variants:
         # Ugly special case for simple union TODO get rid of it
         typ = var.simple_union_type() or var.type
-        tmp += mcgen('''
+        ret += mcgen('''
         %(c_type)s %(c_name)s;
 ''',
                      c_type=typ.c_type(),
                      c_name=c_name(var.name))

-    ret += tmp
-    ret += '    ' + '\n    '.join(tmp.split('\n'))
     ret += mcgen('''
     } u;
-    };
 };
 ''')

-- 
2.4.3

  parent reply	other threads:[~2015-10-23  5:10 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-23  5:09 [Qemu-devel] [PATCH v10 00/25] qapi collision reduction (post-introspection subset B') Eric Blake
2015-10-23  5:09 ` [Qemu-devel] [PATCH v10 01/25] tests/qapi-schema: Test for reserved names, empty struct Eric Blake
2015-10-23 12:33   ` Markus Armbruster
2015-10-23 12:39     ` Eric Blake
2015-10-23 14:17       ` Markus Armbruster
2015-10-23  5:09 ` [Qemu-devel] [PATCH v10 02/25] qapi: More idiomatic string operations Eric Blake
2015-10-23  5:09 ` [Qemu-devel] [PATCH v10 03/25] qapi: More robust conditions for when labels are needed Eric Blake
2015-10-23 12:44   ` Markus Armbruster
2015-10-23  5:09 ` [Qemu-devel] [PATCH v10 04/25] qapi: Reserve '*List' type names for list types Eric Blake
2015-10-23 12:53   ` Markus Armbruster
2015-10-23  5:09 ` [Qemu-devel] [PATCH v10 05/25] qapi: Reserve 'q_*' and 'has_*' member names Eric Blake
2015-10-23 13:05   ` Markus Armbruster
2015-10-23 14:14     ` Eric Blake
2015-10-23 14:47       ` Markus Armbruster
2015-10-23 14:52         ` Eric Blake
2015-10-23  5:09 ` [Qemu-devel] [PATCH v10 06/25] vnc: Hoist allocation of VncBasicInfo to callers Eric Blake
2015-10-23  5:09 ` [Qemu-devel] [PATCH v10 07/25] qapi-visit: Split off visit_type_FOO_fields forward decl Eric Blake
2015-10-23 13:46   ` Markus Armbruster
2015-10-23 14:35     ` Eric Blake
2015-10-23 18:05       ` Markus Armbruster
2015-10-23 19:44         ` Eric Blake
2015-10-23  5:09 ` [Qemu-devel] [PATCH v10 08/25] qapi-types: Refactor base fields output Eric Blake
2015-10-23 15:06   ` Markus Armbruster
2015-10-23 15:16     ` Eric Blake
2015-10-23  5:09 ` [Qemu-devel] [PATCH v10 09/25] qapi: Prefer typesafe upcasts to qapi base classes Eric Blake
2015-10-23 15:30   ` Markus Armbruster
2015-10-23 20:44     ` Eric Blake
2015-10-26  7:33       ` Markus Armbruster
2015-10-26 16:24         ` Eric Blake
2015-10-26 17:54           ` Markus Armbruster
2015-10-26 20:53             ` Eric Blake
2015-10-23  5:09 ` [Qemu-devel] [PATCH v10 10/25] qapi: Unbox base members Eric Blake
2015-10-23 19:14   ` Markus Armbruster
2015-10-23 19:19     ` Eric Blake
2015-10-23 20:45       ` Eric Blake
2015-10-23  5:09 ` [Qemu-devel] [PATCH v10 11/25] qapi-visit: Remove redundant functions for flat union base Eric Blake
2015-10-23 19:35   ` Markus Armbruster
2015-10-23  5:09 ` [Qemu-devel] [PATCH v10 12/25] qapi: Start converting to new qapi union layout Eric Blake
2015-10-23  5:09 ` [Qemu-devel] [PATCH v10 13/25] qapi-visit: Convert " Eric Blake
2015-10-26 17:07   ` Markus Armbruster
2015-10-26 20:39     ` Eric Blake
2015-10-27  7:08       ` Markus Armbruster
2015-10-23  5:09 ` [Qemu-devel] [PATCH v10 14/25] tests: " Eric Blake
2015-10-23  5:09 ` [Qemu-devel] [PATCH v10 15/25] block: " Eric Blake
2015-10-23  5:09 ` [Qemu-devel] [PATCH v10 16/25] sockets: " Eric Blake
2015-10-23  5:09 ` [Qemu-devel] [PATCH v10 17/25] net: " Eric Blake
2015-10-23  5:09 ` [Qemu-devel] [PATCH v10 18/25] char: " Eric Blake
2015-10-23  5:09 ` [Qemu-devel] [PATCH v10 19/25] input: " Eric Blake
2015-10-23  5:09 ` [Qemu-devel] [PATCH v10 20/25] memory: " Eric Blake
2015-10-23  5:09 ` [Qemu-devel] [PATCH v10 21/25] tpm: " Eric Blake
2015-10-23  5:09 ` Eric Blake [this message]
2015-10-27  8:33   ` [Qemu-devel] [PATCH v10 22/25] qapi: Finish converting " Markus Armbruster
2015-10-23  5:09 ` [Qemu-devel] [PATCH v10 23/25] qapi: Reserve 'u' member name Eric Blake
2015-10-26 17:27   ` Markus Armbruster
2015-10-23  5:09 ` [Qemu-devel] [PATCH v10 24/25] qapi: Remove outdated tests related to QMP/branch collisions Eric Blake
2015-10-23 23:27   ` Eric Blake
2015-10-23  5:09 ` [Qemu-devel] [PATCH v10 25/25] qapi: Simplify gen_struct_field() Eric Blake
2015-10-26 17:55 ` [Qemu-devel] [PATCH v10 00/25] qapi collision reduction (post-introspection subset B') Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1445576998-2921-23-git-send-email-eblake@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).