* [Qemu-devel] [PATCH RFC 2/5] qapi: Generate consistent variable names
2015-08-20 0:04 [Qemu-devel] [PATCH RFC 0/5] qapi: support anonymous inline base Eric Blake
2015-08-20 0:04 ` [Qemu-devel] [PATCH RFC 1/5] qapi: Reuse code for flat union base validation Eric Blake
@ 2015-08-20 0:04 ` Eric Blake
2015-08-20 0:04 ` [Qemu-devel] [PATCH RFC 3/5] qapi: Merge generation of per-member visits Eric Blake
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Eric Blake @ 2015-08-20 0:04 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
Code consolidation is easier if similar code uses the same variable
naming conventions. Prefer 'err' over 'local_err' in the command
and event marshallers, and prefer 'v' over 'm' as the name of the
Visitor object in the visitor code. The resulting changes have no
impact to compiled code behavior (only to debug symbols).
Signed-off-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi-commands.py | 22 ++++++++--------
scripts/qapi-event.py | 22 ++++++++--------
scripts/qapi-visit.py | 66 ++++++++++++++++++++++++------------------------
3 files changed, 55 insertions(+), 55 deletions(-)
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index d75a399..a30f79a 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -53,14 +53,14 @@ def gen_call(name, arg_type, box, ret_type):
push_indent()
ret = mcgen('''
-%(lhs)sqmp_%(c_name)s(%(args)s&local_err);
+%(lhs)sqmp_%(c_name)s(%(args)s&err);
''',
c_name=c_name(name), args=argstr, lhs=lhs)
if ret_type:
- ret += gen_err_check('local_err')
+ ret += gen_err_check('err')
ret += mcgen('''
-qmp_marshal_output_%(c_name)s(retval, ret, &local_err);
+qmp_marshal_output_%(c_name)s(retval, ret, &err);
''',
c_name=ret_type.c_name())
pop_indent()
@@ -68,7 +68,7 @@ qmp_marshal_output_%(c_name)s(retval, ret, &local_err);
def gen_marshal_vars(arg_type, box, ret_type):
ret = mcgen('''
- Error *local_err = NULL;
+ Error *err = NULL;
''')
push_indent()
@@ -131,8 +131,8 @@ md = qapi_dealloc_visitor_new();
v = qapi_dealloc_get_visitor(md);
''')
else:
- errparg = '&local_err'
- errarg = 'local_err'
+ errparg = '&err'
+ errarg = 'err'
ret += mcgen('''
v = qmp_input_get_visitor(mi);
''')
@@ -182,20 +182,20 @@ def gen_marshal_output(ret_type):
static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out, Error **errp)
{
- Error *local_err = NULL;
+ Error *err = NULL;
QmpOutputVisitor *mo = qmp_output_visitor_new();
QapiDeallocVisitor *md;
Visitor *v;
v = qmp_output_get_visitor(mo);
- visit_type_%(c_name)s(v, &ret_in, "unused", &local_err);
- if (local_err) {
+ visit_type_%(c_name)s(v, &ret_in, "unused", &err);
+ if (err) {
goto out;
}
*ret_out = qmp_output_get_qobject(mo);
out:
- error_propagate(errp, local_err);
+ error_propagate(errp, err);
qmp_output_visitor_cleanup(mo);
md = qapi_dealloc_visitor_new();
v = qapi_dealloc_get_visitor(md);
@@ -235,7 +235,7 @@ def gen_marshal(name, arg_type, box, ret_type):
out:
''')
ret += mcgen('''
- error_propagate(errp, local_err);
+ error_propagate(errp, err);
''')
ret += gen_marshal_input_visit(arg_type, box, dealloc=True)
ret += mcgen('''
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index 17f0c12..3111185 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -31,7 +31,7 @@ def gen_event_send(name, arg_type, box):
%(proto)s
{
QDict *qmp;
- Error *local_err = NULL;
+ Error *err = NULL;
QMPEventFuncEmit emit;
''',
proto=gen_event_send_proto(name, arg_type, box))
@@ -70,8 +70,8 @@ def gen_event_send(name, arg_type, box):
if box:
ret += mcgen('''
- visit_type_%(c_name)s(v, &arg, NULL, &local_err);
- if (local_err) {
+ visit_type_%(c_name)s(v, &arg, NULL, &err);
+ if (err) {
goto clean;
}
''',
@@ -79,8 +79,8 @@ def gen_event_send(name, arg_type, box):
else:
ret += mcgen('''
/* Fake visit, as if all members are under a structure */
- visit_start_struct(v, NULL, "", "%(name)s", 0, &local_err);
- if (local_err) {
+ visit_start_struct(v, NULL, "", "%(name)s", 0, &err);
+ if (err) {
goto clean;
}
''',
@@ -101,8 +101,8 @@ def gen_event_send(name, arg_type, box):
cast = ''
ret += mcgen('''
- visit_type_%(c_type)s(v, %(cast)s&%(c_name)s, "%(name)s", &local_err);
- if (local_err) {
+ visit_type_%(c_type)s(v, %(cast)s&%(c_name)s, "%(name)s", &err);
+ if (err) {
goto clean;
}
''',
@@ -119,8 +119,8 @@ def gen_event_send(name, arg_type, box):
ret += mcgen('''
- visit_end_struct(v, &local_err);
- if (local_err) {
+ visit_end_struct(v, &err);
+ if (err) {
goto clean;
}
@@ -131,7 +131,7 @@ def gen_event_send(name, arg_type, box):
''')
ret += mcgen('''
- emit(%(c_enum)s, qmp, &local_err);
+ emit(%(c_enum)s, qmp, &err);
''',
c_enum=c_enum_const(event_enum_name, name))
@@ -142,7 +142,7 @@ def gen_event_send(name, arg_type, box):
qmp_output_visitor_cleanup(qov);
''')
ret += mcgen('''
- error_propagate(errp, local_err);
+ error_propagate(errp, err);
QDECREF(qmp);
}
''')
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index e4f7c89..1fe6c64 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -23,7 +23,7 @@ def gen_visit_decl(name, scalar=False):
if not scalar:
c_type += '*'
return mcgen('''
-void visit_type_%(c_name)s(Visitor *m, %(c_type)sobj, const char *name, Error **errp);
+void visit_type_%(c_name)s(Visitor *v, %(c_type)sobj, const char *name, Error **errp);
''',
c_name=c_name(name), c_type=c_type)
@@ -37,23 +37,23 @@ def gen_visit_implicit_struct(typ):
# Need a forward declaration
ret += mcgen('''
-static void visit_type_%(c_type)s_fields(Visitor *m, %(c_type)s **obj, Error **errp);
+static void visit_type_%(c_type)s_fields(Visitor *v, %(c_type)s **obj, Error **errp);
''',
c_type=typ.c_name())
ret += mcgen('''
-static void visit_type_implicit_%(c_type)s(Visitor *m, %(c_type)s **obj, Error **errp)
+static void visit_type_implicit_%(c_type)s(Visitor *v, %(c_type)s **obj, Error **errp)
{
Error *err = NULL;
bool allocated;
- allocated = visit_start_implicit_struct(m, (void **)obj, sizeof(%(c_type)s), &err);
+ allocated = visit_start_implicit_struct(v, (void **)obj, sizeof(%(c_type)s), &err);
if (!err) {
if (!obj || *obj) {
- visit_type_%(c_type)s_fields(m, obj, &err);
+ visit_type_%(c_type)s_fields(v, obj, &err);
}
- visit_end_implicit_struct(m, err ? NULL : &err);
+ visit_end_implicit_struct(v, err ? NULL : &err);
}
if (allocated && err) {
g_free(*obj);
@@ -78,7 +78,7 @@ def gen_visit_struct_fields(name, base, members):
ret += mcgen('''
-static void visit_type_%(c_name)s_fields(Visitor *m, %(c_name)s **obj, Error **errp)
+static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s **obj, Error **errp)
{
Error *err = NULL;
@@ -88,7 +88,7 @@ static void visit_type_%(c_name)s_fields(Visitor *m, %(c_name)s **obj, Error **e
if base:
ret += mcgen('''
-visit_type_%(c_type)s_fields(m, (%(c_type)s **)obj, &err);
+visit_type_%(c_type)s_fields(v, (%(c_type)s **)obj, &err);
if (err) {
goto out;
}
@@ -98,14 +98,14 @@ if (err) {
for memb in members:
if memb.optional:
ret += mcgen('''
-visit_optional(m, &(*obj)->has_%(c_name)s, "%(name)s", &err);
+visit_optional(v, &(*obj)->has_%(c_name)s, "%(name)s", &err);
if (!err && (*obj)->has_%(c_name)s) {
''',
c_name=c_name(memb.name), name=memb.name)
push_indent()
ret += mcgen('''
-visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "%(name)s", &err);
+visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err);
''',
c_type=memb.type.c_name(), c_name=c_name(memb.name),
name=memb.name)
@@ -138,17 +138,17 @@ def gen_visit_struct(name, base, members):
ret += mcgen('''
-void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error **errp)
+void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error **errp)
{
Error *err = NULL;
bool allocated;
- allocated = visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), &err);
+ allocated = visit_start_struct(v, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), &err);
if (!err) {
if (*obj) {
- visit_type_%(c_name)s_fields(m, obj, &err);
+ visit_type_%(c_name)s_fields(v, obj, &err);
}
- visit_end_struct(m, err ? NULL : &err);
+ visit_end_struct(v, err ? NULL : &err);
}
if (allocated && err) {
qapi_free_%(c_name)s(*obj);
@@ -164,24 +164,24 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error
def gen_visit_list(name, element_type):
return mcgen('''
-void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error **errp)
+void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error **errp)
{
Error *err = NULL;
GenericList *i, **prev;
- visit_start_list(m, name, &err);
+ visit_start_list(v, name, &err);
if (err) {
goto out;
}
for (prev = (GenericList **)obj;
- !err && (i = visit_next_list(m, prev, &err)) != NULL;
+ !err && (i = visit_next_list(v, prev, &err)) != NULL;
prev = &i) {
%(c_name)s *native_i = (%(c_name)s *)i;
- visit_type_%(c_elt_type)s(m, &native_i->value, NULL, &err);
+ visit_type_%(c_elt_type)s(v, &native_i->value, NULL, &err);
}
- visit_end_list(m, err ? NULL : &err);
+ visit_end_list(v, err ? NULL : &err);
out:
error_propagate(errp, err);
}
@@ -191,9 +191,9 @@ out:
def gen_visit_enum(name):
return mcgen('''
-void visit_type_%(c_name)s(Visitor *m, %(c_name)s *obj, const char *name, Error **errp)
+void visit_type_%(c_name)s(Visitor *v, %(c_name)s *obj, const char *name, Error **errp)
{
- visit_type_enum(m, (int *)obj, %(c_name)s_lookup, "%(name)s", name, errp);
+ visit_type_enum(v, (int *)obj, %(c_name)s_lookup, "%(name)s", name, errp);
}
''',
c_name=c_name(name), name=name)
@@ -206,16 +206,16 @@ def gen_visit_alternate(name, variants):
ret = mcgen('''
-void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error **errp)
+void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error **errp)
{
Error *err = NULL;
bool allocated;
- allocated = visit_start_implicit_struct(m, (void**) obj, sizeof(%(c_name)s), &err);
+ allocated = visit_start_implicit_struct(v, (void**) obj, sizeof(%(c_name)s), &err);
if (err) {
goto out;
}
- visit_get_next_type(m, &(*obj)->type, %(promote_int)s, name, &err);
+ visit_get_next_type(v, &(*obj)->type, %(promote_int)s, name, &err);
if (err) {
goto out_obj;
}
@@ -226,7 +226,7 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error
for var in variants.variants:
ret += mcgen('''
case %(case)s:
- visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, name, &err);
+ visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, name, &err);
break;
''',
case=var.type.alternate_qtype(),
@@ -239,7 +239,7 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error
"%(name)s");
}
out_obj:
- visit_end_implicit_struct(m, err ? NULL : &err);
+ visit_end_implicit_struct(v, err ? NULL : &err);
if (allocated && err) {
qapi_free_%(c_name)s(*obj);
*obj = NULL;
@@ -266,12 +266,12 @@ def gen_visit_union(name, base, variants):
ret += mcgen('''
-void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error **errp)
+void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error **errp)
{
Error *err = NULL;
bool allocated;
- allocated = visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), &err);
+ allocated = visit_start_struct(v, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), &err);
if (err) {
goto out;
}
@@ -284,7 +284,7 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error
tag_key = variants.tag_member.name
if base:
ret += mcgen('''
- visit_type_%(c_name)s_fields(m, (%(c_name)s **)obj, &err);
+ visit_type_%(c_name)s_fields(v, (%(c_name)s **)obj, &err);
if (err) {
goto out_obj;
}
@@ -292,7 +292,7 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error
c_name=c_name(base.name))
else:
ret += mcgen('''
- visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "%(name)s", &err);
+ visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err);
if (err) {
goto out_obj;
}
@@ -313,13 +313,13 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error
case=c_enum_const(variants.tag_member.type.name, var.name))
if simple_union_type:
ret += mcgen('''
- visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "data", &err);
+ visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "data", &err);
''',
c_type=simple_union_type.c_name(),
c_name=c_name(var.name))
else:
ret += mcgen('''
- visit_type_implicit_%(c_type)s(m, &(*obj)->%(c_name)s, &err);
+ visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err);
''',
c_type=var.type.c_name(),
c_name=c_name(var.name))
@@ -332,7 +332,7 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error
abort();
}
out_obj:
- visit_end_struct(m, err ? NULL : &err);
+ visit_end_struct(v, err ? NULL : &err);
if (allocated && err) {
qapi_free_%(c_name)s(*obj);
*obj = NULL;
--
2.4.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* [Qemu-devel] [PATCH RFC 3/5] qapi: Merge generation of per-member visits
2015-08-20 0:04 [Qemu-devel] [PATCH RFC 0/5] qapi: support anonymous inline base Eric Blake
2015-08-20 0:04 ` [Qemu-devel] [PATCH RFC 1/5] qapi: Reuse code for flat union base validation Eric Blake
2015-08-20 0:04 ` [Qemu-devel] [PATCH RFC 2/5] qapi: Generate consistent variable names Eric Blake
@ 2015-08-20 0:04 ` Eric Blake
2015-08-20 0:04 ` [Qemu-devel] [PATCH RFC 4/5] qapi: Simplify visits of optional fields Eric Blake
2015-08-20 0:04 ` [Qemu-devel] [PATCH RFC 5/5] qapi: Allow anonymous base for flat union Eric Blake
4 siblings, 0 replies; 7+ messages in thread
From: Eric Blake @ 2015-08-20 0:04 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
Consolidate the code between visit and command marshalling that
iterates over the members of a struct. It reduces code duplication
in the generator, with no change to generated marshal code, and
slightly more verbose (but no semantic change) in the generated
visit code:
| visit_optional(v, &(*obj)->has_device, "device", &err);
|- if (!err && (*obj)->has_device) {
|- visit_type_str(v, &(*obj)->device, "device", &err);
|- }
| if (err) {
| goto out;
| }
|+ if ((*obj)->has_device) {
|+ visit_type_str(v, &(*obj)->device, "device", &err);
|+ if (err) {
|+ goto out;
|+ }
|+ }
Signed-off-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi-commands.py | 35 +----------------------------------
scripts/qapi-visit.py | 26 +-------------------------
scripts/qapi.py | 45 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 47 insertions(+), 59 deletions(-)
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index a30f79a..3eb3704 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -23,16 +23,6 @@ def gen_command_decl(name, arg_type, box, ret_type):
c_name=c_name(name),
params=gen_params(arg_type, box, 'Error **errp'))
-def gen_err_check(err):
- if not err:
- return ''
- return mcgen('''
-if (%(err)s) {
- goto out;
-}
-''',
- err=err)
-
def gen_call(name, arg_type, box, ret_type):
ret = ''
@@ -145,30 +135,7 @@ visit_type_%(c_name)s(v, &arg, NULL, %(errp)s);
c_name=arg_type.c_name(), errp=errparg)
ret += gen_err_check(errarg)
else:
- for memb in arg_type.members:
- if memb.optional:
- ret += mcgen('''
-visit_optional(v, &has_%(c_name)s, "%(name)s", %(errp)s);
-''',
- c_name=c_name(memb.name), name=memb.name,
- errp=errparg)
- ret += gen_err_check(errarg)
- ret += mcgen('''
-if (has_%(c_name)s) {
-''',
- c_name=c_name(memb.name))
- push_indent()
- ret += mcgen('''
-visit_type_%(c_type)s(v, &%(c_name)s, "%(name)s", %(errp)s);
-''',
- c_name=c_name(memb.name), name=memb.name,
- c_type=memb.type.c_name(), errp=errparg)
- ret += gen_err_check(errarg)
- if memb.optional:
- pop_indent()
- ret += mcgen('''
-}
-''')
+ ret += gen_visit_fields(arg_type.members, '', errarg)
if dealloc:
ret += mcgen('''
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 1fe6c64..edf97cb 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -95,31 +95,7 @@ if (err) {
''',
c_type=base.c_name())
- for memb in members:
- if memb.optional:
- ret += mcgen('''
-visit_optional(v, &(*obj)->has_%(c_name)s, "%(name)s", &err);
-if (!err && (*obj)->has_%(c_name)s) {
-''',
- c_name=c_name(memb.name), name=memb.name)
- push_indent()
-
- ret += mcgen('''
-visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err);
-''',
- c_type=memb.type.c_name(), c_name=c_name(memb.name),
- name=memb.name)
-
- if memb.optional:
- pop_indent()
- ret += mcgen('''
-}
-''')
- ret += mcgen('''
-if (err) {
- goto out;
-}
-''')
+ ret += gen_visit_fields(members, '(*obj)->', 'err')
pop_indent()
if base or members:
diff --git a/scripts/qapi.py b/scripts/qapi.py
index ac6d7f6..063d299 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1456,6 +1456,51 @@ def gen_params(arg_type, box, extra):
ret += sep + extra
return ret
+def gen_err_check(err):
+ if not err:
+ return ''
+ return mcgen('''
+if (%(err)s) {
+ goto out;
+}
+''',
+ err=err)
+
+def gen_visit_fields(members, prefix, errarg):
+ ret = ''
+ if errarg:
+ errparg = '&' + errarg
+ else:
+ errparg = 'NULL';
+ for memb in members:
+ if memb.optional:
+ ret += mcgen('''
+visit_optional(v, &%(prefix)shas_%(c_name)s, "%(name)s", %(errp)s);
+''',
+ prefix=prefix, c_name=c_name(memb.name),
+ name=memb.name, errp=errparg)
+ ret += gen_err_check(errarg)
+ ret += mcgen('''
+if (%(prefix)shas_%(c_name)s) {
+''',
+ prefix=prefix, c_name=c_name(memb.name))
+ push_indent()
+
+ ret += mcgen('''
+visit_type_%(c_type)s(v, &%(prefix)s%(c_name)s, "%(name)s", %(errp)s);
+''',
+ c_type=memb.type.c_name(), prefix=prefix,
+ c_name=c_name(memb.name), name=memb.name,
+ errp=errparg)
+ ret += gen_err_check(errarg)
+
+ if memb.optional:
+ pop_indent()
+ ret += mcgen('''
+}
+''')
+ return ret
+
#
# Common command line parsing
#
--
2.4.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* [Qemu-devel] [PATCH RFC 4/5] qapi: Simplify visits of optional fields
2015-08-20 0:04 [Qemu-devel] [PATCH RFC 0/5] qapi: support anonymous inline base Eric Blake
` (2 preceding siblings ...)
2015-08-20 0:04 ` [Qemu-devel] [PATCH RFC 3/5] qapi: Merge generation of per-member visits Eric Blake
@ 2015-08-20 0:04 ` Eric Blake
2015-08-20 0:04 ` [Qemu-devel] [PATCH RFC 5/5] qapi: Allow anonymous base for flat union Eric Blake
4 siblings, 0 replies; 7+ messages in thread
From: Eric Blake @ 2015-08-20 0:04 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
None of the visitor callbacks would set an error when testing
if an optional field was present; make this part of the interface
contract by eliminating the errp argument. Then, for less code,
reflect the determined boolean value back to the caller instead
of making the caller read the boolean after the fact.
The resulting generated code has a nice diff:
|- visit_optional(v, &has_fdset_id, "fdset-id", &err);
|- if (err) {
|- goto out;
|- }
|- if (has_fdset_id) {
|+ if (visit_optional(v, &has_fdset_id, "fdset-id")) {
| visit_type_int(v, &fdset_id, "fdset-id", &err);
| if (err) {
| goto out;
| }
| }
Signed-off-by: Eric Blake <eblake@redhat.com>
---
include/qapi/visitor-impl.h | 3 +--
include/qapi/visitor.h | 5 ++---
qapi/opts-visitor.c | 2 +-
qapi/qapi-visit-core.c | 6 +++---
qapi/qmp-input-visitor.c | 3 +--
qapi/string-input-visitor.c | 3 +--
scripts/qapi.py | 9 ++-------
7 files changed, 11 insertions(+), 20 deletions(-)
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 7e8f728..6feb654 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -69,8 +69,7 @@ struct Visitor
Error **errp);
/* May be NULL; most useful for input visitors. */
- void (*optional)(Visitor *v, bool *present, const char *name,
- Error **errp);
+ void (*optional)(Visitor *v, bool *present, const char *name);
/* Only required to visit uint64 differently than (*type_int)(). */
void (*type_uint64)(Visitor *v, uint64_t *obj, const char *name,
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index cd1431a..ef1105b 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -118,10 +118,9 @@ void visit_end_list(Visitor *v, Error **errp);
* Check if an optional member @name of a QDict needs visiting.
* For input visitors, set *@present according to whether the
* corresponding visit_type_*() needs calling; for other visitors,
- * leave *@present unchanged.
+ * leave *@present unchanged. Return *@present for convenience.
*/
-void visit_optional(Visitor *v, bool *present, const char *name,
- Error **errp);
+bool visit_optional(Visitor *v, bool *present, const char *name);
/**
* Determine the qtype of the item @name in the current QDict visit.
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 7ae33b3..b2cd5b3 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -488,7 +488,7 @@ opts_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp)
static void
-opts_optional(Visitor *v, bool *present, const char *name, Error **errp)
+opts_optional(Visitor *v, bool *present, const char *name)
{
OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 9cd17f8..d885347 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -62,12 +62,12 @@ void visit_end_list(Visitor *v, Error **errp)
v->end_list(v, errp);
}
-void visit_optional(Visitor *v, bool *present, const char *name,
- Error **errp)
+bool visit_optional(Visitor *v, bool *present, const char *name)
{
if (v->optional) {
- v->optional(v, present, name, errp);
+ v->optional(v, present, name);
}
+ return *present;
}
void visit_get_next_type(Visitor *v, qtype_code *type, bool promote_int,
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 5310db5..f714dfc 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -300,8 +300,7 @@ static void qmp_input_type_any(Visitor *v, QObject **obj, const char *name,
*obj = qobj;
}
-static void qmp_input_optional(Visitor *v, bool *present, const char *name,
- Error **errp)
+static void qmp_input_optional(Visitor *v, bool *present, const char *name)
{
QmpInputVisitor *qiv = to_qiv(v);
QObject *qobj = qmp_input_get_object(qiv, name, true);
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index bbd6a54..dee780a 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -299,8 +299,7 @@ static void parse_type_number(Visitor *v, double *obj, const char *name,
*obj = val;
}
-static void parse_optional(Visitor *v, bool *present, const char *name,
- Error **errp)
+static void parse_optional(Visitor *v, bool *present, const char *name)
{
StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 063d299..9af310f 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1475,15 +1475,10 @@ def gen_visit_fields(members, prefix, errarg):
for memb in members:
if memb.optional:
ret += mcgen('''
-visit_optional(v, &%(prefix)shas_%(c_name)s, "%(name)s", %(errp)s);
+if (visit_optional(v, &%(prefix)shas_%(c_name)s, "%(name)s")) {
''',
prefix=prefix, c_name=c_name(memb.name),
- name=memb.name, errp=errparg)
- ret += gen_err_check(errarg)
- ret += mcgen('''
-if (%(prefix)shas_%(c_name)s) {
-''',
- prefix=prefix, c_name=c_name(memb.name))
+ name=memb.name)
push_indent()
ret += mcgen('''
--
2.4.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* [Qemu-devel] [PATCH RFC 5/5] qapi: Allow anonymous base for flat union
2015-08-20 0:04 [Qemu-devel] [PATCH RFC 0/5] qapi: support anonymous inline base Eric Blake
` (3 preceding siblings ...)
2015-08-20 0:04 ` [Qemu-devel] [PATCH RFC 4/5] qapi: Simplify visits of optional fields Eric Blake
@ 2015-08-20 0:04 ` Eric Blake
2015-08-22 3:48 ` Eric Blake
4 siblings, 1 reply; 7+ messages in thread
From: Eric Blake @ 2015-08-20 0:04 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
Rather than requiring all flat unions to explicitly create
a separate base struct, we want to allow the qapi schema
to specify the common fields via an inline dictionary. This
is similar to how commands can specify inline types for the
arguments.
Now that the feature is legal, we can drop the former
flat-union-bad-base negative test, and instead change the
positive tests in qapi-schema-test to use it.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi-commands.py | 2 +-
scripts/qapi-types.py | 2 +-
scripts/qapi-visit.py | 13 +++++++---
scripts/qapi.py | 39 ++++++++++++++++++------------
tests/Makefile | 3 +--
tests/qapi-schema/flat-union-bad-base.err | 1 -
tests/qapi-schema/flat-union-bad-base.exit | 1 -
tests/qapi-schema/flat-union-bad-base.json | 13 ----------
tests/qapi-schema/flat-union-bad-base.out | 0
tests/qapi-schema/qapi-schema-test.json | 2 +-
tests/qapi-schema/qapi-schema-test.out | 5 +++-
11 files changed, 41 insertions(+), 40 deletions(-)
delete mode 100644 tests/qapi-schema/flat-union-bad-base.err
delete mode 100644 tests/qapi-schema/flat-union-bad-base.exit
delete mode 100644 tests/qapi-schema/flat-union-bad-base.json
delete mode 100644 tests/qapi-schema/flat-union-bad-base.out
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 3eb3704..18fbedf 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -135,7 +135,7 @@ visit_type_%(c_name)s(v, &arg, NULL, %(errp)s);
c_name=arg_type.c_name(), errp=errparg)
ret += gen_err_check(errarg)
else:
- ret += gen_visit_fields(arg_type.members, '', errarg)
+ ret += gen_visit_fields(arg_type.members, '', errarg, 'out')
if dealloc:
ret += mcgen('''
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 8f92b38..8d22b5a 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -95,7 +95,7 @@ struct %(c_name)s {
''',
c_name=c_name(name))
if base:
- ret += gen_struct_fields([], base)
+ ret += gen_struct_fields([], base, not base.info)
elif not variants.tag_member:
ret += mcgen('''
qtype_code type;
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index edf97cb..6e492f2 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -95,7 +95,7 @@ if (err) {
''',
c_type=base.c_name())
- ret += gen_visit_fields(members, '(*obj)->', 'err')
+ ret += gen_visit_fields(members, '(*obj)->', 'err', 'out')
pop_indent()
if base or members:
@@ -231,7 +231,7 @@ out:
def gen_visit_union(name, base, variants):
ret = ''
- if base:
+ if base and base.info:
ret += gen_visit_struct_fields(base.name, base.base,
base.local_members)
@@ -259,13 +259,18 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
tag_key = variants.tag_member.name
if base:
- ret += mcgen('''
+ if base.info:
+ ret += mcgen('''
visit_type_%(c_name)s_fields(v, (%(c_name)s **)obj, &err);
if (err) {
goto out_obj;
}
''',
- c_name=c_name(base.name))
+ c_name=c_name(base.name))
+ else:
+ push_indent()
+ ret += gen_visit_fields(base.members, '(*obj)->', 'err', 'out_obj')
+ pop_indent()
else:
ret += mcgen('''
visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err);
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 9af310f..7eef19d 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -308,6 +308,8 @@ class QAPISchemaParser(object):
#
def find_base_fields(base):
+ if isinstance(base, OrderedDict):
+ return base
base_struct_define = find_struct(base)
if not base_struct_define:
return None
@@ -475,19 +477,23 @@ def check_type(expr_info, source, value, allow_array = False,
allow_metas=['built-in', 'union', 'alternate', 'struct',
'enum'])
-def check_member_clash(expr_info, base_name, data, source = ""):
- base = find_struct(base_name)
- assert base
- base_members = base['data']
+def check_member_clash(expr_info, base, data, source = ""):
+ base_obj = None
+ base_name = ''
+ if isinstance(base, str):
+ base_name = " '%s'" % base
+ base_obj = find_struct(base)
+ assert base_obj
+ base = base_obj['data']
for key in data.keys():
if key.startswith('*'):
key = key[1:]
- if key in base_members or "*" + key in base_members:
+ if key in base or "*" + key in base:
raise QAPIExprError(expr_info,
- "Member name '%s'%s clashes with base '%s'"
+ "Member name '%s'%s clashes with base%s"
% (key, source, base_name))
- if base.get('base'):
- check_member_clash(expr_info, base['base'], data, source)
+ if base_obj and base_obj.get('base'):
+ check_member_clash(expr_info, base_obj['base'], data, source)
def check_command(expr, expr_info):
name = expr['command']
@@ -553,9 +559,9 @@ def check_union(expr, expr_info):
# Else, it's a flat union.
else:
- # The object must have a string member 'base'.
+ # The object must have a string or dictionary 'base'.
check_type(expr_info, "'base' for union '%s'" % name,
- base, allow_metas=['struct'])
+ base, allow_dict=True, allow_metas=['struct'])
if not base:
raise QAPIExprError(expr_info,
"Flat union '%s' must have a valid base"
@@ -1161,6 +1167,9 @@ class QAPISchema(object):
base = expr.get('base')
tag_name = expr.get('discriminator')
tag_enum = None
+ if isinstance(base, OrderedDict):
+ base = self._make_implicit_object_type(name, 'base',
+ self._make_members(base))
if tag_name:
variants = [self._make_variant(key, data[key])
for key in data.keys()]
@@ -1456,17 +1465,17 @@ def gen_params(arg_type, box, extra):
ret += sep + extra
return ret
-def gen_err_check(err):
+def gen_err_check(err, label='out'):
if not err:
return ''
return mcgen('''
if (%(err)s) {
- goto out;
+ goto %(label)s;
}
''',
- err=err)
+ err=err, label=label)
-def gen_visit_fields(members, prefix, errarg):
+def gen_visit_fields(members, prefix, errarg, label):
ret = ''
if errarg:
errparg = '&' + errarg
@@ -1487,7 +1496,7 @@ visit_type_%(c_type)s(v, &%(prefix)s%(c_name)s, "%(name)s", %(errp)s);
c_type=memb.type.c_name(), prefix=prefix,
c_name=c_name(memb.name), name=memb.name,
errp=errparg)
- ret += gen_err_check(errarg)
+ ret += gen_err_check(errarg, label)
if memb.optional:
pop_indent()
diff --git a/tests/Makefile b/tests/Makefile
index 31b8761..606b4ed 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -247,8 +247,7 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
flat-union-invalid-discriminator.json flat-union-inline.json \
flat-union-invalid-branch-key.json flat-union-reverse-define.json \
flat-union-string-discriminator.json union-base-no-discriminator.json \
- flat-union-bad-discriminator.json flat-union-bad-base.json \
- flat-union-base-any.json \
+ flat-union-bad-discriminator.json flat-union-base-any.json \
flat-union-array-branch.json flat-union-int-branch.json \
flat-union-base-union.json flat-union-branch-clash.json \
alternate-nested.json alternate-unknown.json alternate-clash.json \
diff --git a/tests/qapi-schema/flat-union-bad-base.err b/tests/qapi-schema/flat-union-bad-base.err
deleted file mode 100644
index 79b8a71..0000000
--- a/tests/qapi-schema/flat-union-bad-base.err
+++ /dev/null
@@ -1 +0,0 @@
-tests/qapi-schema/flat-union-bad-base.json:9: 'base' for union 'TestUnion' should be a type name
diff --git a/tests/qapi-schema/flat-union-bad-base.exit b/tests/qapi-schema/flat-union-bad-base.exit
deleted file mode 100644
index d00491f..0000000
--- a/tests/qapi-schema/flat-union-bad-base.exit
+++ /dev/null
@@ -1 +0,0 @@
-1
diff --git a/tests/qapi-schema/flat-union-bad-base.json b/tests/qapi-schema/flat-union-bad-base.json
deleted file mode 100644
index e2e622b..0000000
--- a/tests/qapi-schema/flat-union-bad-base.json
+++ /dev/null
@@ -1,13 +0,0 @@
-# we require the base to be an existing struct
-# TODO: should we allow an anonymous inline base type?
-{ 'enum': 'TestEnum',
- 'data': [ 'value1', 'value2' ] }
-{ 'struct': 'TestTypeA',
- 'data': { 'string': 'str' } }
-{ 'struct': 'TestTypeB',
- 'data': { 'integer': 'int' } }
-{ 'union': 'TestUnion',
- 'base': { 'enum1': 'TestEnum', 'kind': 'str' },
- 'discriminator': 'enum1',
- 'data': { 'value1': 'TestTypeA',
- 'value2': 'TestTypeB' } }
diff --git a/tests/qapi-schema/flat-union-bad-base.out b/tests/qapi-schema/flat-union-bad-base.out
deleted file mode 100644
index e69de29..0000000
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index b4893ce..3c87f87 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -47,7 +47,7 @@
# this variant of UserDefFlatUnion defaults to a union that uses fields with
# allocated types to test corner cases in the cleanup/dealloc visitor
{ 'union': 'UserDefFlatUnion2',
- 'base': 'UserDefUnionBase',
+ 'base': { 'string': 'str', 'enum1': 'EnumOne' },
'discriminator': 'enum1',
'data': { 'value1' : 'UserDefC', # intentional forward reference
'value2' : 'UserDefB',
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 89f50b8..5be0ff8 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -8,6 +8,9 @@ object :obj-EVENT_D-arg
member b: str optional=False
member c: str optional=True
member enum3: EnumOne optional=True
+object :obj-UserDefFlatUnion2-base
+ member string: str optional=False
+ member enum1: EnumOne optional=False
object :obj-__org.qemu_x-command-arg
member a: __org.qemu_x-EnumList optional=False
member b: __org.qemu_x-StructList optional=False
@@ -107,7 +110,7 @@ object UserDefFlatUnion
case value2: UserDefB
case value3: UserDefB
object UserDefFlatUnion2
- base UserDefUnionBase
+ base :obj-UserDefFlatUnion2-base
tag enum1
case value1: UserDefC
case value2: UserDefB
--
2.4.3
^ permalink raw reply related [flat|nested] 7+ messages in thread