[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL v2 13/25] qapi: Start converting to new qapi union la
From: |
Markus Armbruster |
Subject: |
[Qemu-devel] [PULL v2 13/25] qapi: Start converting to new qapi union layout |
Date: |
Mon, 2 Nov 2015 10:13:18 +0100 |
From: Eric Blake <address@hidden>
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 non-variant
member's 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.
One additional change is needed in qapi.py: check_union() now
needs to check for collisions with 'type' in addition to those
with 'kind'.
Later, when the conversions are complete, we will remove the
duplication hacks, and also drop the check_union() restrictions.
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 <address@hidden>
Message-Id: <address@hidden>
[Commit message tweaked slightly]
Signed-off-by: Markus Armbruster <address@hidden>
---
scripts/qapi-types.py | 26 +++++++++++++++++++-------
scripts/qapi.py | 3 ++-
2 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index faf7022..1420e00 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -149,11 +149,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
@@ -162,25 +174,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;
};
};
''')
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 3ff7b11..00a1620 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -548,7 +548,8 @@ def check_union(expr, expr_info):
base = expr.get('base')
discriminator = expr.get('discriminator')
members = expr['data']
- values = {'MAX': '(automatic)', 'KIND': '(automatic)'}
+ values = {'MAX': '(automatic)', 'KIND': '(automatic)',
+ 'TYPE': '(automatic)'}
# Two types of unions, determined by discriminator.
--
2.4.3
- [Qemu-devel] [PULL v2 07/25] vnc: Hoist allocation of VncBasicInfo to callers, (continued)
- [Qemu-devel] [PULL v2 07/25] vnc: Hoist allocation of VncBasicInfo to callers, Markus Armbruster, 2015/11/02
- [Qemu-devel] [PULL v2 10/25] qapi: Prefer typesafe upcasts to qapi base classes, Markus Armbruster, 2015/11/02
- [Qemu-devel] [PULL v2 03/25] qapi: More idiomatic string operations, Markus Armbruster, 2015/11/02
- [Qemu-devel] [PULL v2 04/25] qapi: More robust conditions for when labels are needed, Markus Armbruster, 2015/11/02
- [Qemu-devel] [PULL v2 02/25] tests/qapi-schema: Test for reserved names, empty struct, Markus Armbruster, 2015/11/02
- [Qemu-devel] [PULL v2 14/25] qapi-visit: Convert to new qapi union layout, Markus Armbruster, 2015/11/02
- [Qemu-devel] [PULL v2 22/25] tpm: Convert to new qapi union layout, Markus Armbruster, 2015/11/02
- [Qemu-devel] [PULL v2 06/25] qapi: Reserve 'q_*' and 'has_*' member names, Markus Armbruster, 2015/11/02
- [Qemu-devel] [PULL v2 12/25] qapi-visit: Remove redundant functions for flat union base, Markus Armbruster, 2015/11/02
- [Qemu-devel] [PULL v2 09/25] qapi-types: Refactor base fields output, Markus Armbruster, 2015/11/02
- [Qemu-devel] [PULL v2 13/25] qapi: Start converting to new qapi union layout,
Markus Armbruster <=
- [Qemu-devel] [PULL v2 11/25] qapi: Unbox base members, Markus Armbruster, 2015/11/02
- [Qemu-devel] [PULL v2 18/25] net: Convert to new qapi union layout, Markus Armbruster, 2015/11/02
- [Qemu-devel] [PULL v2 15/25] tests: Convert to new qapi union layout, Markus Armbruster, 2015/11/02
- [Qemu-devel] [PULL v2 16/25] block: Convert to new qapi union layout, Markus Armbruster, 2015/11/02
- [Qemu-devel] [PULL v2 20/25] input: Convert to new qapi union layout, Markus Armbruster, 2015/11/02
- [Qemu-devel] [PULL v2 17/25] sockets: Convert to new qapi union layout, Markus Armbruster, 2015/11/02
- [Qemu-devel] [PULL v2 23/25] qapi: Finish converting to new qapi union layout, Markus Armbruster, 2015/11/02
- [Qemu-devel] [PULL v2 25/25] qapi: Simplify gen_struct_field(), Markus Armbruster, 2015/11/02
- [Qemu-devel] [PULL v2 21/25] memory: Convert to new qapi union layout, Markus Armbruster, 2015/11/02
- [Qemu-devel] [PULL v2 24/25] qapi: Reserve 'u' member name, Markus Armbruster, 2015/11/02