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 12/25] qapi: Start converting to new qapi union layout
Date: Thu, 22 Oct 2015 23:09:45 -0600 [thread overview]
Message-ID: <1445576998-2921-13-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 front end for a series that converts to a
saner qapi union layout. By the end of the series, we will no
longer have the type/kind mismatch, and all tag values will be
under a named union, which requires clients to access
'obj->u.value' instead of 'obj->value'. But since the
conversion touches a number of files, it is easiest if we
temporarily support BOTH layouts simultaneously.
Given a simple union qapi type:
{ 'union':'Foo', 'data': { 'a':'int', 'b':'bool' } }
make the following changes in generated qapi-types.h:
| struct Foo {
|- FooKind kind;
|- union { /* union tag is @kind */
|+ union {
|+ FooKind kind;
|+ FooKind type;
|+ };
|+ union { /* union tag is @type */
| void *data;
| int64_t a;
| bool b;
|+ union { /* union tag is @type */
|+ void *data;
|+ int64_t a;
|+ bool b;
|+ } u;
| };
| };
Flat unions do not need the anonymous union for the tag member,
as we already fixed that to use the member name instead of 'kind'
back in commit 0f61af3e.
Later, when the conversions are complete, we will remove the
duplication hacks.
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, improve commit message, defer
qapi-visit and testsuite 'u' changes to later
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, 19 insertions(+), 7 deletions(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 9c51d8f..579532d 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -148,11 +148,23 @@ 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('''
- %(c_type)s kind;
+ union {
+ %(c_type)s kind;
+ %(c_type)s type;
+ };
''',
c_type=c_name(variants.tag_member.type.name))
+ # 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
@@ -161,25 +173,25 @@ 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?
- ret += mcgen('''
+ tmp += mcgen('''
union { /* union tag is @%(c_name)s */
void *data;
''',
- # TODO ugly special case for simple union
- # Use same tag name in C as on the wire to get rid of
- # it, then: c_name=c_name(variants.tag_member.name)
- c_name=c_name(variants.tag_name or 'kind'))
+ c_name=c_name(variants.tag_member.name))
for var in variants.variants:
# Ugly special case for simple union TODO get rid of it
typ = var.simple_union_type() or var.type
- ret += mcgen('''
+ tmp += 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
next prev 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 ` Eric Blake [this message]
2015-10-23 5:09 ` [Qemu-devel] [PATCH v10 13/25] qapi-visit: Convert to new qapi union layout 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 ` [Qemu-devel] [PATCH v10 22/25] qapi: Finish converting " Eric Blake
2015-10-27 8:33 ` 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-13-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).