OpenE2K/qemu-e2k
13
4
Fork 0
Code Issues 4 Pull Requests Projects 2 Releases Activity

qapi: Don't box struct branch of alternate

Browse Source 
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>
This commit is contained in:
Eric Blake 2016-02-17 23:48:26 -07:00 committed by Markus Armbruster
parent 2208d64998
commit becceedc4d
5 changed files with 59 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
scripts/qapi-types.py
View File

@ -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('''

35
scripts/qapi-visit.py
View File

@ -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:
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))
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);
''',
c_type=var.type.c_name(),
c_name=c_name(var.name))
ret += mcgen('''
break;
''')
ret += mcgen('''
default:

12
scripts/qapi.py
View File

@ -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'

20
tests/test-qmp-input-visitor.c
View File

@ -327,11 +327,11 @@ static void test_visitor_in_alternate(TestInputVisitorData *data,
"'enum1':'value1', 'boolean':true}");
visit_type_UserDefAlternate(v, NULL, &tmp, &error_abort);
g_assert_cmpint(tmp->type, ==, QTYPE_QDICT);
g_assert_cmpint(tmp->u.udfu->integer, ==, 1);
g_assert_cmpstr(tmp->u.udfu->string, ==, "str");
g_assert_cmpint(tmp->u.udfu->enum1, ==, ENUM_ONE_VALUE1);
g_assert_cmpint(tmp->u.udfu->u.value1->boolean, ==, true);
g_assert_cmpint(tmp->u.udfu->u.value1->has_a_b, ==, false);
g_assert_cmpint(tmp->u.udfu.integer, ==, 1);
g_assert_cmpstr(tmp->u.udfu.string, ==, "str");
g_assert_cmpint(tmp->u.udfu.enum1, ==, ENUM_ONE_VALUE1);
g_assert_cmpint(tmp->u.udfu.u.value1->boolean, ==, true);
g_assert_cmpint(tmp->u.udfu.u.value1->has_a_b, ==, false);
qapi_free_UserDefAlternate(tmp);
v = visitor_input_test_init(data, "false");
@ -355,11 +355,11 @@ static void test_visitor_in_alternate(TestInputVisitorData *data,
"'enum1':'value1', 'boolean':true} }");
visit_type_WrapAlternate(v, NULL, &wrap, &error_abort);
g_assert_cmpint(wrap->alt->type, ==, QTYPE_QDICT);
g_assert_cmpint(wrap->alt->u.udfu->integer, ==, 1);
g_assert_cmpstr(wrap->alt->u.udfu->string, ==, "str");
g_assert_cmpint(wrap->alt->u.udfu->enum1, ==, ENUM_ONE_VALUE1);
g_assert_cmpint(wrap->alt->u.udfu->u.value1->boolean, ==, true);
g_assert_cmpint(wrap->alt->u.udfu->u.value1->has_a_b, ==, false);
g_assert_cmpint(wrap->alt->u.udfu.integer, ==, 1);
g_assert_cmpstr(wrap->alt->u.udfu.string, ==, "str");
g_assert_cmpint(wrap->alt->u.udfu.enum1, ==, ENUM_ONE_VALUE1);
g_assert_cmpint(wrap->alt->u.udfu.u.value1->boolean, ==, true);
g_assert_cmpint(wrap->alt->u.udfu.u.value1->has_a_b, ==, false);
qapi_free_WrapAlternate(wrap);
}

11
tests/test-qmp-output-visitor.c
View File

@ -457,12 +457,11 @@ static void test_visitor_out_alternate(TestOutputVisitorData *data,
tmp = g_new0(UserDefAlternate, 1);
tmp->type = QTYPE_QDICT;
tmp->u.udfu = g_new0(UserDefFlatUnion, 1);
tmp->u.udfu->integer = 1;
tmp->u.udfu->string = g_strdup("str");
tmp->u.udfu->enum1 = ENUM_ONE_VALUE1;
tmp->u.udfu->u.value1 = g_new0(UserDefA, 1);
tmp->u.udfu->u.value1->boolean = true;
tmp->u.udfu.integer = 1;
tmp->u.udfu.string = g_strdup("str");
tmp->u.udfu.enum1 = ENUM_ONE_VALUE1;
tmp->u.udfu.u.value1 = g_new0(UserDefA, 1);
tmp->u.udfu.u.value1->boolean = true;
visit_type_UserDefAlternate(data->ov, NULL, &tmp, &error_abort);
arg = qmp_output_get_qobject(data->qov);