From 1158bb2a058fcdd0c8fc3e60dc77f7a57ddbb271 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 9 Jun 2016 10:48:34 -0600 Subject: qapi: Add parameter to visit_end_* Rather than making the dealloc visitor track of stack of pointers remembered during visit_start_* in order to free them during visit_end_*, it's a lot easier to just make all callers pass the same pointer to visit_end_*. The generated code has access to the same pointer, while all other users are doing virtual walks and can pass NULL. The dealloc visitor is then greatly simplified. All three visit_end_*() functions intentionally take a void**, even though the visit_start_*() functions differ between void**, GenericList**, and GenericAlternate**. This is done for several reasons: when doing a virtual walk, passing NULL doesn't care what the type is, but when doing a generated walk, we already have to cast the caller's specific FOO* to call visit_start, while using void** lets us use visit_end without a cast. Also, an upcoming patch will add a clone visitor that wants to use the same implementation for all three visit_end callbacks, which is made easier if all three share the same signature. For visitors with already track per-object state (the QMP visitors via a stack, and the string visitors which do not allow nesting), add an assertion that the caller is indeed passing the same pointer to paired calls. Signed-off-by: Eric Blake Message-Id: <1465490926-28625-4-git-send-email-eblake@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- scripts/qapi-commands.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'scripts/qapi-commands.py') diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 8c6acb3f3f..971dc4ea8e 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -129,7 +129,7 @@ def gen_marshal(name, arg_type, ret_type): if (!err) { visit_check_struct(v, &err); } - visit_end_struct(v); + visit_end_struct(v, NULL); if (err) { goto out; } @@ -160,7 +160,7 @@ out: v = qapi_dealloc_get_visitor(qdv); visit_start_struct(v, NULL, NULL, 0, NULL); visit_type_%(c_name)s_members(v, &arg, NULL); - visit_end_struct(v); + visit_end_struct(v, NULL); qapi_dealloc_visitor_cleanup(qdv); ''', c_name=arg_type.c_name()) -- cgit 1.4.1 From 2c0ef9f411ae6081efa9eca5b3eab2dbeee45a6c Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 9 Jun 2016 10:48:35 -0600 Subject: qapi: Add new visit_free() function Making each visitor provide its own (awkwardly-named) FOO_cleanup() is unusual, when we can instead have a polymorphic visit_free() interface. Over the next few patches, we can use the polymorphic functions to eliminate the need for a FOO_get_visitor() function for accessing specific visitor functionality, once everything can be accessed directly through the Visitor* interfaces. The dealloc visitor is the first one converted to completely use the new entry point, since qapi_dealloc_visitor_cleanup() was the only reason that qapi_dealloc_get_visitor() existed, and only generated and testsuite code was even using it. With the new visit_free() entry point in place, we no longer need to expose the QapiDeallocVisitor subtype through qapi_dealloc_visitor_new(), and can get by with less generated code, with diffs that look like: | void qapi_free_ACPIOSTInfo(ACPIOSTInfo *obj) | { |- QapiDeallocVisitor *qdv; | Visitor *v; | | if (!obj) { | return; | } | |- qdv = qapi_dealloc_visitor_new(); |- v = qapi_dealloc_get_visitor(qdv); |+ v = qapi_dealloc_visitor_new(); | visit_type_ACPIOSTInfo(v, NULL, &obj, NULL); |- qapi_dealloc_visitor_cleanup(qdv); |+ visit_free(v); |} Signed-off-by: Eric Blake Message-Id: <1465490926-28625-5-git-send-email-eblake@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- docs/qapi-code-gen.txt | 28 ++++++++++------------------ include/qapi/dealloc-visitor.h | 5 +---- include/qapi/visitor-impl.h | 3 +++ include/qapi/visitor.h | 37 ++++++++++++++++++++++++++++++++++--- qapi/opts-visitor.c | 10 ++++++++++ qapi/qapi-dealloc-visitor.c | 14 +++++--------- qapi/qapi-visit-core.c | 7 +++++++ qapi/qmp-input-visitor.c | 8 ++++++++ qapi/qmp-output-visitor.c | 8 ++++++++ qapi/string-input-visitor.c | 8 ++++++++ qapi/string-output-visitor.c | 8 ++++++++ scripts/qapi-commands.py | 16 ++++++---------- scripts/qapi-event.py | 2 +- scripts/qapi-types.py | 6 ++---- tests/test-visitor-serialization.c | 6 +++--- 15 files changed, 114 insertions(+), 52 deletions(-) (limited to 'scripts/qapi-commands.py') diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index 0ae239ad6c..db9d5b6357 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -802,32 +802,28 @@ Example: void qapi_free_UserDefOne(UserDefOne *obj) { - QapiDeallocVisitor *qdv; Visitor *v; if (!obj) { return; } - qdv = qapi_dealloc_visitor_new(); - v = qapi_dealloc_get_visitor(qdv); + v = qapi_dealloc_visitor_new(); visit_type_UserDefOne(v, NULL, &obj, NULL); - qapi_dealloc_visitor_cleanup(qdv); + visit_free(v); } void qapi_free_UserDefOneList(UserDefOneList *obj) { - QapiDeallocVisitor *qdv; Visitor *v; if (!obj) { return; } - qdv = qapi_dealloc_visitor_new(); - v = qapi_dealloc_get_visitor(qdv); + v = qapi_dealloc_visitor_new(); visit_type_UserDefOneList(v, NULL, &obj, NULL); - qapi_dealloc_visitor_cleanup(qdv); + visit_free(v); } === scripts/qapi-visit.py === @@ -985,7 +981,6 @@ Example: { Error *err = NULL; QmpOutputVisitor *qov = qmp_output_visitor_new(); - QapiDeallocVisitor *qdv; Visitor *v; v = qmp_output_get_visitor(qov); @@ -997,11 +992,10 @@ Example: out: error_propagate(errp, err); - qmp_output_visitor_cleanup(qov); - qdv = qapi_dealloc_visitor_new(); - v = qapi_dealloc_get_visitor(qdv); + visit_free(v); + v = qapi_dealloc_visitor_new(); visit_type_UserDefOne(v, "unused", &ret_in, NULL); - qapi_dealloc_visitor_cleanup(qdv); + visit_free(v); } static void qmp_marshal_my_command(QDict *args, QObject **ret, Error **errp) @@ -1009,7 +1003,6 @@ Example: Error *err = NULL; UserDefOne *retval; QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true); - QapiDeallocVisitor *qdv; Visitor *v; UserDefOneList *arg1 = NULL; @@ -1036,13 +1029,12 @@ Example: out: error_propagate(errp, err); - qmp_input_visitor_cleanup(qiv); - qdv = qapi_dealloc_visitor_new(); - v = qapi_dealloc_get_visitor(qdv); + visit_free(v); + v = qapi_dealloc_visitor_new(); visit_start_struct(v, NULL, NULL, 0, NULL); visit_type_UserDefOneList(v, "arg1", &arg1, NULL); visit_end_struct(v, NULL); - qapi_dealloc_visitor_cleanup(qdv); + visit_free(v); } static void qmp_init_marshal(void) diff --git a/include/qapi/dealloc-visitor.h b/include/qapi/dealloc-visitor.h index 45b06b248c..b3e5c85fd8 100644 --- a/include/qapi/dealloc-visitor.h +++ b/include/qapi/dealloc-visitor.h @@ -23,9 +23,6 @@ typedef struct QapiDeallocVisitor QapiDeallocVisitor; * qapi_free_FOO() functions, and is the only visitor designed to work * correctly in the face of a partially-constructed QAPI tree. */ -QapiDeallocVisitor *qapi_dealloc_visitor_new(void); -void qapi_dealloc_visitor_cleanup(QapiDeallocVisitor *d); - -Visitor *qapi_dealloc_get_visitor(QapiDeallocVisitor *v); +Visitor *qapi_dealloc_visitor_new(void); #endif diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h index a495bf0937..525b06872b 100644 --- a/include/qapi/visitor-impl.h +++ b/include/qapi/visitor-impl.h @@ -104,6 +104,9 @@ struct Visitor /* Must be set */ VisitorType type; + + /* Must be set */ + void (*free)(Visitor *v); }; #endif diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index 25d0cc275a..2ded852307 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -37,6 +37,24 @@ * implemented by each visitor, and docs/qapi-code-gen.txt for more * about the QAPI code generator. * + * All of the visitors are created via: + * + * Type *subtype_visitor_new(parameters...); + * + * where Type is either directly 'Visitor *', or is a subtype that can + * be trivially upcast to Visitor * via another function: + * + * Visitor *subtype_get_visitor(SubtypeVisitor *); + * + * A visitor should be used for exactly one top-level visit_type_FOO() + * or virtual walk, then passed to visit_free() to clean up resources. + * It is okay to free the visitor without completing the visit, if + * some other error is detected in the meantime. Output visitors + * provide an additional function, for collecting the final results of + * a successful visit: string_output_get_string() and + * qmp_output_get_qobject(); this collection function should not be + * called if any errors were reported during the visit. + * * All QAPI types have a corresponding function with a signature * roughly compatible with this: * @@ -222,6 +240,19 @@ typedef struct GenericAlternate { char padding[]; } GenericAlternate; +/*** Visitor cleanup ***/ + +/* + * Free @v and any resources it has tied up. + * + * May be called whether or not the visit has been successfully + * completed, but should not be called until a top-level + * visit_type_FOO() or visit_start_ITEM() has been performed on the + * visitor. Safe if @v is NULL. + */ +void visit_free(Visitor *v); + + /*** Visiting structures ***/ /* @@ -272,7 +303,7 @@ void visit_check_struct(Visitor *v, Error **errp); * Must be called after any successful use of visit_start_struct(), * even if intermediate processing was skipped due to errors, to allow * the backend to release any resources. Destroying the visitor early - * behaves as if this was implicitly called. + * with visit_free() behaves as if this was implicitly called. */ void visit_end_struct(Visitor *v, void **obj); @@ -332,7 +363,7 @@ GenericList *visit_next_list(Visitor *v, GenericList *tail, size_t size); * Must be called after any successful use of visit_start_list(), even * if intermediate processing was skipped due to errors, to allow the * backend to release any resources. Destroying the visitor early - * behaves as if this was implicitly called. + * with visit_free() behaves as if this was implicitly called. */ void visit_end_list(Visitor *v, void **list); @@ -368,7 +399,7 @@ void visit_start_alternate(Visitor *v, const char *name, * Must be called after any successful use of visit_start_alternate(), * even if intermediate processing was skipped due to errors, to allow * the backend to release any resources. Destroying the visitor early - * behaves as if this was implicitly called. + * with visit_free() behaves as if this was implicitly called. * */ void visit_end_alternate(Visitor *v, void **obj); diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index dcfbf92faf..72c95ac7c1 100644 --- a/qapi/opts-visitor.c +++ b/qapi/opts-visitor.c @@ -513,6 +513,15 @@ opts_optional(Visitor *v, const char *name, bool *present) } +static void +opts_free(Visitor *v) +{ + OptsVisitor *ov = to_ov(v); + + opts_visitor_cleanup(ov); +} + + OptsVisitor * opts_visitor_new(const QemuOpts *opts) { @@ -540,6 +549,7 @@ opts_visitor_new(const QemuOpts *opts) * skip some mandatory methods... */ ov->visitor.optional = &opts_optional; + ov->visitor.free = opts_free; ov->opts_root = opts; diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c index 9391dea203..e39457bc79 100644 --- a/qapi/qapi-dealloc-visitor.c +++ b/qapi/qapi-dealloc-visitor.c @@ -107,17 +107,12 @@ static void qapi_dealloc_type_null(Visitor *v, const char *name, Error **errp) { } -Visitor *qapi_dealloc_get_visitor(QapiDeallocVisitor *v) +static void qapi_dealloc_free(Visitor *v) { - return &v->visitor; + g_free(container_of(v, QapiDeallocVisitor, visitor)); } -void qapi_dealloc_visitor_cleanup(QapiDeallocVisitor *v) -{ - g_free(v); -} - -QapiDeallocVisitor *qapi_dealloc_visitor_new(void) +Visitor *qapi_dealloc_visitor_new(void) { QapiDeallocVisitor *v; @@ -138,6 +133,7 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void) v->visitor.type_number = qapi_dealloc_type_number; v->visitor.type_any = qapi_dealloc_type_anything; v->visitor.type_null = qapi_dealloc_type_null; + v->visitor.free = qapi_dealloc_free; - return v; + return &v->visitor; } diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c index dba11c66a7..5f68c251d2 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qapi-visit-core.c @@ -20,6 +20,13 @@ #include "qapi/visitor.h" #include "qapi/visitor-impl.h" +void visit_free(Visitor *v) +{ + if (v) { + v->free(v); + } +} + void visit_start_struct(Visitor *v, const char *name, void **obj, size_t size, Error **errp) { diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index e16b4b0725..41b7f87ddb 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -378,6 +378,13 @@ Visitor *qmp_input_get_visitor(QmpInputVisitor *v) return &v->visitor; } +static void qmp_input_free(Visitor *v) +{ + QmpInputVisitor *qiv = to_qiv(v); + + qmp_input_visitor_cleanup(qiv); +} + void qmp_input_visitor_cleanup(QmpInputVisitor *v) { qobject_decref(v->root); @@ -406,6 +413,7 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj, bool strict) v->visitor.type_any = qmp_input_type_any; v->visitor.type_null = qmp_input_type_null; v->visitor.optional = qmp_input_optional; + v->visitor.free = qmp_input_free; v->strict = strict; v->root = obj; diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c index dca893584d..5f0035cbf6 100644 --- a/qapi/qmp-output-visitor.c +++ b/qapi/qmp-output-visitor.c @@ -214,6 +214,13 @@ Visitor *qmp_output_get_visitor(QmpOutputVisitor *v) return &v->visitor; } +static void qmp_output_free(Visitor *v) +{ + QmpOutputVisitor *qov = to_qov(v); + + qmp_output_visitor_cleanup(qov); +} + void qmp_output_visitor_cleanup(QmpOutputVisitor *v) { QStackEntry *e, *tmp; @@ -246,6 +253,7 @@ QmpOutputVisitor *qmp_output_visitor_new(void) v->visitor.type_number = qmp_output_type_number; v->visitor.type_any = qmp_output_type_any; v->visitor.type_null = qmp_output_type_null; + v->visitor.free = qmp_output_free; QTAILQ_INIT(&v->stack); diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c index d7f3c2bd1e..6fb1229586 100644 --- a/qapi/string-input-visitor.c +++ b/qapi/string-input-visitor.c @@ -331,6 +331,13 @@ Visitor *string_input_get_visitor(StringInputVisitor *v) return &v->visitor; } +static void string_input_free(Visitor *v) +{ + StringInputVisitor *siv = to_siv(v); + + string_input_visitor_cleanup(siv); +} + void string_input_visitor_cleanup(StringInputVisitor *v) { g_list_foreach(v->ranges, free_range, NULL); @@ -355,6 +362,7 @@ StringInputVisitor *string_input_visitor_new(const char *str) v->visitor.next_list = next_list; v->visitor.end_list = end_list; v->visitor.optional = parse_optional; + v->visitor.free = string_input_free; v->string = str; return v; diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c index bdc0fd0be4..ff9ddf068a 100644 --- a/qapi/string-output-visitor.c +++ b/qapi/string-output-visitor.c @@ -322,6 +322,13 @@ static void free_range(void *range, void *dummy) g_free(range); } +static void string_output_free(Visitor *v) +{ + StringOutputVisitor *sov = to_sov(v); + + string_output_visitor_cleanup(sov); +} + void string_output_visitor_cleanup(StringOutputVisitor *sov) { if (sov->string) { @@ -351,6 +358,7 @@ StringOutputVisitor *string_output_visitor_new(bool human) v->visitor.start_list = start_list; v->visitor.next_list = next_list; v->visitor.end_list = end_list; + v->visitor.free = string_output_free; return v; } diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 971dc4ea8e..77ecd47071 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -62,7 +62,6 @@ static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out, { Error *err = NULL; QmpOutputVisitor *qov = qmp_output_visitor_new(); - QapiDeallocVisitor *qdv; Visitor *v; v = qmp_output_get_visitor(qov); @@ -74,11 +73,10 @@ static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out, out: error_propagate(errp, err); - qmp_output_visitor_cleanup(qov); - qdv = qapi_dealloc_visitor_new(); - v = qapi_dealloc_get_visitor(qdv); + visit_free(v); + v = qapi_dealloc_visitor_new(); visit_type_%(c_name)s(v, "unused", &ret_in, NULL); - qapi_dealloc_visitor_cleanup(qdv); + visit_free(v); } ''', c_type=ret_type.c_type(), c_name=ret_type.c_name()) @@ -116,7 +114,6 @@ def gen_marshal(name, arg_type, ret_type): if arg_type and arg_type.members: ret += mcgen(''' QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true); - QapiDeallocVisitor *qdv; Visitor *v; %(c_name)s arg = {0}; @@ -155,13 +152,12 @@ out: ''') if arg_type and arg_type.members: ret += mcgen(''' - qmp_input_visitor_cleanup(qiv); - qdv = qapi_dealloc_visitor_new(); - v = qapi_dealloc_get_visitor(qdv); + visit_free(v); + v = qapi_dealloc_visitor_new(); visit_start_struct(v, NULL, NULL, 0, NULL); visit_type_%(c_name)s_members(v, &arg, NULL); visit_end_struct(v, NULL); - qapi_dealloc_visitor_cleanup(qdv); + visit_free(v); ''', c_name=arg_type.c_name()) diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py index 084c40a100..909e8d6b8f 100644 --- a/scripts/qapi-event.py +++ b/scripts/qapi-event.py @@ -119,7 +119,7 @@ def gen_event_send(name, arg_type): if arg_type and arg_type.members: ret += mcgen(''' out: - qmp_output_visitor_cleanup(qov); + visit_free(v); ''') ret += mcgen(''' error_propagate(errp, err); diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index 437cf6c8e3..5ace2cf065 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -150,17 +150,15 @@ def gen_type_cleanup(name): void qapi_free_%(c_name)s(%(c_name)s *obj) { - QapiDeallocVisitor *qdv; Visitor *v; if (!obj) { return; } - qdv = qapi_dealloc_visitor_new(); - v = qapi_dealloc_get_visitor(qdv); + v = qapi_dealloc_visitor_new(); visit_type_%(c_name)s(v, NULL, &obj, NULL); - qapi_dealloc_visitor_cleanup(qdv); + visit_free(v); } ''', c_name=c_name(name)) diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c index db781c0a79..bcbfd2aeb7 100644 --- a/tests/test-visitor-serialization.c +++ b/tests/test-visitor-serialization.c @@ -89,11 +89,11 @@ typedef void (*VisitorFunc)(Visitor *v, void **native, Error **errp); static void dealloc_helper(void *native_in, VisitorFunc visit, Error **errp) { - QapiDeallocVisitor *qdv = qapi_dealloc_visitor_new(); + Visitor *v = qapi_dealloc_visitor_new(); - visit(qapi_dealloc_get_visitor(qdv), &native_in, errp); + visit(v, &native_in, errp); - qapi_dealloc_visitor_cleanup(qdv); + visit_free(v); } static void visit_primitive_type(Visitor *v, void **native, Error **errp) -- cgit 1.4.1 From b70ce1018a251c0c33498d9c927a07cade655a5e Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 9 Jun 2016 10:48:38 -0600 Subject: qmp-input-visitor: Favor new visit_free() function Now that we have a polymorphic visit_free(), we no longer need qmp_input_visitor_cleanup(); which in turn means we no longer need to return a subtype from qmp_input_visitor_new() nor a public upcast function. Generated code changes to qmp-marshal.c look like: |@@ -52,11 +52,10 @@ void qmp_marshal_add_fd(QDict *args, QOb | { | Error *err = NULL; | AddfdInfo *retval; |- QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true); | Visitor *v; | q_obj_add_fd_arg arg = {0}; | |- v = qmp_input_get_visitor(qiv); |+ v = qmp_input_visitor_new(QOBJECT(args), true); | visit_start_struct(v, NULL, NULL, 0, &err); | if (err) { | goto out; Signed-off-by: Eric Blake Message-Id: <1465490926-28625-8-git-send-email-eblake@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- docs/qapi-code-gen.txt | 3 +-- include/qapi/qmp-input-visitor.h | 6 +----- qapi/qmp-input-visitor.c | 18 ++++-------------- qmp.c | 9 ++++----- qom/qom-qobject.c | 9 ++++----- replay/replay-input.c | 6 ++---- scripts/qapi-commands.py | 3 +-- tests/check-qnull.c | 8 ++++---- tests/test-qmp-commands.c | 8 ++++---- tests/test-qmp-input-strict.c | 12 +++--------- tests/test-qmp-input-visitor.c | 12 +++--------- tests/test-visitor-serialization.c | 6 +++--- util/qemu-sockets.c | 6 ++---- 13 files changed, 36 insertions(+), 70 deletions(-) (limited to 'scripts/qapi-commands.py') diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index db9d5b6357..7c30762720 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -1002,11 +1002,10 @@ Example: { Error *err = NULL; UserDefOne *retval; - QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true); Visitor *v; UserDefOneList *arg1 = NULL; - v = qmp_input_get_visitor(qiv); + v = qmp_input_visitor_new(QOBJECT(args), true); visit_start_struct(v, NULL, NULL, 0, &err); if (err) { goto out; diff --git a/include/qapi/qmp-input-visitor.h b/include/qapi/qmp-input-visitor.h index b0624d8683..f3ff5f3e98 100644 --- a/include/qapi/qmp-input-visitor.h +++ b/include/qapi/qmp-input-visitor.h @@ -25,10 +25,6 @@ typedef struct QmpInputVisitor QmpInputVisitor; * Set @strict to reject a parse that doesn't consume all keys of a * dictionary; otherwise excess input is ignored. */ -QmpInputVisitor *qmp_input_visitor_new(QObject *obj, bool strict); - -void qmp_input_visitor_cleanup(QmpInputVisitor *v); - -Visitor *qmp_input_get_visitor(QmpInputVisitor *v); +Visitor *qmp_input_visitor_new(QObject *obj, bool strict); #endif diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index 41b7f87ddb..21edb3928e 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -373,25 +373,15 @@ static void qmp_input_optional(Visitor *v, const char *name, bool *present) *present = true; } -Visitor *qmp_input_get_visitor(QmpInputVisitor *v) -{ - return &v->visitor; -} - static void qmp_input_free(Visitor *v) { QmpInputVisitor *qiv = to_qiv(v); - qmp_input_visitor_cleanup(qiv); + qobject_decref(qiv->root); + g_free(qiv); } -void qmp_input_visitor_cleanup(QmpInputVisitor *v) -{ - qobject_decref(v->root); - g_free(v); -} - -QmpInputVisitor *qmp_input_visitor_new(QObject *obj, bool strict) +Visitor *qmp_input_visitor_new(QObject *obj, bool strict) { QmpInputVisitor *v; @@ -419,5 +409,5 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj, bool strict) v->root = obj; qobject_incref(obj); - return v; + return &v->visitor; } diff --git a/qmp.c b/qmp.c index 7df6543871..b6d531ebe2 100644 --- a/qmp.c +++ b/qmp.c @@ -655,7 +655,7 @@ void qmp_object_add(const char *type, const char *id, bool has_props, QObject *props, Error **errp) { const QDict *pdict = NULL; - QmpInputVisitor *qiv; + Visitor *v; Object *obj; if (props) { @@ -666,10 +666,9 @@ void qmp_object_add(const char *type, const char *id, } } - qiv = qmp_input_visitor_new(props, true); - obj = user_creatable_add_type(type, id, pdict, - qmp_input_get_visitor(qiv), errp); - qmp_input_visitor_cleanup(qiv); + v = qmp_input_visitor_new(props, true); + obj = user_creatable_add_type(type, id, pdict, v, errp); + visit_free(v); if (obj) { object_unref(obj); } diff --git a/qom/qom-qobject.c b/qom/qom-qobject.c index b66088d730..c3c9188d00 100644 --- a/qom/qom-qobject.c +++ b/qom/qom-qobject.c @@ -21,12 +21,11 @@ void object_property_set_qobject(Object *obj, QObject *value, const char *name, Error **errp) { - QmpInputVisitor *qiv; + Visitor *v; /* TODO: Should we reject, rather than ignore, excess input? */ - qiv = qmp_input_visitor_new(value, false); - object_property_set(obj, qmp_input_get_visitor(qiv), name, errp); - - qmp_input_visitor_cleanup(qiv); + v = qmp_input_visitor_new(value, false); + object_property_set(obj, v, name, errp); + visit_free(v); } QObject *object_property_get_qobject(Object *obj, const char *name, diff --git a/replay/replay-input.c b/replay/replay-input.c index 03e99d5aba..7b6fa93b8d 100644 --- a/replay/replay-input.c +++ b/replay/replay-input.c @@ -23,7 +23,6 @@ static InputEvent *qapi_clone_InputEvent(InputEvent *src) { QmpOutputVisitor *qov; - QmpInputVisitor *qiv; Visitor *ov, *iv; QObject *obj; InputEvent *dst = NULL; @@ -37,10 +36,9 @@ static InputEvent *qapi_clone_InputEvent(InputEvent *src) return NULL; } - qiv = qmp_input_visitor_new(obj, true); - iv = qmp_input_get_visitor(qiv); + iv = qmp_input_visitor_new(obj, true); visit_type_InputEvent(iv, NULL, &dst, &error_abort); - qmp_input_visitor_cleanup(qiv); + visit_free(iv); qobject_decref(obj); return dst; diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 77ecd47071..49d4ce2590 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -113,11 +113,10 @@ def gen_marshal(name, arg_type, ret_type): if arg_type and arg_type.members: ret += mcgen(''' - QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true); Visitor *v; %(c_name)s arg = {0}; - v = qmp_input_get_visitor(qiv); + v = qmp_input_visitor_new(QOBJECT(args), true); visit_start_struct(v, NULL, NULL, 0, &err); if (err) { goto out; diff --git a/tests/check-qnull.c b/tests/check-qnull.c index 05d562d494..d0412e4169 100644 --- a/tests/check-qnull.c +++ b/tests/check-qnull.c @@ -38,7 +38,7 @@ static void qnull_visit_test(void) { QObject *obj; QmpOutputVisitor *qov; - QmpInputVisitor *qiv; + Visitor *v; /* * Most tests of interactions between QObject and visitors are in @@ -48,10 +48,10 @@ static void qnull_visit_test(void) g_assert(qnull_.refcnt == 1); obj = qnull(); - qiv = qmp_input_visitor_new(obj, true); + v = qmp_input_visitor_new(obj, true); qobject_decref(obj); - visit_type_null(qmp_input_get_visitor(qiv), NULL, &error_abort); - qmp_input_visitor_cleanup(qiv); + visit_type_null(v, NULL, &error_abort); + visit_free(v); qov = qmp_output_visitor_new(); visit_type_null(qmp_output_get_visitor(qov), NULL, &error_abort); diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c index e8f619d331..8ffeb045cd 100644 --- a/tests/test-qmp-commands.c +++ b/tests/test-qmp-commands.c @@ -216,14 +216,14 @@ static void test_dealloc_partial(void) /* create partial object */ { QDict *ud2_dict; - QmpInputVisitor *qiv; + Visitor *v; ud2_dict = qdict_new(); qdict_put_obj(ud2_dict, "string0", QOBJECT(qstring_from_str(text))); - qiv = qmp_input_visitor_new(QOBJECT(ud2_dict), true); - visit_type_UserDefTwo(qmp_input_get_visitor(qiv), NULL, &ud2, &err); - qmp_input_visitor_cleanup(qiv); + v = qmp_input_visitor_new(QOBJECT(ud2_dict), true); + visit_type_UserDefTwo(v, NULL, &ud2, &err); + visit_free(v); QDECREF(ud2_dict); } diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c index 79739e9808..814550ac71 100644 --- a/tests/test-qmp-input-strict.c +++ b/tests/test-qmp-input-strict.c @@ -26,7 +26,7 @@ typedef struct TestInputVisitorData { QObject *obj; - QmpInputVisitor *qiv; + Visitor *qiv; } TestInputVisitorData; static void validate_teardown(TestInputVisitorData *data, @@ -36,7 +36,7 @@ static void validate_teardown(TestInputVisitorData *data, data->obj = NULL; if (data->qiv) { - qmp_input_visitor_cleanup(data->qiv); + visit_free(data->qiv); data->qiv = NULL; } } @@ -48,8 +48,6 @@ static Visitor *validate_test_init_internal(TestInputVisitorData *data, const char *json_string, va_list *ap) { - Visitor *v; - validate_teardown(data, NULL); data->obj = qobject_from_jsonv(json_string, ap); @@ -57,11 +55,7 @@ static Visitor *validate_test_init_internal(TestInputVisitorData *data, data->qiv = qmp_input_visitor_new(data->obj, true); g_assert(data->qiv); - - v = qmp_input_get_visitor(data->qiv); - g_assert(v); - - return v; + return data->qiv; } static GCC_FMT_ATTR(2, 3) diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c index b236183ec0..f583dce3ed 100644 --- a/tests/test-qmp-input-visitor.c +++ b/tests/test-qmp-input-visitor.c @@ -22,7 +22,7 @@ typedef struct TestInputVisitorData { QObject *obj; - QmpInputVisitor *qiv; + Visitor *qiv; } TestInputVisitorData; static void visitor_input_teardown(TestInputVisitorData *data, @@ -32,7 +32,7 @@ static void visitor_input_teardown(TestInputVisitorData *data, data->obj = NULL; if (data->qiv) { - qmp_input_visitor_cleanup(data->qiv); + visit_free(data->qiv); data->qiv = NULL; } } @@ -44,8 +44,6 @@ static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data, const char *json_string, va_list *ap) { - Visitor *v; - visitor_input_teardown(data, NULL); data->obj = qobject_from_jsonv(json_string, ap); @@ -53,11 +51,7 @@ static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data, data->qiv = qmp_input_visitor_new(data->obj, false); g_assert(data->qiv); - - v = qmp_input_get_visitor(data->qiv); - g_assert(v); - - return v; + return data->qiv; } static GCC_FMT_ATTR(2, 3) diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c index c3c8634294..0766dcc5a1 100644 --- a/tests/test-visitor-serialization.c +++ b/tests/test-visitor-serialization.c @@ -1013,7 +1013,7 @@ static PrimitiveType pt_values[] = { typedef struct QmpSerializeData { QmpOutputVisitor *qov; - QmpInputVisitor *qiv; + Visitor *qiv; } QmpSerializeData; static void qmp_serialize(void *native_in, void **datap, @@ -1041,14 +1041,14 @@ static void qmp_deserialize(void **native_out, void *datap, d->qiv = qmp_input_visitor_new(obj, true); qobject_decref(obj_orig); qobject_decref(obj); - visit(qmp_input_get_visitor(d->qiv), native_out, errp); + visit(d->qiv, native_out, errp); } static void qmp_cleanup(void *datap) { QmpSerializeData *d = datap; qmp_output_visitor_cleanup(d->qov); - qmp_input_visitor_cleanup(d->qiv); + visit_free(d->qiv); g_free(d); } diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c index cc2b043907..3677cd21dc 100644 --- a/util/qemu-sockets.c +++ b/util/qemu-sockets.c @@ -1148,7 +1148,6 @@ void qapi_copy_SocketAddress(SocketAddress **p_dest, SocketAddress *src) { QmpOutputVisitor *qov; - QmpInputVisitor *qiv; Visitor *ov, *iv; QObject *obj; @@ -1163,10 +1162,9 @@ void qapi_copy_SocketAddress(SocketAddress **p_dest, return; } - qiv = qmp_input_visitor_new(obj, true); - iv = qmp_input_get_visitor(qiv); + iv = qmp_input_visitor_new(obj, true); visit_type_SocketAddress(iv, NULL, p_dest, &error_abort); - qmp_input_visitor_cleanup(qiv); + visit_free(iv); qobject_decref(obj); } -- cgit 1.4.1 From 3b098d56979d2f7fd707c5be85555d114353a28d Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 9 Jun 2016 10:48:43 -0600 Subject: qapi: Add new visit_complete() function Making each output visitor provide its own output collection function was the only remaining reason for exposing visitor sub-types to the rest of the code base. Add a polymorphic visit_complete() function which is a no-op for input visitors, and which populates an opaque pointer for output visitors. For maximum type-safety, also add a parameter to the output visitor constructors with a type-correct version of the output pointer, and assert that the two uses match. This approach was considered superior to either passing the output parameter only during construction (action at a distance during visit_free() feels awkward) or only during visit_complete() (defeating type safety makes it easier to use incorrectly). Most callers were function-local, and therefore a mechanical conversion; the testsuite was a bit trickier, but the previous cleanup patch minimized the churn here. The visit_complete() function may be called at most once; doing so lets us use transfer semantics rather than duplication or ref-count semantics to get the just-built output back to the caller, even though it means our behavior is not idempotent. Generated code is simplified as follows for events: |@@ -26,7 +26,7 @@ void qapi_event_send_acpi_device_ost(ACP | QDict *qmp; | Error *err = NULL; | QMPEventFuncEmit emit; |- QmpOutputVisitor *qov; |+ QObject *obj; | Visitor *v; | q_obj_ACPI_DEVICE_OST_arg param = { | info |@@ -39,8 +39,7 @@ void qapi_event_send_acpi_device_ost(ACP | | qmp = qmp_event_build_dict("ACPI_DEVICE_OST"); | |- qov = qmp_output_visitor_new(); |- v = qmp_output_get_visitor(qov); |+ v = qmp_output_visitor_new(&obj); | | visit_start_struct(v, "ACPI_DEVICE_OST", NULL, 0, &err); | if (err) { |@@ -55,7 +54,8 @@ void qapi_event_send_acpi_device_ost(ACP | goto out; | } | |- qdict_put_obj(qmp, "data", qmp_output_get_qobject(qov)); |+ visit_complete(v, &obj); |+ qdict_put_obj(qmp, "data", obj); | emit(QAPI_EVENT_ACPI_DEVICE_OST, qmp, &err); and for commands: | { | Error *err = NULL; |- QmpOutputVisitor *qov = qmp_output_visitor_new(); | Visitor *v; | |- v = qmp_output_get_visitor(qov); |+ v = qmp_output_visitor_new(ret_out); | visit_type_AddfdInfo(v, "unused", &ret_in, &err); |- if (err) { |- goto out; |+ if (!err) { |+ visit_complete(v, ret_out); | } |- *ret_out = qmp_output_get_qobject(qov); |- |-out: | error_propagate(errp, err); Signed-off-by: Eric Blake Message-Id: <1465490926-28625-13-git-send-email-eblake@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- block/qapi.c | 9 +++---- blockdev.c | 9 +++---- docs/qapi-code-gen.txt | 10 +++----- hmp.c | 11 ++++---- include/qapi/qmp-output-visitor.h | 11 +++++--- include/qapi/string-output-visitor.h | 13 +++++++--- include/qapi/visitor-impl.h | 3 +++ include/qapi/visitor.h | 50 +++++++++++++++++++++--------------- net/net.c | 11 ++++---- qapi/qapi-visit-core.c | 8 ++++++ qapi/qmp-output-visitor.c | 21 ++++++++------- qapi/string-output-visitor.c | 22 ++++++++-------- qemu-img.c | 30 +++++++++++----------- qom/object.c | 28 +++++++++----------- qom/qom-qobject.c | 10 ++++---- replay/replay-input.c | 6 ++--- scripts/qapi-commands.py | 10 +++----- scripts/qapi-event.py | 8 +++--- tests/check-qnull.c | 9 +++---- tests/test-qmp-output-visitor.c | 11 +++----- tests/test-string-output-visitor.c | 8 ++---- tests/test-visitor-serialization.c | 22 ++++++++-------- util/qemu-sockets.c | 6 ++--- 23 files changed, 166 insertions(+), 160 deletions(-) (limited to 'scripts/qapi-commands.py') diff --git a/block/qapi.c b/block/qapi.c index 41fe4f9fc2..6f947e3e66 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -690,16 +690,15 @@ static void dump_qdict(fprintf_function func_fprintf, void *f, int indentation, void bdrv_image_info_specific_dump(fprintf_function func_fprintf, void *f, ImageInfoSpecific *info_spec) { - QmpOutputVisitor *ov = qmp_output_visitor_new(); QObject *obj, *data; + Visitor *v = qmp_output_visitor_new(&obj); - visit_type_ImageInfoSpecific(qmp_output_get_visitor(ov), NULL, &info_spec, - &error_abort); - obj = qmp_output_get_qobject(ov); + visit_type_ImageInfoSpecific(v, NULL, &info_spec, &error_abort); + visit_complete(v, &obj); assert(qobject_type(obj) == QTYPE_QDICT); data = qdict_get(qobject_to_qdict(obj), "data"); dump_qobject(func_fprintf, f, 1, data); - visit_free(qmp_output_get_visitor(ov)); + visit_free(v); } void bdrv_image_info_dump(fprintf_function func_fprintf, void *f, diff --git a/blockdev.c b/blockdev.c index c05aaa6ea6..0f8065c4a5 100644 --- a/blockdev.c +++ b/blockdev.c @@ -3950,10 +3950,10 @@ out: void qmp_blockdev_add(BlockdevOptions *options, Error **errp) { - QmpOutputVisitor *ov = qmp_output_visitor_new(); BlockDriverState *bs; BlockBackend *blk = NULL; QObject *obj; + Visitor *v = qmp_output_visitor_new(&obj); QDict *qdict; Error *local_err = NULL; @@ -3972,14 +3972,13 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp) } } - visit_type_BlockdevOptions(qmp_output_get_visitor(ov), NULL, &options, - &local_err); + visit_type_BlockdevOptions(v, NULL, &options, &local_err); if (local_err) { error_propagate(errp, local_err); goto fail; } - obj = qmp_output_get_qobject(ov); + visit_complete(v, &obj); qdict = qobject_to_qdict(obj); qdict_flatten(qdict); @@ -4020,7 +4019,7 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp) } fail: - visit_free(qmp_output_get_visitor(ov)); + visit_free(v); } void qmp_x_blockdev_del(bool has_id, const char *id, diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index 7c30762720..48b0b31f2e 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -980,17 +980,13 @@ Example: static void qmp_marshal_output_UserDefOne(UserDefOne *ret_in, QObject **ret_out, Error **errp) { Error *err = NULL; - QmpOutputVisitor *qov = qmp_output_visitor_new(); Visitor *v; - v = qmp_output_get_visitor(qov); + v = qmp_output_visitor_new(ret_out); visit_type_UserDefOne(v, "unused", &ret_in, &err); - if (err) { - goto out; + if (!err) { + visit_complete(v, ret_out); } - *ret_out = qmp_output_get_qobject(qov); - - out: error_propagate(errp, err); visit_free(v); v = qapi_dealloc_visitor_new(); diff --git a/hmp.c b/hmp.c index 559cad1bb5..0cf5baa069 100644 --- a/hmp.c +++ b/hmp.c @@ -1983,15 +1983,14 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict) Error *err = NULL; MemdevList *memdev_list = qmp_query_memdev(&err); MemdevList *m = memdev_list; - StringOutputVisitor *ov; + Visitor *v; char *str; int i = 0; while (m) { - ov = string_output_visitor_new(false); - visit_type_uint16List(string_output_get_visitor(ov), NULL, - &m->value->host_nodes, NULL); + v = string_output_visitor_new(false, &str); + visit_type_uint16List(v, NULL, &m->value->host_nodes, NULL); monitor_printf(mon, "memory backend: %d\n", i); monitor_printf(mon, " size: %" PRId64 "\n", m->value->size); monitor_printf(mon, " merge: %s\n", @@ -2002,11 +2001,11 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict) m->value->prealloc ? "true" : "false"); monitor_printf(mon, " policy: %s\n", HostMemPolicy_lookup[m->value->policy]); - str = string_output_get_string(ov); + visit_complete(v, &str); monitor_printf(mon, " host nodes: %s\n", str); g_free(str); - visit_free(string_output_get_visitor(ov)); + visit_free(v); m = m->next; i++; } diff --git a/include/qapi/qmp-output-visitor.h b/include/qapi/qmp-output-visitor.h index 29c9a2e3cd..040fdda142 100644 --- a/include/qapi/qmp-output-visitor.h +++ b/include/qapi/qmp-output-visitor.h @@ -19,9 +19,12 @@ typedef struct QmpOutputVisitor QmpOutputVisitor; -QmpOutputVisitor *qmp_output_visitor_new(void); - -QObject *qmp_output_get_qobject(QmpOutputVisitor *v); -Visitor *qmp_output_get_visitor(QmpOutputVisitor *v); +/* + * Create a new QMP output visitor. + * + * If everything else succeeds, pass @result to visit_complete() to + * collect the result of the visit. + */ +Visitor *qmp_output_visitor_new(QObject **result); #endif diff --git a/include/qapi/string-output-visitor.h b/include/qapi/string-output-visitor.h index 03e377e50b..268dfe9986 100644 --- a/include/qapi/string-output-visitor.h +++ b/include/qapi/string-output-visitor.h @@ -18,13 +18,18 @@ typedef struct StringOutputVisitor StringOutputVisitor; /* + * Create a new string output visitor. + * + * Using @human creates output that is a bit easier for humans to read + * (for example, showing integer values in both decimal and hex). + * + * If everything else succeeds, pass @result to visit_complete() to + * collect the result of the visit. + * * The string output visitor does not implement support for visiting * QAPI structs, alternates, null, or arbitrary QTypes. It also * requires a non-null list argument to visit_start_list(). */ -StringOutputVisitor *string_output_visitor_new(bool human); - -char *string_output_get_string(StringOutputVisitor *v); -Visitor *string_output_get_visitor(StringOutputVisitor *v); +Visitor *string_output_visitor_new(bool human, char **result); #endif diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h index 525b06872b..16e0b86d57 100644 --- a/include/qapi/visitor-impl.h +++ b/include/qapi/visitor-impl.h @@ -105,6 +105,9 @@ struct Visitor /* Must be set */ VisitorType type; + /* Must be set for output visitors, optional otherwise. */ + void (*complete)(Visitor *v, void *opaque); + /* Must be set */ void (*free)(Visitor *v); }; diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index 2ded852307..00a60eaaab 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -39,21 +39,15 @@ * * All of the visitors are created via: * - * Type *subtype_visitor_new(parameters...); - * - * where Type is either directly 'Visitor *', or is a subtype that can - * be trivially upcast to Visitor * via another function: - * - * Visitor *subtype_get_visitor(SubtypeVisitor *); + * Visitor *subtype_visitor_new(parameters...); * * A visitor should be used for exactly one top-level visit_type_FOO() - * or virtual walk, then passed to visit_free() to clean up resources. + * or virtual walk; if that is successful, the caller can optionally + * call visit_complete() (for now, useful only for output visits, but + * safe to call on all visits). Then, regardless of success or + * failure, the user should call visit_free() to clean up resources. * It is okay to free the visitor without completing the visit, if - * some other error is detected in the meantime. Output visitors - * provide an additional function, for collecting the final results of - * a successful visit: string_output_get_string() and - * qmp_output_get_qobject(); this collection function should not be - * called if any errors were reported during the visit. + * some other error is detected in the meantime. * * All QAPI types have a corresponding function with a signature * roughly compatible with this: @@ -123,14 +117,14 @@ * Error *err = NULL; * Visitor *v; * - * v = ...obtain input visitor... + * v = FOO_visitor_new(...); * visit_type_Foo(v, NULL, &f, &err); * if (err) { * ...handle error... * } else { * ...use f... * } - * ...clean up v... + * visit_free(v); * qapi_free_Foo(f); * * @@ -140,7 +134,7 @@ * Error *err = NULL; * Visitor *v; * - * v = ...obtain input visitor... + * v = FOO_visitor_new(...); * visit_type_FooList(v, NULL, &l, &err); * if (err) { * ...handle error... @@ -149,7 +143,7 @@ * ...use l->value... * } * } - * ...clean up v... + * visit_free(v); * qapi_free_FooList(l); * * @@ -159,13 +153,17 @@ * Foo *f = ...obtain populated object... * Error *err = NULL; * Visitor *v; + * Type *result; * - * v = ...obtain output visitor... + * v = FOO_visitor_new(..., &result); * visit_type_Foo(v, NULL, &f, &err); * if (err) { * ...handle error... + * } else { + * visit_complete(v, &result); + * ...use result... * } - * ...clean up v... + * visit_free(v); * * * When visiting a real QAPI struct, this file provides several @@ -191,7 +189,7 @@ * Error *err = NULL; * int value; * - * v = ...obtain visitor... + * v = FOO_visitor_new(...); * visit_start_struct(v, NULL, NULL, 0, &err); * if (err) { * goto out; @@ -219,7 +217,7 @@ * visit_end_struct(v, NULL); * out: * error_propagate(errp, err); - * ...clean up v... + * visit_free(v); * */ @@ -242,6 +240,18 @@ typedef struct GenericAlternate { /*** Visitor cleanup ***/ +/* + * Complete the visit, collecting any output. + * + * May only be called only once after a successful top-level + * visit_type_FOO() or visit_end_ITEM(), and marks the end of the + * visit. The @opaque pointer should match the output parameter + * passed to the subtype_visitor_new() used to create an output + * visitor, or NULL for any other visitor. Needed for output + * visitors, but may also be called with other visitors. + */ +void visit_complete(Visitor *v, void *opaque); + /* * Free @v and any resources it has tied up. * diff --git a/net/net.c b/net/net.c index 336469f9cc..019aaad0fc 100644 --- a/net/net.c +++ b/net/net.c @@ -1198,7 +1198,7 @@ static void netfilter_print_info(Monitor *mon, NetFilterState *nf) char *str; ObjectProperty *prop; ObjectPropertyIterator iter; - StringOutputVisitor *ov; + Visitor *v; /* generate info str */ object_property_iter_init(&iter, OBJECT(nf)); @@ -1206,11 +1206,10 @@ static void netfilter_print_info(Monitor *mon, NetFilterState *nf) if (!strcmp(prop->name, "type")) { continue; } - ov = string_output_visitor_new(false); - object_property_get(OBJECT(nf), string_output_get_visitor(ov), - prop->name, NULL); - str = string_output_get_string(ov); - visit_free(string_output_get_visitor(ov)); + v = string_output_visitor_new(false, &str); + object_property_get(OBJECT(nf), v, prop->name, NULL); + visit_complete(v, &str); + visit_free(v); monitor_printf(mon, ",%s=%s", prop->name, str); g_free(str); } diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c index 5f68c251d2..eb7dd7253c 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qapi-visit-core.c @@ -20,6 +20,14 @@ #include "qapi/visitor.h" #include "qapi/visitor-impl.h" +void visit_complete(Visitor *v, void *opaque) +{ + assert(v->type != VISITOR_OUTPUT || v->complete); + if (v->complete) { + v->complete(v, opaque); + } +} + void visit_free(Visitor *v) { if (v) { diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c index 3d12623cf9..0452056d42 100644 --- a/qapi/qmp-output-visitor.c +++ b/qapi/qmp-output-visitor.c @@ -33,6 +33,7 @@ struct QmpOutputVisitor Visitor visitor; QStack stack; /* Stack of containers that haven't yet been finished */ QObject *root; /* Root of the output visit */ + QObject **result; /* User's storage location for result */ }; #define qmp_output_add(qov, name, value) \ @@ -200,18 +201,17 @@ static void qmp_output_type_null(Visitor *v, const char *name, Error **errp) /* Finish building, and return the root object. * The root object is never null. The caller becomes the object's * owner, and should use qobject_decref() when done with it. */ -QObject *qmp_output_get_qobject(QmpOutputVisitor *qov) +static void qmp_output_complete(Visitor *v, void *opaque) { + QmpOutputVisitor *qov = to_qov(v); + /* A visit must have occurred, with each start paired with end. */ assert(qov->root && QTAILQ_EMPTY(&qov->stack)); + assert(opaque == qov->result); qobject_incref(qov->root); - return qov->root; -} - -Visitor *qmp_output_get_visitor(QmpOutputVisitor *v) -{ - return &v->visitor; + *qov->result = qov->root; + qov->result = NULL; } static void qmp_output_free(Visitor *v) @@ -228,7 +228,7 @@ static void qmp_output_free(Visitor *v) g_free(qov); } -QmpOutputVisitor *qmp_output_visitor_new(void) +Visitor *qmp_output_visitor_new(QObject **result) { QmpOutputVisitor *v; @@ -247,9 +247,12 @@ QmpOutputVisitor *qmp_output_visitor_new(void) v->visitor.type_number = qmp_output_type_number; v->visitor.type_any = qmp_output_type_any; v->visitor.type_null = qmp_output_type_null; + v->visitor.complete = qmp_output_complete; v->visitor.free = qmp_output_free; QTAILQ_INIT(&v->stack); + *result = NULL; + v->result = result; - return v; + return &v->visitor; } diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c index 78aab876e3..94ac8211d1 100644 --- a/qapi/string-output-visitor.c +++ b/qapi/string-output-visitor.c @@ -58,6 +58,7 @@ struct StringOutputVisitor Visitor visitor; bool human; GString *string; + char **result; ListMode list_mode; union { int64_t s; @@ -305,16 +306,13 @@ static void end_list(Visitor *v, void **obj) sov->list_mode = LM_NONE; } -char *string_output_get_string(StringOutputVisitor *sov) +static void string_output_complete(Visitor *v, void *opaque) { - char *string = g_string_free(sov->string, false); - sov->string = NULL; - return string; -} + StringOutputVisitor *sov = to_sov(v); -Visitor *string_output_get_visitor(StringOutputVisitor *sov) -{ - return &sov->visitor; + assert(opaque == sov->result); + *sov->result = g_string_free(sov->string, false); + sov->string = NULL; } static void free_range(void *range, void *dummy) @@ -335,7 +333,7 @@ static void string_output_free(Visitor *v) g_free(sov); } -StringOutputVisitor *string_output_visitor_new(bool human) +Visitor *string_output_visitor_new(bool human, char **result) { StringOutputVisitor *v; @@ -343,6 +341,9 @@ StringOutputVisitor *string_output_visitor_new(bool human) v->string = g_string_new(NULL); v->human = human; + v->result = result; + *result = NULL; + v->visitor.type = VISITOR_OUTPUT; v->visitor.type_int64 = print_type_int64; v->visitor.type_uint64 = print_type_uint64; @@ -353,7 +354,8 @@ StringOutputVisitor *string_output_visitor_new(bool human) v->visitor.start_list = start_list; v->visitor.next_list = next_list; v->visitor.end_list = end_list; + v->visitor.complete = string_output_complete; v->visitor.free = string_output_free; - return v; + return &v->visitor; } diff --git a/qemu-img.c b/qemu-img.c index 728f471801..4dfb27df37 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -491,16 +491,16 @@ fail: static void dump_json_image_check(ImageCheck *check, bool quiet) { QString *str; - QmpOutputVisitor *ov = qmp_output_visitor_new(); QObject *obj; - visit_type_ImageCheck(qmp_output_get_visitor(ov), NULL, &check, - &error_abort); - obj = qmp_output_get_qobject(ov); + Visitor *v = qmp_output_visitor_new(&obj); + + visit_type_ImageCheck(v, NULL, &check, &error_abort); + visit_complete(v, &obj); str = qobject_to_json_pretty(obj); assert(str != NULL); qprintf(quiet, "%s\n", qstring_get_str(str)); qobject_decref(obj); - visit_free(qmp_output_get_visitor(ov)); + visit_free(v); QDECREF(str); } @@ -2181,32 +2181,32 @@ static void dump_snapshots(BlockDriverState *bs) static void dump_json_image_info_list(ImageInfoList *list) { QString *str; - QmpOutputVisitor *ov = qmp_output_visitor_new(); QObject *obj; - visit_type_ImageInfoList(qmp_output_get_visitor(ov), NULL, &list, - &error_abort); - obj = qmp_output_get_qobject(ov); + Visitor *v = qmp_output_visitor_new(&obj); + + visit_type_ImageInfoList(v, NULL, &list, &error_abort); + visit_complete(v, &obj); str = qobject_to_json_pretty(obj); assert(str != NULL); printf("%s\n", qstring_get_str(str)); qobject_decref(obj); - visit_free(qmp_output_get_visitor(ov)); + visit_free(v); QDECREF(str); } static void dump_json_image_info(ImageInfo *info) { QString *str; - QmpOutputVisitor *ov = qmp_output_visitor_new(); QObject *obj; - visit_type_ImageInfo(qmp_output_get_visitor(ov), NULL, &info, - &error_abort); - obj = qmp_output_get_qobject(ov); + Visitor *v = qmp_output_visitor_new(&obj); + + visit_type_ImageInfo(v, NULL, &info, &error_abort); + visit_complete(v, &obj); str = qobject_to_json_pretty(obj); assert(str != NULL); printf("%s\n", qstring_get_str(str)); qobject_decref(obj); - visit_free(qmp_output_get_visitor(ov)); + visit_free(v); QDECREF(str); } diff --git a/qom/object.c b/qom/object.c index 2407b667d9..8166b7dace 100644 --- a/qom/object.c +++ b/qom/object.c @@ -1221,7 +1221,6 @@ int object_property_get_enum(Object *obj, const char *name, const char *typename, Error **errp) { Error *err = NULL; - StringOutputVisitor *sov; Visitor *v; char *str; int ret; @@ -1241,15 +1240,14 @@ int object_property_get_enum(Object *obj, const char *name, enumprop = prop->opaque; - sov = string_output_visitor_new(false); - v = string_output_get_visitor(sov); + v = string_output_visitor_new(false, &str); object_property_get(obj, v, name, &err); if (err) { error_propagate(errp, err); visit_free(v); return 0; } - str = string_output_get_string(sov); + visit_complete(v, &str); visit_free(v); v = string_input_visitor_new(str); visit_type_enum(v, name, &ret, enumprop->strings, errp); @@ -1264,25 +1262,23 @@ void object_property_get_uint16List(Object *obj, const char *name, uint16List **list, Error **errp) { Error *err = NULL; - StringOutputVisitor *ov; Visitor *v; char *str; - ov = string_output_visitor_new(false); - object_property_get(obj, string_output_get_visitor(ov), - name, &err); + v = string_output_visitor_new(false, &str); + object_property_get(obj, v, name, &err); if (err) { error_propagate(errp, err); goto out; } - str = string_output_get_string(ov); + visit_complete(v, &str); + visit_free(v); v = string_input_visitor_new(str); visit_type_uint16List(v, NULL, list, errp); g_free(str); - visit_free(v); out: - visit_free(string_output_get_visitor(ov)); + visit_free(v); } void object_property_parse(Object *obj, const char *string, @@ -1296,21 +1292,21 @@ void object_property_parse(Object *obj, const char *string, char *object_property_print(Object *obj, const char *name, bool human, Error **errp) { - StringOutputVisitor *sov; + Visitor *v; char *string = NULL; Error *local_err = NULL; - sov = string_output_visitor_new(human); - object_property_get(obj, string_output_get_visitor(sov), name, &local_err); + v = string_output_visitor_new(human, &string); + object_property_get(obj, v, name, &local_err); if (local_err) { error_propagate(errp, local_err); goto out; } - string = string_output_get_string(sov); + visit_complete(v, &string); out: - visit_free(string_output_get_visitor(sov)); + visit_free(v); return string; } diff --git a/qom/qom-qobject.c b/qom/qom-qobject.c index 6714565f13..c225abcbad 100644 --- a/qom/qom-qobject.c +++ b/qom/qom-qobject.c @@ -33,14 +33,14 @@ QObject *object_property_get_qobject(Object *obj, const char *name, { QObject *ret = NULL; Error *local_err = NULL; - QmpOutputVisitor *qov; + Visitor *v; - qov = qmp_output_visitor_new(); - object_property_get(obj, qmp_output_get_visitor(qov), name, &local_err); + v = qmp_output_visitor_new(&ret); + object_property_get(obj, v, name, &local_err); if (!local_err) { - ret = qmp_output_get_qobject(qov); + visit_complete(v, &ret); } error_propagate(errp, local_err); - visit_free(qmp_output_get_visitor(qov)); + visit_free(v); return ret; } diff --git a/replay/replay-input.c b/replay/replay-input.c index 9cbb4c19e9..296399c877 100644 --- a/replay/replay-input.c +++ b/replay/replay-input.c @@ -22,15 +22,13 @@ static InputEvent *qapi_clone_InputEvent(InputEvent *src) { - QmpOutputVisitor *qov; Visitor *ov, *iv; QObject *obj; InputEvent *dst = NULL; - qov = qmp_output_visitor_new(); - ov = qmp_output_get_visitor(qov); + ov = qmp_output_visitor_new(&obj); visit_type_InputEvent(ov, NULL, &src, &error_abort); - obj = qmp_output_get_qobject(qov); + visit_complete(ov, &obj); visit_free(ov); if (!obj) { return NULL; diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 49d4ce2590..34b6a3a07f 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -61,17 +61,13 @@ 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 *err = NULL; - QmpOutputVisitor *qov = qmp_output_visitor_new(); Visitor *v; - v = qmp_output_get_visitor(qov); + v = qmp_output_visitor_new(ret_out); visit_type_%(c_name)s(v, "unused", &ret_in, &err); - if (err) { - goto out; + if (!err) { + visit_complete(v, ret_out); } - *ret_out = qmp_output_get_qobject(qov); - -out: error_propagate(errp, err); visit_free(v); v = qapi_dealloc_visitor_new(); diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py index 909e8d6b8f..9c88627c9f 100644 --- a/scripts/qapi-event.py +++ b/scripts/qapi-event.py @@ -71,7 +71,7 @@ def gen_event_send(name, arg_type): if arg_type and arg_type.members: ret += mcgen(''' - QmpOutputVisitor *qov; + QObject *obj; Visitor *v; ''') ret += gen_param_var(arg_type) @@ -90,8 +90,7 @@ def gen_event_send(name, arg_type): if arg_type and arg_type.members: ret += mcgen(''' - qov = qmp_output_visitor_new(); - v = qmp_output_get_visitor(qov); + v = qmp_output_visitor_new(&obj); visit_start_struct(v, "%(name)s", NULL, 0, &err); if (err) { @@ -106,7 +105,8 @@ def gen_event_send(name, arg_type): goto out; } - qdict_put_obj(qmp, "data", qmp_output_get_qobject(qov)); + visit_complete(v, &obj); + qdict_put_obj(qmp, "data", obj); ''', name=name, c_name=arg_type.c_name()) diff --git a/tests/check-qnull.c b/tests/check-qnull.c index 6310bf7d41..dc906b116e 100644 --- a/tests/check-qnull.c +++ b/tests/check-qnull.c @@ -37,7 +37,6 @@ static void qnull_ref_test(void) static void qnull_visit_test(void) { QObject *obj; - QmpOutputVisitor *qov; Visitor *v; /* @@ -53,12 +52,12 @@ static void qnull_visit_test(void) visit_type_null(v, NULL, &error_abort); visit_free(v); - qov = qmp_output_visitor_new(); - visit_type_null(qmp_output_get_visitor(qov), NULL, &error_abort); - obj = qmp_output_get_qobject(qov); + v = qmp_output_visitor_new(&obj); + visit_type_null(v, NULL, &error_abort); + visit_complete(v, &obj); g_assert(obj == &qnull_); qobject_decref(obj); - visit_free(qmp_output_get_visitor(qov)); + visit_free(v); g_assert(qnull_.refcnt == 1); } diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c index af24b1087f..513d71f0d1 100644 --- a/tests/test-qmp-output-visitor.c +++ b/tests/test-qmp-output-visitor.c @@ -21,7 +21,6 @@ #include "qapi/qmp/qjson.h" typedef struct TestOutputVisitorData { - QmpOutputVisitor *qov; Visitor *ov; QObject *obj; } TestOutputVisitorData; @@ -29,18 +28,14 @@ typedef struct TestOutputVisitorData { static void visitor_output_setup(TestOutputVisitorData *data, const void *unused) { - data->qov = qmp_output_visitor_new(); - g_assert(data->qov != NULL); - - data->ov = qmp_output_get_visitor(data->qov); - g_assert(data->ov != NULL); + data->ov = qmp_output_visitor_new(&data->obj); + g_assert(data->ov); } static void visitor_output_teardown(TestOutputVisitorData *data, const void *unused) { visit_free(data->ov); - data->qov = NULL; data->ov = NULL; qobject_decref(data->obj); data->obj = NULL; @@ -48,7 +43,7 @@ static void visitor_output_teardown(TestOutputVisitorData *data, static QObject *visitor_get(TestOutputVisitorData *data) { - data->obj = qmp_output_get_qobject(data->qov); + visit_complete(data->ov, &data->obj); g_assert(data->obj); return data->obj; } diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c index e17035be5e..444844a15a 100644 --- a/tests/test-string-output-visitor.c +++ b/tests/test-string-output-visitor.c @@ -20,7 +20,6 @@ #include "qapi/qmp/types.h" typedef struct TestOutputVisitorData { - StringOutputVisitor *sov; Visitor *ov; char *str; bool human; @@ -30,9 +29,7 @@ static void visitor_output_setup_internal(TestOutputVisitorData *data, bool human) { data->human = human; - data->sov = string_output_visitor_new(human); - g_assert(data->sov); - data->ov = string_output_get_visitor(data->sov); + data->ov = string_output_visitor_new(human, &data->str); g_assert(data->ov); } @@ -52,7 +49,6 @@ static void visitor_output_teardown(TestOutputVisitorData *data, const void *unused) { visit_free(data->ov); - data->sov = NULL; data->ov = NULL; g_free(data->str); data->str = NULL; @@ -60,7 +56,7 @@ static void visitor_output_teardown(TestOutputVisitorData *data, static char *visitor_get(TestOutputVisitorData *data) { - data->str = string_output_get_string(data->sov); + visit_complete(data->ov, &data->str); g_assert(data->str); return data->str; } diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c index 377ee3e788..dba4670762 100644 --- a/tests/test-visitor-serialization.c +++ b/tests/test-visitor-serialization.c @@ -1012,7 +1012,8 @@ static PrimitiveType pt_values[] = { /* visitor-specific op implementations */ typedef struct QmpSerializeData { - QmpOutputVisitor *qov; + Visitor *qov; + QObject *obj; Visitor *qiv; } QmpSerializeData; @@ -1021,8 +1022,8 @@ static void qmp_serialize(void *native_in, void **datap, { QmpSerializeData *d = g_malloc0(sizeof(*d)); - d->qov = qmp_output_visitor_new(); - visit(qmp_output_get_visitor(d->qov), &native_in, errp); + d->qov = qmp_output_visitor_new(&d->obj); + visit(d->qov, &native_in, errp); *datap = d; } @@ -1033,7 +1034,8 @@ static void qmp_deserialize(void **native_out, void *datap, QString *output_json; QObject *obj_orig, *obj; - obj_orig = qmp_output_get_qobject(d->qov); + visit_complete(d->qov, &d->obj); + obj_orig = d->obj; output_json = qobject_to_json(obj_orig); obj = qobject_from_json(qstring_get_str(output_json)); @@ -1047,7 +1049,7 @@ static void qmp_deserialize(void **native_out, void *datap, static void qmp_cleanup(void *datap) { QmpSerializeData *d = datap; - visit_free(qmp_output_get_visitor(d->qov)); + visit_free(d->qov); visit_free(d->qiv); g_free(d); @@ -1055,7 +1057,7 @@ static void qmp_cleanup(void *datap) typedef struct StringSerializeData { char *string; - StringOutputVisitor *sov; + Visitor *sov; Visitor *siv; } StringSerializeData; @@ -1064,8 +1066,8 @@ static void string_serialize(void *native_in, void **datap, { StringSerializeData *d = g_malloc0(sizeof(*d)); - d->sov = string_output_visitor_new(false); - visit(string_output_get_visitor(d->sov), &native_in, errp); + d->sov = string_output_visitor_new(false, &d->string); + visit(d->sov, &native_in, errp); *datap = d; } @@ -1074,7 +1076,7 @@ static void string_deserialize(void **native_out, void *datap, { StringSerializeData *d = datap; - d->string = string_output_get_string(d->sov); + visit_complete(d->sov, &d->string); d->siv = string_input_visitor_new(d->string); visit(d->siv, native_out, errp); } @@ -1083,7 +1085,7 @@ static void string_cleanup(void *datap) { StringSerializeData *d = datap; - visit_free(string_output_get_visitor(d->sov)); + visit_free(d->sov); visit_free(d->siv); g_free(d->string); g_free(d); diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c index 01fc15481a..a0ca6d4a7e 100644 --- a/util/qemu-sockets.c +++ b/util/qemu-sockets.c @@ -1147,16 +1147,14 @@ SocketAddress *socket_remote_address(int fd, Error **errp) void qapi_copy_SocketAddress(SocketAddress **p_dest, SocketAddress *src) { - QmpOutputVisitor *qov; Visitor *ov, *iv; QObject *obj; *p_dest = NULL; - qov = qmp_output_visitor_new(); - ov = qmp_output_get_visitor(qov); + ov = qmp_output_visitor_new(&obj); visit_type_SocketAddress(ov, NULL, &src, &error_abort); - obj = qmp_output_get_qobject(qov); + visit_complete(ov, &obj); visit_free(ov); if (!obj) { return; -- cgit 1.4.1