david.venhoff/QEMU-Nyx-fork
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

qapi-visit: Convert to QAPISchemaVisitor, fixing bugs

Browse Source 
Fixes flat unions to visit the base's base members (the previous
commit merely added them to the struct).  Same test case.

Patch's effect on visit_type_UserDefFlatUnion():

     static void visit_type_UserDefFlatUnion_fields(Visitor *m, UserDefFlatUnion **obj, Error **errp)
     {
         Error *err = NULL;

    +    visit_type_int(m, &(*obj)->integer, "integer", &err);
    +    if (err) {
    +        goto out;
    +    }
         visit_type_str(m, &(*obj)->string, "string", &err);
         if (err) {
             goto out;

Test cases updated for the bug fix.

Fixes alternates to generate a visitor for their implicit enumeration
type.  None of them are currently used, obviously.  Example:
block-core.json's BlockdevRef now generates
visit_type_BlockdevRefKind().

Code is generated in a different order now, and therefore has got a
few new forward declarations.  Doesn't matter.

The guard QAPI_VISIT_BUILTIN_VISITOR_DECL is renamed to
QAPI_VISIT_BUILTIN.

The previous commit's two ugly special cases exist here, too.  Mark
both TODO.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
This commit is contained in:
Markus Armbruster 2015-09-16 13:06:10 +02:00
parent 2b162ccbe8
commit 441cbac0c7
4 changed files with 117 additions and 164 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

270
scripts/qapi-visit.py
View File

@ -12,7 +12,6 @@
# This work is licensed under the terms of the GNU GPL, version 2.
# See the COPYING file in the top-level directory.
from ordereddict import OrderedDict
from qapi import *
import re
@ -24,13 +23,13 @@ def generate_visit_implicit_struct(type):
return ''
implicit_structs_seen.add(type)
ret = ''
if type not in struct_fields_seen:
if type.name not in struct_fields_seen:
# Need a forward declaration
ret += mcgen('''
static void visit_type_%(c_type)s_fields(Visitor *m, %(c_type)s **obj, Error **errp);
''',
c_type=type_name(type))
c_type=type.c_name())
ret += mcgen('''
@ -46,7 +45,7 @@ static void visit_type_implicit_%(c_type)s(Visitor *m, %(c_type)s **obj, Error *
error_propagate(errp, err);
}
''',
c_type=type_name(type))
c_type=type.c_name())
return ret
def generate_visit_struct_fields(name, members, base = None):
@ -74,24 +73,24 @@ if (err) {
goto out;
}
''',
type=type_name(base), c_name=c_name('base'))
type=base.c_name(), c_name=c_name('base'))
for argname, argentry, optional in parse_args(members):
if optional:
for memb in members:
if memb.optional:
ret += mcgen('''
visit_optional(m, &(*obj)->has_%(c_name)s, "%(name)s", &err);
if (!err && (*obj)->has_%(c_name)s) {
''',
c_name=c_name(argname), name=argname)
c_name=c_name(memb.name), name=memb.name)
push_indent()
ret += mcgen('''
visit_type_%(type)s(m, &(*obj)->%(c_name)s, "%(name)s", &err);
''',
type=type_name(argentry), c_name=c_name(argname),
name=argname)
type=memb.type.c_name(), c_name=c_name(memb.name),
name=memb.name)
if optional:
if memb.optional:
pop_indent()
ret += mcgen('''
}
@ -136,12 +135,7 @@ def generate_visit_struct_body(name):
return ret
def generate_visit_struct(expr):
name = expr['struct']
members = expr['data']
base = expr.get('base')
def gen_visit_struct(name, base, members):
ret = generate_visit_struct_fields(name, members, base)
ret += mcgen('''
@ -158,10 +152,10 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **e
''')
return ret
def generate_visit_list(name):
def gen_visit_list(name, element_type):
return mcgen('''
void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name, Error **errp)
void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **errp)
{
Error *err = NULL;
GenericList *i, **prev;
@ -174,8 +168,8 @@ void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name, E
for (prev = (GenericList **)obj;
!err && (i = visit_next_list(m, prev, &err)) != NULL;
prev = &i) {
%(name)sList *native_i = (%(name)sList *)i;
visit_type_%(name)s(m, &native_i->value, NULL, &err);
%(name)s *native_i = (%(name)s *)i;
visit_type_%(c_elt_type)s(m, &native_i->value, NULL, &err);
}
error_propagate(errp, err);
@ -185,7 +179,8 @@ out:
error_propagate(errp, err);
}
''',
name=type_name(name))
name=c_name(name),
c_elt_type=element_type.c_name())
def generate_visit_enum(name):
return mcgen('''
@ -197,7 +192,7 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s *obj, const char *name, Error
''',
c_name=c_name(name), name=name)
def generate_visit_alternate(name, members):
def gen_visit_alternate(name, variants):
ret = mcgen('''
void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **errp)
@ -216,25 +211,17 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **e
''',
name=c_name(name))
# For alternate, always use the default enum type automatically generated
# as name + 'Kind'
disc_type = c_name(name) + 'Kind'
for key in members:
assert (members[key] in builtin_types.keys()
or find_struct(members[key])
or find_union(members[key])
or find_enum(members[key])), "Invalid alternate member"
enum_full_value = c_enum_const(disc_type, key)
for var in variants.variants:
enum_full_value = c_enum_const(variants.tag_member.type.name,
var.name)
ret += mcgen('''
case %(enum_full_value)s:
visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, name, &err);
break;
''',
enum_full_value = enum_full_value,
c_type = type_name(members[key]),
c_name = c_name(key))
c_type=var.type.c_name(),
c_name=c_name(var.name))
ret += mcgen('''
default:
@ -251,35 +238,17 @@ out:
return ret
def generate_visit_union(expr):
name = expr['union']
members = expr['data']
base = expr.get('base')
discriminator = expr.get('discriminator')
enum_define = discriminator_find_enum_define(expr)
if enum_define:
# Use the enum type as discriminator
ret = ""
disc_type = c_name(enum_define['enum_name'])
else:
# There will always be a discriminator in the C switch code, by default
# it is an enum type generated silently
ret = generate_visit_enum(name + 'Kind')
disc_type = c_name(name) + 'Kind'
def gen_visit_union(name, base, variants):
ret = ''
if base:
assert discriminator
base_fields = find_struct(base)['data'].copy()
del base_fields[discriminator]
ret += generate_visit_struct_fields(name, base_fields)
members = [m for m in base.members if m != variants.tag_member]
ret += generate_visit_struct_fields(name, members)
if discriminator:
for key in members:
ret += generate_visit_implicit_struct(members[key])
for var in variants.variants:
# Ugly special case for simple union TODO get rid of it
if not var.simple_union_type():
ret += generate_visit_implicit_struct(var.type)
ret += mcgen('''
@ -304,41 +273,44 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error
''',
name=c_name(name))
if not discriminator:
tag = 'kind'
disc_key = "type"
else:
tag = discriminator
disc_key = discriminator
disc_key = variants.tag_member.name
if not variants.tag_name:
# we pointlessly use a different key for simple unions
disc_key = 'type'
ret += mcgen('''
visit_type_%(disc_type)s(m, &(*obj)->%(c_tag)s, "%(disc_key)s", &err);
visit_type_%(disc_type)s(m, &(*obj)->%(c_name)s, "%(disc_key)s", &err);
if (err) {
goto out_obj;
}
if (!visit_start_union(m, !!(*obj)->data, &err) || err) {
goto out_obj;
}
switch ((*obj)->%(c_tag)s) {
switch ((*obj)->%(c_name)s) {
''',
disc_type = disc_type,
c_tag=c_name(tag),
disc_type=variants.tag_member.type.c_name(),
# 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'),
disc_key = disc_key)
for key in members:
if not discriminator:
for var in variants.variants:
# TODO ugly special case for simple union
simple_union_type = var.simple_union_type()
if simple_union_type:
fmt = 'visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "data", &err);'
else:
fmt = 'visit_type_implicit_%(c_type)s(m, &(*obj)->%(c_name)s, &err);'
enum_full_value = c_enum_const(disc_type, key)
enum_full_value = c_enum_const(variants.tag_member.type.name, var.name)
ret += mcgen('''
case %(enum_full_value)s:
''' + fmt + '''
break;
''',
enum_full_value = enum_full_value,
c_type=type_name(members[key]),
c_name=c_name(key))
c_type=(simple_union_type or var.type).c_name(),
c_name=c_name(var.name))
ret += mcgen('''
default:
@ -359,38 +331,68 @@ out:
return ret
def generate_declaration(name, builtin_type=False):
ret = ""
if not builtin_type:
name = c_name(name)
ret += mcgen('''
void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **errp);
''',
name=name)
ret += mcgen('''
void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name, Error **errp);
''',
name=name)
return ret
def generate_enum_declaration(name):
ret = mcgen('''
void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name, Error **errp);
''',
name=c_name(name))
return ret
def generate_decl_enum(name):
def gen_visit_decl(name, scalar=False):
c_type = c_name(name) + ' *'
if not scalar:
c_type += '*'
return mcgen('''
void visit_type_%(name)s(Visitor *m, %(name)s *obj, const char *name, Error **errp);
void visit_type_%(c_name)s(Visitor *m, %(c_type)sobj, const char *name, Error **errp);
''',
name=c_name(name))
c_name=c_name(name), c_type=c_type)
class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
def __init__(self):
self.decl = None
self.defn = None
self._btin = None
def visit_begin(self, schema):
self.decl = ''
self.defn = ''
self._btin = guardstart('QAPI_VISIT_BUILTIN')
def visit_end(self):
# To avoid header dependency hell, we always generate
# declarations for built-in types in our header files and
# simply guard them. See also do_builtins (command line
# option -b).
self._btin += guardend('QAPI_VISIT_BUILTIN')
self.decl = self._btin + self.decl
self._btin = None
def visit_enum_type(self, name, info, values, prefix):
self.decl += gen_visit_decl(name, scalar=True)
self.defn += generate_visit_enum(name)
def visit_array_type(self, name, info, element_type):
decl = gen_visit_decl(name)
defn = gen_visit_list(name, element_type)
if isinstance(element_type, QAPISchemaBuiltinType):
self._btin += decl
if do_builtins:
self.defn += defn
else:
self.decl += decl
self.defn += defn
def visit_object_type(self, name, info, base, members, variants):
if info:
self.decl += gen_visit_decl(name)
if variants:
assert not members # not implemented
self.defn += gen_visit_union(name, base, variants)
else:
self.defn += gen_visit_struct(name, base, members)
def visit_alternate_type(self, name, info, variants):
self.decl += gen_visit_decl(name)
self.defn += gen_visit_alternate(name, variants)
# If you link code generated from multiple schemata, you want only one
# instance of the code for built-in types. Generate it only when
# do_builtins, enabled by command line option -b. See also
# QAPISchemaGenVisitVisitor.visit_end().
do_builtins = False
(input_file, output_dir, do_c, do_h, prefix, opts) = \
@ -446,56 +448,10 @@ fdecl.write(mcgen('''
''',
prefix=prefix))
exprs = QAPISchema(input_file).get_exprs()
# to avoid header dependency hell, we always generate declarations
# for built-in types in our header files and simply guard them
fdecl.write(guardstart("QAPI_VISIT_BUILTIN_VISITOR_DECL"))
for typename in builtin_types.keys():
fdecl.write(generate_declaration(typename, builtin_type=True))
fdecl.write(guardend("QAPI_VISIT_BUILTIN_VISITOR_DECL"))
# ...this doesn't work for cases where we link in multiple objects that
# have the functions defined, so we use -b option to provide control
# over these cases
if do_builtins:
for typename in builtin_types.keys():
fdef.write(generate_visit_list(typename))
for expr in exprs:
if expr.has_key('struct'):
ret = generate_visit_struct(expr)
ret += generate_visit_list(expr['struct'])
fdef.write(ret)
ret = generate_declaration(expr['struct'])
fdecl.write(ret)
elif expr.has_key('union'):
ret = generate_visit_union(expr)
ret += generate_visit_list(expr['union'])
fdef.write(ret)
enum_define = discriminator_find_enum_define(expr)
ret = ""
if not enum_define:
ret = generate_decl_enum('%sKind' % expr['union'])
ret += generate_declaration(expr['union'])
fdecl.write(ret)
elif expr.has_key('alternate'):
ret = generate_visit_alternate(expr['alternate'], expr['data'])
ret += generate_visit_list(expr['alternate'])
fdef.write(ret)
ret = generate_decl_enum('%sKind' % expr['alternate'])
ret += generate_declaration(expr['alternate'])
fdecl.write(ret)
elif expr.has_key('enum'):
ret = generate_visit_list(expr['enum'])
ret += generate_visit_enum(expr['enum'])
fdef.write(ret)
ret = generate_decl_enum(expr['enum'])
ret += generate_enum_declaration(expr['enum'])
fdecl.write(ret)
schema = QAPISchema(input_file)
gen = QAPISchemaGenVisitVisitor()
schema.visit(gen)
fdef.write(gen.defn)
fdecl.write(gen.decl)
close_output(fdef, fdecl)

3
tests/qapi-schema/qapi-schema-test.json
View File

@ -44,8 +44,6 @@
'data': { 'value1' : 'UserDefA',
'value2' : 'UserDefB',
'value3' : 'UserDefB' } }
# FIXME generated visit_type_UserDefFlatUnion_fields() fails to visit
# members of indirect base UserDefZero
{ 'struct': 'UserDefUnionBase',
'base': 'UserDefZero',
@ -62,7 +60,6 @@
{ 'alternate': 'UserDefAlternate',
'data': { 'uda': 'UserDefA', 's': 'str', 'i': 'int' } }
# FIXME only a declaration of visit_type_UserDefAlternateKind() generated
{ 'struct': 'UserDefC',
'data': { 'string1': 'str', 'string2': 'str' } }

4
tests/test-qmp-input-strict.c
View File

@ -167,9 +167,9 @@ static void test_validate_union_flat(TestInputVisitorData *data,
v = validate_test_init(data,
"{ 'enum1': 'value1', "
"'integer': 41, "
"'string': 'str', "
"'boolean': true }");
/* TODO when generator bug is fixed, add 'integer': 41 */
visit_type_UserDefFlatUnion(v, &tmp, NULL, &err);
g_assert(!err);
@ -272,7 +272,7 @@ static void test_validate_fail_union_flat_no_discrim(TestInputVisitorData *data,
Visitor *v;
/* test situation where discriminator field ('enum1' here) is missing */
v = validate_test_init(data, "{ 'string': 'c', 'string1': 'd', 'string2': 'e' }");
v = validate_test_init(data, "{ 'integer': 42, 'string': 'c', 'string1': 'd', 'string2': 'e' }");
visit_type_UserDefFlatUnion2(v, &tmp, NULL, &err);
g_assert(err);

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

@ -307,15 +307,15 @@ static void test_visitor_in_union_flat(TestInputVisitorData *data,
v = visitor_input_test_init(data,
"{ 'enum1': 'value1', "
"'integer': 41, "
"'string': 'str', "
"'boolean': true }");
/* TODO when generator bug is fixed, add 'integer': 41 */
visit_type_UserDefFlatUnion(v, &tmp, NULL, &err);
g_assert(err == NULL);
g_assert_cmpint(tmp->enum1, ==, ENUM_ONE_VALUE1);
g_assert_cmpstr(tmp->string, ==, "str");
/* TODO g_assert_cmpint(tmp->integer, ==, 41); */
g_assert_cmpint(tmp->integer, ==, 41);
g_assert_cmpint(tmp->value1->boolean, ==, true);
qapi_free_UserDefFlatUnion(tmp);
}