From 2a0f50e8d973b01eda4c63bac4a5c79ea0f584ef Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 29 Sep 2015 16:21:08 -0600 Subject: qapi: Consistent generated code: prefer error 'err' We had some pointless differences in the generated code for visit, command marshalling, and events; unifying them makes it easier for future patches to consolidate to common helper functions. This is one patch of a series to clean up these differences. This patch consistently names the local error variable 'err' rather than 'local_err'. No change in semantics to the generated code. Signed-off-by: Eric Blake Message-Id: <1443565276-4535-11-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- scripts/qapi-event.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'scripts/qapi-event.py') diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py index d15fad98f3..d41af40799 100644 --- a/scripts/qapi-event.py +++ b/scripts/qapi-event.py @@ -34,7 +34,7 @@ def gen_event_send(name, arg_type): %(proto)s { QDict *qmp; - Error *local_err = NULL; + Error *err = NULL; QMPEventFuncEmit emit; ''', proto=gen_event_send_proto(name, arg_type)) @@ -67,8 +67,8 @@ def gen_event_send(name, arg_type): g_assert(v); /* 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; } @@ -90,8 +90,8 @@ def gen_event_send(name, arg_type): 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; } ''', @@ -108,8 +108,8 @@ def gen_event_send(name, arg_type): ret += mcgen(''' - visit_end_struct(v, &local_err); - if (local_err) { + visit_end_struct(v, &err); + if (err) { goto clean; } @@ -120,7 +120,7 @@ def gen_event_send(name, arg_type): ''') ret += mcgen(''' - emit(%(c_enum)s, qmp, &local_err); + emit(%(c_enum)s, qmp, &err); ''', c_enum=c_enum_const(event_enum_name, name)) @@ -131,7 +131,7 @@ def gen_event_send(name, arg_type): qmp_output_visitor_cleanup(qov); ''') ret += mcgen(''' - error_propagate(errp, local_err); + error_propagate(errp, err); QDECREF(qmp); } ''') -- cgit 1.4.1 From f782399cb4fa3fc4182cb046817f65a6db92ab07 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 29 Sep 2015 16:21:10 -0600 Subject: qapi: Consistent generated code: prefer common labels We had some pointless differences in the generated code for visit, command marshalling, and events; unifying them makes it easier for future patches to consolidate to common helper functions. This is one patch of a series to clean up these differences. This patch names the goto labels 'out' (not 'clean') and 'out_obj' (not 'out_end'). Additionally, the generator was inconsistent on whether labels had a leading space [our HACKING is silent; while emacs 'gnu' style adds the space to avoid littering column 1]. For minimal churn, prefer no leading space; this also matches the style that is more prevalent in current qemu.git. No change in semantics to the generated code. Signed-off-by: Eric Blake Message-Id: <1443565276-4535-13-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- scripts/qapi-event.py | 8 ++++---- scripts/qapi-visit.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) (limited to 'scripts/qapi-event.py') diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py index d41af40799..b5a9d4f364 100644 --- a/scripts/qapi-event.py +++ b/scripts/qapi-event.py @@ -69,7 +69,7 @@ def gen_event_send(name, arg_type): /* Fake visit, as if all members are under a structure */ visit_start_struct(v, NULL, "", "%(name)s", 0, &err); if (err) { - goto clean; + goto out; } ''', @@ -92,7 +92,7 @@ def gen_event_send(name, arg_type): ret += mcgen(''' visit_type_%(c_type)s(v, %(cast)s&%(c_name)s, "%(name)s", &err); if (err) { - goto clean; + goto out; } ''', cast=cast, @@ -110,7 +110,7 @@ def gen_event_send(name, arg_type): visit_end_struct(v, &err); if (err) { - goto clean; + goto out; } obj = qmp_output_get_qobject(qov); @@ -127,7 +127,7 @@ def gen_event_send(name, arg_type): if arg_type and arg_type.members: ret += mcgen(''' - clean: +out: qmp_output_visitor_cleanup(qov); ''') ret += mcgen(''' diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 348efe0757..5a453ea021 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -209,7 +209,7 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error } visit_get_next_type(v, (int*) &(*obj)->kind, %(c_name)s_qtypes, name, &err); if (err) { - goto out_end; + goto out_obj; } switch ((*obj)->kind) { ''', @@ -230,7 +230,7 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error default: abort(); } -out_end: +out_obj: error_propagate(errp, err); err = NULL; visit_end_implicit_struct(v, &err); -- cgit 1.4.1 From 1f35334489a43800df4d20cd91362a87cee39a29 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 29 Sep 2015 16:21:13 -0600 Subject: qapi: Share gen_err_check() qapi-commands has a nice helper gen_err_check(), but did not use it everywhere. In fact, using it in more places makes it easier to reduce the lines of code used for generating error checks. This in turn will make it easier for later patches to consolidate another common pattern among the generators. The generated code has fewer blank lines in qapi-event.c functions, but has no semantic difference. Signed-off-by: Eric Blake Message-Id: <1443565276-4535-16-git-send-email-eblake@redhat.com> [Drop another blank line for symmetry] Signed-off-by: Markus Armbruster --- scripts/qapi-commands.py | 17 +++-------------- scripts/qapi-event.py | 10 ++-------- scripts/qapi-visit.py | 14 +++----------- scripts/qapi.py | 12 ++++++++++++ 4 files changed, 20 insertions(+), 33 deletions(-) (limited to 'scripts/qapi-event.py') diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 267991ed8b..4e99c1de8b 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -25,17 +25,6 @@ def gen_command_decl(name, arg_type, ret_type): params=gen_params(arg_type, '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, ret_type): ret = '' @@ -56,7 +45,7 @@ def gen_call(name, arg_type, ret_type): ''', c_name=c_name(name), args=argstr, lhs=lhs) if ret_type: - ret += gen_err_check('err') + ret += gen_err_check() ret += mcgen(''' qmp_marshal_output_%(c_name)s(retval, ret, &err); @@ -133,7 +122,7 @@ def gen_marshal_input_visit(arg_type, dealloc=False): ''', c_name=c_name(memb.name), name=memb.name, errp=errparg) - ret += gen_err_check(errarg) + ret += gen_err_check(err=errarg) ret += mcgen(''' if (has_%(c_name)s) { ''', @@ -144,7 +133,7 @@ def gen_marshal_input_visit(arg_type, dealloc=False): ''', c_name=c_name(memb.name), name=memb.name, c_type=memb.type.c_name(), errp=errparg) - ret += gen_err_check(errarg) + ret += gen_err_check(err=errarg) if memb.optional: pop_indent() ret += mcgen(''' diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py index b5a9d4f364..eaaac05154 100644 --- a/scripts/qapi-event.py +++ b/scripts/qapi-event.py @@ -68,12 +68,9 @@ def gen_event_send(name, arg_type): /* Fake visit, as if all members are under a structure */ visit_start_struct(v, NULL, "", "%(name)s", 0, &err); - if (err) { - goto out; - } - ''', name=name) + ret += gen_err_check() for memb in arg_type.members: if memb.optional: @@ -91,14 +88,12 @@ def gen_event_send(name, arg_type): ret += mcgen(''' visit_type_%(c_type)s(v, %(cast)s&%(c_name)s, "%(name)s", &err); - if (err) { - goto out; - } ''', cast=cast, c_name=c_name(memb.name), c_type=memb.type.c_name(), name=memb.name) + ret += gen_err_check() if memb.optional: pop_indent() @@ -107,7 +102,6 @@ def gen_event_send(name, arg_type): ''') ret += mcgen(''' - visit_end_struct(v, &err); if (err) { goto out; diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 78ad5560d0..bc6911f8fe 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -81,11 +81,9 @@ static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s **obj, Error **e if base: ret += mcgen(''' visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err); - if (err) { - goto out; - } ''', c_type=base.c_name(), c_name=c_name('base')) + ret += gen_err_check() for memb in members: if memb.optional: @@ -107,11 +105,7 @@ static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s **obj, Error **e ret += mcgen(''' } ''') - ret += mcgen(''' - if (err) { - goto out; - } -''') + ret += gen_err_check() if re.search('^ *goto out;', ret, re.MULTILINE): ret += mcgen(''' @@ -271,11 +265,9 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error if base: ret += mcgen(''' visit_type_%(c_name)s_fields(v, obj, &err); - if (err) { - goto out_obj; - } ''', c_name=c_name(name)) + ret += gen_err_check(label='out_obj') tag_key = variants.tag_member.name if not variants.tag_name: diff --git a/scripts/qapi.py b/scripts/qapi.py index c0728d73e1..62a415ccd9 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -1536,6 +1536,18 @@ def gen_params(arg_type, extra): ret += sep + extra return ret + +def gen_err_check(err='err', label='out'): + if not err: + return '' + return mcgen(''' + if (%(err)s) { + goto %(label)s; + } +''', + err=err, label=label) + + # # Common command line parsing # -- cgit 1.4.1 From 82ca8e469666b169ccf818a0e36136aee97d7db0 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 29 Sep 2015 16:21:14 -0600 Subject: qapi: Share gen_visit_fields() Consolidate the code between visit, command marshalling, and event generation that iterates over the members of a struct. It reduces code duplication in the generator, so that a future patch can reduce the size of generated code while touching only one instead of three locations. There are no changes to the generated marshal code. The visitor code becomes slightly more verbose, but remains semantically equivalent, and is actually easier to read as it follows a more common idiom: | 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; |+ } |+ } The event code becomes slightly more verbose, but this is arguably a bug fix: although the visitors are not well documented, use of an optional member should not be attempted unless guarded by a prior call to visit_optional(). Works only because the output qmp visitor has a no-op visit_optional(): |+ visit_optional(v, &has_offset, "offset", &err); |+ if (err) { |+ goto out; |+ } | if (has_offset) { | visit_type_int(v, &offset, "offset", &err); Signed-off-by: Eric Blake Message-Id: <1443565276-4535-17-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- scripts/qapi-commands.py | 27 +-------------------------- scripts/qapi-event.py | 31 +------------------------------ scripts/qapi-visit.py | 22 +--------------------- scripts/qapi.py | 43 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 46 insertions(+), 77 deletions(-) (limited to 'scripts/qapi-event.py') diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 4e99c1de8b..9d214a6609 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -101,7 +101,6 @@ def gen_marshal_input_visit(arg_type, dealloc=False): return ret if dealloc: - errparg = 'NULL' errarg = None ret += mcgen(''' qmp_input_visitor_cleanup(qiv); @@ -109,36 +108,12 @@ def gen_marshal_input_visit(arg_type, dealloc=False): v = qapi_dealloc_get_visitor(qdv); ''') else: - errparg = '&err' errarg = 'err' ret += mcgen(''' v = qmp_input_get_visitor(qiv); ''') - 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(err=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(err=errarg) - if memb.optional: - pop_indent() - ret += mcgen(''' - } -''') + ret += gen_visit_fields(arg_type.members, errarg=errarg) if dealloc: ret += mcgen(''' diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py index eaaac05154..720486f06c 100644 --- a/scripts/qapi-event.py +++ b/scripts/qapi-event.py @@ -71,36 +71,7 @@ def gen_event_send(name, arg_type): ''', name=name) ret += gen_err_check() - - for memb in arg_type.members: - if memb.optional: - ret += mcgen(''' - if (has_%(c_name)s) { -''', - c_name=c_name(memb.name)) - push_indent() - - # Ugly: need to cast away the const - if memb.type.name == "str": - cast = '(char **)' - else: - cast = '' - - ret += mcgen(''' - visit_type_%(c_type)s(v, %(cast)s&%(c_name)s, "%(name)s", &err); -''', - cast=cast, - c_name=c_name(memb.name), - c_type=memb.type.c_name(), - name=memb.name) - ret += gen_err_check() - - if memb.optional: - pop_indent() - ret += mcgen(''' - } -''') - + ret += gen_visit_fields(arg_type.members, need_cast=True) ret += mcgen(''' visit_end_struct(v, &err); if (err) { diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index bc6911f8fe..4f97781348 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -85,27 +85,7 @@ static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s **obj, Error **e c_type=base.c_name(), c_name=c_name('base')) ret += gen_err_check() - 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 += gen_err_check() + ret += gen_visit_fields(members, prefix='(*obj)->') if re.search('^ *goto out;', ret, re.MULTILINE): ret += mcgen(''' diff --git a/scripts/qapi.py b/scripts/qapi.py index 62a415ccd9..ada6380db2 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -1548,6 +1548,49 @@ def gen_err_check(err='err', label='out'): err=err, label=label) +def gen_visit_fields(members, prefix='', need_cast=False, errarg='err'): + 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(err=errarg) + ret += mcgen(''' + if (%(prefix)shas_%(c_name)s) { +''', + prefix=prefix, c_name=c_name(memb.name)) + push_indent() + + # Ugly: sometimes we need to cast away const + if need_cast and memb.type.name == 'str': + cast = '(char **)' + else: + cast = '' + + ret += mcgen(''' + visit_type_%(c_type)s(v, %(cast)s&%(prefix)s%(c_name)s, "%(name)s", %(errp)s); +''', + c_type=memb.type.c_name(), prefix=prefix, cast=cast, + c_name=c_name(memb.name), name=memb.name, + errp=errparg) + ret += gen_err_check(err=errarg) + + if memb.optional: + pop_indent() + ret += mcgen(''' + } +''') + return ret + + # # Common command line parsing # -- cgit 1.4.1