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. - Consistently name the error variable 'err' - Consistently use the labels 'out' and 'out_obj' - If allocation fails, jump to the right label rather than indenting everything else
No change in semantics to the generated code. Signed-off-by: Eric Blake <ebl...@redhat.com> --- scripts/qapi-commands.py | 22 +++++----- scripts/qapi-event.py | 26 +++++------ scripts/qapi-visit.py | 111 ++++++++++++++++++++++++----------------------- 3 files changed, 80 insertions(+), 79 deletions(-) diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 810a897..2151120 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -53,14 +53,14 @@ def gen_call(name, arg_type, 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() @@ -69,7 +69,7 @@ qmp_marshal_output_%(c_name)s(retval, ret, &local_err); def gen_marshal_vars(arg_type, ret_type): ret = mcgen(''' - Error *local_err = NULL; + Error *err = NULL; ''') push_indent() @@ -127,8 +127,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); ''') @@ -171,20 +171,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); @@ -227,7 +227,7 @@ def gen_marshal(name, arg_type, ret_type): out: ''') ret += mcgen(''' - error_propagate(errp, local_err); + error_propagate(errp, err); ''') ret += gen_marshal_input_visit(arg_type, dealloc=True) ret += mcgen(''' diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py index d15fad9..b43bbc2 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,9 +67,9 @@ 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) { - goto clean; + visit_start_struct(v, NULL, "", "%(name)s", 0, &err); + if (err) { + goto out; } ''', @@ -90,9 +90,9 @@ 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) { - goto clean; + visit_type_%(c_type)s(v, %(cast)s&%(c_name)s, "%(name)s", &err); + if (err) { + goto out; } ''', cast=cast, @@ -108,9 +108,9 @@ def gen_event_send(name, arg_type): ret += mcgen(''' - visit_end_struct(v, &local_err); - if (local_err) { - goto clean; + visit_end_struct(v, &err); + if (err) { + goto out; } obj = qmp_output_get_qobject(qov); @@ -120,18 +120,18 @@ 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)) if arg_type and arg_type.members: ret += mcgen(''' - clean: + out: 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 d911b20..9c0328d 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -24,7 +24,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) @@ -39,20 +39,20 @@ 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; - visit_start_implicit_struct(m, (void **)obj, sizeof(%(c_type)s), &err); + visit_start_implicit_struct(v, (void **)obj, sizeof(%(c_type)s), &err); if (!err) { - visit_type_%(c_type)s_fields(m, obj, &err); - visit_end_implicit_struct(m, err ? NULL : &err); + visit_type_%(c_type)s_fields(v, obj, &err); + visit_end_implicit_struct(v, err ? NULL : &err); } error_propagate(errp, err); } @@ -71,7 +71,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; @@ -81,7 +81,7 @@ static void visit_type_%(c_name)s_fields(Visitor *m, %(c_name)s **obj, Error **e if base: 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); if (err) { goto out; } @@ -91,14 +91,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) @@ -136,16 +136,16 @@ def gen_visit_struct(name, base, members): # call qapi_free_FOO() to avoid a memory leak of the partial FOO. 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; - visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), &err); + 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); } error_propagate(errp, err); } @@ -158,24 +158,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); } @@ -186,9 +186,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) @@ -197,17 +197,17 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s *obj, const char *name, Error 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; - visit_start_implicit_struct(m, (void**) obj, sizeof(%(c_name)s), &err); + visit_start_implicit_struct(v, (void**) obj, sizeof(%(c_name)s), &err); if (err) { goto out; } - visit_get_next_type(m, (int*) &(*obj)->kind, %(c_name)s_qtypes, name, &err); + 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) { ''', @@ -216,7 +216,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=c_enum_const(variants.tag_member.type.name, @@ -228,8 +228,8 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error default: abort(); } -out_end: - visit_end_implicit_struct(m, err ? NULL : &err); +out_obj: + visit_end_implicit_struct(v, err ? NULL : &err); out: error_propagate(errp, err); } @@ -252,24 +252,26 @@ 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; - visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), &err); + visit_start_struct(v, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), &err); if (err) { goto out; } - if (*obj) { + if (!*obj) { + goto out_obj; + } ''', c_name=c_name(name), name=name) if base: ret += mcgen(''' - visit_type_%(c_name)s_fields(m, obj, &err); - if (err) { - goto out_obj; - } + visit_type_%(c_name)s_fields(v, obj, &err); + if (err) { + goto out_obj; + } ''', c_name=c_name(name)) @@ -278,14 +280,14 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error # we pointlessly use a different key for simple unions tag_key = 'type' ret += mcgen(''' - visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "%(name)s", &err); - if (err) { - goto out_obj; - } - if (!visit_start_union(m, !!(*obj)->data, &err) || err) { - goto out_obj; - } - switch ((*obj)->%(c_name)s) { + visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err); + if (err) { + goto out_obj; + } + if (!visit_start_union(v, !!(*obj)->data, &err) || err) { + goto out_obj; + } + switch ((*obj)->%(c_name)s) { ''', c_type=variants.tag_member.type.c_name(), # TODO ugly special case for simple union @@ -298,36 +300,35 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error # TODO ugly special case for simple union simple_union_type = var.simple_union_type() ret += mcgen(''' - case %(case)s: + case %(case)s: ''', 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)) ret += mcgen(''' - break; + break; ''') ret += mcgen(''' - default: - abort(); - } -out_obj: - error_propagate(errp, err); - err = NULL; - visit_end_union(m, !!(*obj)->data, &err); + default: + abort(); } - visit_end_struct(m, err ? NULL : &err); +out_obj: + error_propagate(errp, err); + err = NULL; + visit_end_union(v, !!(*obj)->data, &err); + visit_end_struct(v, err ? NULL : &err); out: error_propagate(errp, err); } -- 2.4.3