diff options
author | Eric Blake <eblake@redhat.com> | 2016-02-17 23:48:26 -0700 |
---|---|---|
committer | Markus Armbruster <armbru@redhat.com> | 2016-02-19 11:08:57 +0100 |
commit | becceedc4d9bc1435099c90a0514945a89844d3a (patch) | |
tree | 514ffcbc4c7fecc0a372808baa380ec35b97b9a0 /scripts | |
parent | 2208d64998c5f867ccee7eeee298971685bf822d (diff) | |
download | qemu-becceedc4d9bc1435099c90a0514945a89844d3a.zip qemu-becceedc4d9bc1435099c90a0514945a89844d3a.tar.gz qemu-becceedc4d9bc1435099c90a0514945a89844d3a.tar.bz2 |
qapi: Don't box struct branch of alternate
There's no reason to do two malloc's for an alternate type visiting
a QAPI struct; let's just inline the struct directly as the C union
branch of the struct.
Surprisingly, no clients were actually using the struct member prior
to this patch outside of the testsuite; an earlier patch in the series
added some testsuite coverage to make the effect of this patch more
obvious.
In qapi.py, c_type() gains a new is_unboxed flag to control when we
are emitting a C struct unboxed within the context of an outer
struct (different from our other two modes of usage with no flags
for normal local variable declarations, and with is_param for adding
'const' in a parameter list). I don't know if there is any more
pythonic way of collapsing the two flags into a single parameter,
as we never have a caller setting both flags at once.
Ultimately, we want to also unbox branches for QAPI unions, but as
that touches a lot more client code, it is better as separate
patches. But since unions and alternates share gen_variants(), I
had to hack in a way to test if we are visiting an alternate type
for setting the is_unboxed flag: look for a non-object branch.
This works because alternates have at least two branches, with at
most one object branch, while unions have only object branches.
The hack will go away in a later patch.
The generated code difference to qapi-types.h is relatively small:
| struct BlockdevRef {
| QType type;
| union { /* union tag is @type */
| void *data;
|- BlockdevOptions *definition;
|+ BlockdevOptions definition;
| char *reference;
| } u;
| };
The corresponding spot in qapi-visit.c calls visit_type_FOO(), which
first calls visit_start_struct() to allocate or deallocate the member
and handle a layer of {} from the JSON stream, then visits the
members. To peel off the indirection and the memory management that
comes with it, we inline this call, then suppress allocation /
deallocation by passing NULL to visit_start_struct(), and adjust the
member visit:
| switch ((*obj)->type) {
| case QTYPE_QDICT:
|- visit_type_BlockdevOptions(v, name, &(*obj)->u.definition, &err);
|+ visit_start_struct(v, name, NULL, 0, &err);
|+ if (err) {
|+ break;
|+ }
|+ visit_type_BlockdevOptions_fields(v, &(*obj)->u.definition, &err);
|+ error_propagate(errp, err);
|+ err = NULL;
|+ visit_end_struct(v, &err);
| break;
| case QTYPE_QSTRING:
| visit_type_str(v, name, &(*obj)->u.reference, &err);
The visit of non-object fields is unchanged.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1455778109-6278-13-git-send-email-eblake@redhat.com>
[Commit message tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Diffstat (limited to 'scripts')
-rw-r--r-- | scripts/qapi-types.py | 10 | ||||
-rw-r--r-- | scripts/qapi-visit.py | 33 | ||||
-rw-r--r-- | scripts/qapi.py | 12 |
3 files changed, 43 insertions, 12 deletions
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index 6ea0ae6..4dabe91 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -116,6 +116,14 @@ static inline %(base)s *qapi_%(c_name)s_base(const %(c_name)s *obj) def gen_variants(variants): + # HACK: Determine if this is an alternate (at least one variant + # is not an object); unions have all branches as objects. + unboxed = False + for v in variants.variants: + if not isinstance(v.type, QAPISchemaObjectType): + unboxed = True + break + # 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 @@ -136,7 +144,7 @@ def gen_variants(variants): ret += mcgen(''' %(c_type)s %(c_name)s; ''', - c_type=typ.c_type(), + c_type=typ.c_type(is_unboxed=unboxed), c_name=c_name(var.name)) ret += mcgen(''' diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index d7d7ed5..f4e38d1 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -201,11 +201,14 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s *obj, Error def gen_visit_alternate(name, variants): promote_int = 'true' + ret = '' for var in variants.variants: if var.type.alternate_qtype() == 'QTYPE_QINT': promote_int = 'false' + if isinstance(var.type, QAPISchemaObjectType): + ret += gen_visit_fields_decl(var.type) - ret = mcgen(''' + ret += mcgen(''' void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error **errp) { @@ -221,17 +224,35 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error } switch ((*obj)->type) { ''', - c_name=c_name(name), promote_int=promote_int) + c_name=c_name(name), promote_int=promote_int) for var in variants.variants: ret += mcgen(''' case %(case)s: +''', + case=var.type.alternate_qtype()) + if isinstance(var.type, QAPISchemaObjectType): + ret += mcgen(''' + visit_start_struct(v, name, NULL, 0, &err); + if (err) { + break; + } + visit_type_%(c_type)s_fields(v, &(*obj)->u.%(c_name)s, &err); + error_propagate(errp, err); + err = NULL; + visit_end_struct(v, &err); +''', + c_type=var.type.c_name(), + c_name=c_name(var.name)) + else: + ret += mcgen(''' visit_type_%(c_type)s(v, name, &(*obj)->u.%(c_name)s, &err); - break; ''', - case=var.type.alternate_qtype(), - c_type=var.type.c_name(), - c_name=c_name(var.name)) + c_type=var.type.c_name(), + c_name=c_name(var.name)) + ret += mcgen(''' + break; +''') ret += mcgen(''' default: diff --git a/scripts/qapi.py b/scripts/qapi.py index 17bf633..8497777 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -824,7 +824,7 @@ class QAPISchemaVisitor(object): class QAPISchemaType(QAPISchemaEntity): - def c_type(self, is_param=False): + def c_type(self, is_param=False, is_unboxed=False): return c_name(self.name) + pointer_suffix def c_null(self): @@ -857,7 +857,7 @@ class QAPISchemaBuiltinType(QAPISchemaType): def c_name(self): return self.name - def c_type(self, is_param=False): + def c_type(self, is_param=False, is_unboxed=False): if is_param and self.name == 'str': return 'const ' + self._c_type_name return self._c_type_name @@ -891,7 +891,7 @@ class QAPISchemaEnumType(QAPISchemaType): # See QAPISchema._make_implicit_enum_type() return self.name.endswith('Kind') - def c_type(self, is_param=False): + def c_type(self, is_param=False, is_unboxed=False): return c_name(self.name) def member_names(self): @@ -987,9 +987,11 @@ class QAPISchemaObjectType(QAPISchemaType): assert not self.is_implicit() return QAPISchemaType.c_name(self) - def c_type(self, is_param=False): + def c_type(self, is_param=False, is_unboxed=False): assert not self.is_implicit() - return QAPISchemaType.c_type(self) + if is_unboxed: + return c_name(self.name) + return c_name(self.name) + pointer_suffix def json_type(self): return 'object' |