diff options
| -rw-r--r-- | docs/qapi-code-gen.txt | 2 | ||||
| -rw-r--r-- | error.c | 3 | ||||
| -rw-r--r-- | error.h | 2 | ||||
| -rw-r--r-- | qapi/qapi-visit-core.c | 10 | ||||
| -rw-r--r-- | scripts/qapi-visit.py | 150 | ||||
| -rw-r--r-- | tests/test-qmp-input-visitor.c | 24 |
6 files changed, 121 insertions, 70 deletions
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index ad11767a2f..cccb11e562 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -220,6 +220,8 @@ Example: #endif mdroth@illuin:~/w/qemu2.git$ +(The actual structure of the visit_type_* functions is a bit more complex +in order to propagate errors correctly and avoid leaking memory). === scripts/qapi-commands.py === @@ -32,6 +32,7 @@ void error_set(Error **errp, const char *fmt, ...) if (errp == NULL) { return; } + assert(*errp == NULL); err = g_malloc0(sizeof(*err)); @@ -132,7 +133,7 @@ bool error_is_type(Error *err, const char *fmt) void error_propagate(Error **dst_err, Error *local_err) { - if (dst_err) { + if (dst_err && !*dst_err) { *dst_err = local_err; } else if (local_err) { error_free(local_err); @@ -57,7 +57,7 @@ void error_set_field(Error *err, const char *field, const char *value); /** * Propagate an error to an indirect pointer to an error. This function will * always transfer ownership of the error reference and handles the case where - * dst_err is NULL correctly. + * dst_err is NULL correctly. Errors after the first are discarded. */ void error_propagate(Error **dst_err, Error *local_err); diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c index 705eca90aa..d41595eaa1 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qapi-visit-core.c @@ -39,9 +39,8 @@ void visit_start_struct(Visitor *v, void **obj, const char *kind, void visit_end_struct(Visitor *v, Error **errp) { - if (!error_is_set(errp)) { - v->end_struct(v, errp); - } + assert(!error_is_set(errp)); + v->end_struct(v, errp); } void visit_start_list(Visitor *v, const char *name, Error **errp) @@ -62,9 +61,8 @@ GenericList *visit_next_list(Visitor *v, GenericList **list, Error **errp) void visit_end_list(Visitor *v, Error **errp) { - if (!error_is_set(errp)) { - v->end_list(v, errp); - } + assert(!error_is_set(errp)); + v->end_list(v, errp); } void visit_start_optional(Visitor *v, bool *present, const char *name, diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 8d4e94a45f..04ef7c41ab 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -17,32 +17,49 @@ import os import getopt import errno -def generate_visit_struct_body(field_prefix, members): - ret = "" +def generate_visit_struct_body(field_prefix, name, members): + ret = mcgen(''' +if (!error_is_set(errp)) { +''') + push_indent() + if len(field_prefix): field_prefix = field_prefix + "." + ret += mcgen(''' +Error **errp = &err; /* from outer scope */ +Error *err = NULL; +visit_start_struct(m, NULL, "", "%(name)s", 0, &err); +''', + name=name) + else: + ret += mcgen(''' +Error *err = NULL; +visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err); +''', + name=name) + + ret += mcgen(''' +if (!err) { + if (!obj || *obj) { +''') + + push_indent() + push_indent() for argname, argentry, optional, structured in parse_args(members): if optional: ret += mcgen(''' -visit_start_optional(m, (obj && *obj) ? &(*obj)->%(c_prefix)shas_%(c_name)s : NULL, "%(name)s", errp); -if ((*obj)->%(prefix)shas_%(c_name)s) { +visit_start_optional(m, obj ? &(*obj)->%(c_prefix)shas_%(c_name)s : NULL, "%(name)s", &err); +if (obj && (*obj)->%(prefix)shas_%(c_name)s) { ''', c_prefix=c_var(field_prefix), prefix=field_prefix, c_name=c_var(argname), name=argname) push_indent() if structured: - ret += mcgen(''' -visit_start_struct(m, NULL, "", "%(name)s", 0, errp); -''', - name=argname) - ret += generate_visit_struct_body(field_prefix + argname, argentry) - ret += mcgen(''' -visit_end_struct(m, errp); -''') + ret += generate_visit_struct_body(field_prefix + argname, argname, argentry) else: ret += mcgen(''' -visit_type_%(type)s(m, (obj && *obj) ? &(*obj)->%(c_prefix)s%(c_name)s : NULL, "%(name)s", errp); +visit_type_%(type)s(m, obj ? &(*obj)->%(c_prefix)s%(c_name)s : NULL, "%(name)s", &err); ''', c_prefix=c_var(field_prefix), prefix=field_prefix, type=type_name(argentry), c_name=c_var(argname), @@ -52,7 +69,25 @@ visit_type_%(type)s(m, (obj && *obj) ? &(*obj)->%(c_prefix)s%(c_name)s : NULL, " pop_indent() ret += mcgen(''' } -visit_end_optional(m, errp); +visit_end_optional(m, &err); +''') + + pop_indent() + ret += mcgen(''' + + error_propagate(errp, err); + err = NULL; +} +''') + + pop_indent() + pop_indent() + ret += mcgen(''' + /* Always call end_struct if start_struct succeeded. */ + visit_end_struct(m, &err); + } + error_propagate(errp, err); +} ''') return ret @@ -61,22 +96,14 @@ def generate_visit_struct(name, members): void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **errp) { - if (error_is_set(errp)) { - return; - } - visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), errp); - if (obj && !*obj) { - goto end; - } ''', name=name) + push_indent() - ret += generate_visit_struct_body("", members) + ret += generate_visit_struct_body("", name, members) pop_indent() ret += mcgen(''' -end: - visit_end_struct(m, errp); } ''') return ret @@ -87,18 +114,23 @@ def generate_visit_list(name, members): void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char *name, Error **errp) { GenericList *i, **prev = (GenericList **)obj; + Error *err = NULL; - if (error_is_set(errp)) { - return; - } - visit_start_list(m, name, errp); - - for (; (i = visit_next_list(m, prev, errp)) != NULL; prev = &i) { - %(name)sList *native_i = (%(name)sList *)i; - visit_type_%(name)s(m, &native_i->value, NULL, errp); + if (!error_is_set(errp)) { + visit_start_list(m, name, &err); + if (!err) { + for (; (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); + } + error_propagate(errp, err); + err = NULL; + + /* Always call end_list if start_list succeeded. */ + visit_end_list(m, &err); + } + error_propagate(errp, err); } - - visit_end_list(m, errp); } ''', name=name) @@ -122,27 +154,23 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error ** { Error *err = NULL; - if (error_is_set(errp)) { - return; - } - visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err); - if (obj && !*obj) { - goto end; - } - visit_type_%(name)sKind(m, &(*obj)->kind, "type", &err); - if (err) { - error_propagate(errp, err); - goto end; - } - switch ((*obj)->kind) { + if (!error_is_set(errp)) { + visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err); + if (!err) { + if (!obj || *obj) { + visit_type_%(name)sKind(m, &(*obj)->kind, "type", &err); + if (!err) { + switch ((*obj)->kind) { ''', name=name) + push_indent() + push_indent() for key in members: ret += mcgen(''' - case %(abbrev)s_KIND_%(enum)s: - visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "data", errp); - break; + case %(abbrev)s_KIND_%(enum)s: + visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "data", &err); + break; ''', abbrev = de_camel_case(name).upper(), enum = c_fun(de_camel_case(key)).upper(), @@ -150,11 +178,25 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error ** c_name=c_fun(key)) ret += mcgen(''' - default: - abort(); + default: + abort(); + } + } + error_propagate(errp, err); + err = NULL; + } +''') + pop_indent() + ret += mcgen(''' + /* Always call end_struct if start_struct succeeded. */ + visit_end_struct(m, &err); } -end: - visit_end_struct(m, errp); + error_propagate(errp, err); +} +''') + + pop_indent(); + ret += mcgen(''' } ''') diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c index c30fdc4e59..8f5a509582 100644 --- a/tests/test-qmp-input-visitor.c +++ b/tests/test-qmp-input-visitor.c @@ -151,14 +151,22 @@ typedef struct TestStruct static void visit_type_TestStruct(Visitor *v, TestStruct **obj, const char *name, Error **errp) { - visit_start_struct(v, (void **)obj, "TestStruct", name, sizeof(TestStruct), - errp); - - visit_type_int(v, &(*obj)->integer, "integer", errp); - visit_type_bool(v, &(*obj)->boolean, "boolean", errp); - visit_type_str(v, &(*obj)->string, "string", errp); - - visit_end_struct(v, errp); + Error *err = NULL; + if (!error_is_set(errp)) { + visit_start_struct(v, (void **)obj, "TestStruct", name, sizeof(TestStruct), + &err); + if (!err) { + visit_type_int(v, &(*obj)->integer, "integer", &err); + visit_type_bool(v, &(*obj)->boolean, "boolean", &err); + visit_type_str(v, &(*obj)->string, "string", &err); + + /* Always call end_struct if start_struct succeeded. */ + error_propagate(errp, err); + err = NULL; + visit_end_struct(v, &err); + } + error_propagate(errp, err); + } } static void test_visitor_in_struct(TestInputVisitorData *data, |