[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [PATCH v3 18/18] qapi: Add parameter to visi
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [PATCH v3 18/18] qapi: Add parameter to visit_end_* |
Date: |
Mon, 02 May 2016 20:20:37 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> 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.
>
> The clone visitor also gets a minor simplification of not having
> to track quite as much depth.
Do this before adding the clone visitor, to reduce churn?
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> v3: new patch
> ---
> include/qapi/visitor.h | 32 ++++++++++++++++-----------
> include/qapi/visitor-impl.h | 6 ++---
> scripts/qapi-commands.py | 4 ++--
> scripts/qapi-event.py | 2 +-
> scripts/qapi-visit.py | 8 +++----
> qapi/qapi-visit-core.c | 12 +++++-----
> block/crypto.c | 4 ++--
> hw/ppc/spapr_drc.c | 4 ++--
> hw/virtio/virtio-balloon.c | 4 ++--
> migration/savevm.c | 10 ++++-----
> migration/vmstate.c | 10 ++++-----
> qapi/json-output-visitor.c | 4 ++--
> qapi/opts-visitor.c | 4 ++--
> qapi/qapi-clone-visitor.c | 11 +++++-----
> qapi/qapi-dealloc-visitor.c | 47
> +++-------------------------------------
> qapi/qmp-input-visitor.c | 2 +-
> qapi/qmp-output-visitor.c | 4 ++--
> qapi/string-input-visitor.c | 2 +-
> qapi/string-output-visitor.c | 2 +-
> qom/object.c | 2 +-
> qom/object_interfaces.c | 4 ++--
> tests/test-json-output-visitor.c | 4 ++--
> tests/test-qmp-input-visitor.c | 2 +-
> tests/test-qmp-output-visitor.c | 2 +-
> docs/qapi-code-gen.txt | 4 ++--
> 25 files changed, 77 insertions(+), 113 deletions(-)
>
> diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
> index 4c122cc..34fdd44 100644
> --- a/include/qapi/visitor.h
> +++ b/include/qapi/visitor.h
> @@ -196,12 +196,12 @@
> * goto outlist;
> * }
> * outlist:
> - * visit_end_list(v);
> + * visit_end_list(v, NULL);
> * if (!err) {
> * visit_check_struct(v, &err);
> * }
> * outobj:
> - * visit_end_struct(v);
> + * visit_end_struct(v, NULL);
> * error_propagate(errp, err);
> * ...clean up v...
> * </example>
> @@ -244,8 +244,8 @@ typedef struct GenericAlternate {
> * After visit_start_struct() succeeds, the caller may visit its
> * members one after the other, passing the member's name and address
> * within the struct. Finally, visit_end_struct() needs to be called
> - * to clean up, even if intermediate visits fail. See the examples
> - * above.
> + * with the same @obj to clean up, even if intermediate visits fail.
> + * See the examples above.
The examples don't show "the same @obj" very well, because it's NULL.
It'll do; the comment containing them is large enough as it is.
> *
> * FIXME Should this be named visit_start_object, since it is also
> * used for QAPI unions, and maps to JSON objects?
> @@ -269,12 +269,14 @@ void visit_check_struct(Visitor *v, Error **errp);
> /*
> * Complete an object visit started earlier.
> *
> + * @obj must match what was passed to the paired visit_start_struct().
> + *
> * 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.
> */
> -void visit_end_struct(Visitor *v);
> +void visit_end_struct(Visitor *v, void **obj);
>
>
> /*** Visiting lists ***/
> @@ -301,8 +303,9 @@ void visit_end_struct(Visitor *v);
> * visit (where @obj is NULL) uses other means. For each list
> * element, call the appropriate visit_type_FOO() with name set to
> * NULL and obj set to the address of the value member of the list
> - * element. Finally, visit_end_list() needs to be called to clean up,
> - * even if intermediate visits fail. See the examples above.
> + * element. Finally, visit_end_list() needs to be called with the
> + * same @list to clean up, even if intermediate visits fail. See the
> + * examples above.
> */
> void visit_start_list(Visitor *v, const char *name, GenericList **list,
> size_t size, Error **errp);
> @@ -326,12 +329,14 @@ GenericList *visit_next_list(Visitor *v, GenericList
> *tail, size_t size);
> /*
> * Complete a list visit started earlier.
> *
> + * @list must match what was passed to the paired visit_start_list().
> + *
> * 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.
> */
> -void visit_end_list(Visitor *v);
> +void visit_end_list(Visitor *v, void **list);
>
>
> /*** Visiting alternates ***/
> @@ -349,8 +354,9 @@ void visit_end_list(Visitor *v);
> *
> * If @promote_int, treat integers as QTYPE_FLOAT.
> *
> - * If successful, this must be paired with visit_end_alternate() to
> - * clean up, even if visiting the contents of the alternate fails.
> + * If successful, this must be paired with visit_end_alternate() with
> + * the same @obj to clean up, even if visiting the contents of the
> + * alternate fails.
> */
> void visit_start_alternate(Visitor *v, const char *name,
> GenericAlternate **obj, size_t size,
> @@ -359,15 +365,15 @@ void visit_start_alternate(Visitor *v, const char *name,
> /*
> * Finish visiting an alternate type.
> *
> + * @obj must match what was passed to the paired visit_start_alternate().
> + *
> * 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.
> *
> - * TODO: Should all the visit_end_* interfaces take obj parameter, so
> - * that dealloc visitor need not track what was passed in visit_start?
> */
> -void visit_end_alternate(Visitor *v);
> +void visit_end_alternate(Visitor *v, void **obj);
>
>
> /*** Other helpers ***/
> diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
> index a5a2dd0..fdc1f71 100644
> --- a/include/qapi/visitor-impl.h
> +++ b/include/qapi/visitor-impl.h
> @@ -48,7 +48,7 @@ struct Visitor
> void (*check_struct)(Visitor *v, Error **errp);
>
> /* Must be set to visit structs */
> - void (*end_struct)(Visitor *v);
> + void (*end_struct)(Visitor *v, void **obj);
>
> /* Must be set; implementations may require @list to be non-null,
> * but must document it. */
> @@ -59,7 +59,7 @@ struct Visitor
> GenericList *(*next_list)(Visitor *v, GenericList *tail, size_t size);
>
> /* Must be set */
> - void (*end_list)(Visitor *v);
> + void (*end_list)(Visitor *v, void **list);
Sure you want void ** and not GenericList **?
>
> /* Must be set by input and dealloc visitors to visit alternates;
> * optional for output visitors. */
> @@ -68,7 +68,7 @@ struct Visitor
> bool promote_int, Error **errp);
>
> /* Optional, needed for dealloc visitor */
> - void (*end_alternate)(Visitor *v);
> + void (*end_alternate)(Visitor *v, void **obj);
Sure you want void ** and not GenericAlternate **?
>
> /* Must be set */
> void (*type_int64)(Visitor *v, const char *name, int64_t *obj,
> diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
> index 8c6acb3..971dc4e 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())
> diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
> index 21fb167..084c40a 100644
> --- a/scripts/qapi-event.py
> +++ b/scripts/qapi-event.py
> @@ -101,7 +101,7 @@ def gen_event_send(name, arg_type):
> if (!err) {
> visit_check_struct(v, &err);
> }
> - visit_end_struct(v);
> + visit_end_struct(v, NULL);
> if (err) {
> goto out;
> }
> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
> index 70ea8ca..7b85d2b 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -129,7 +129,7 @@ void visit_type_%(c_name)s(Visitor *v, const char *name,
> %(c_name)s **obj, Error
> }
> }
>
> - visit_end_list(v);
> + visit_end_list(v, (void **)obj);
> if (err && visit_is_input(v)) {
> qapi_free_%(c_name)s(*obj);
> *obj = NULL;
> @@ -191,7 +191,7 @@ void visit_type_%(c_name)s(Visitor *v, const char *name,
> %(c_name)s **obj, Error
> if (!err) {
> visit_check_struct(v, &err);
> }
> - visit_end_struct(v);
> + visit_end_struct(v, NULL);
> ''',
> c_type=var.type.c_name(),
> c_name=c_name(var.name))
> @@ -210,7 +210,7 @@ void visit_type_%(c_name)s(Visitor *v, const char *name,
> %(c_name)s **obj, Error
> error_setg(&err, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
> "%(name)s");
> }
> - visit_end_alternate(v);
> + visit_end_alternate(v, (void **)obj);
> if (err && visit_is_input(v)) {
> qapi_free_%(c_name)s(*obj);
> *obj = NULL;
> @@ -244,7 +244,7 @@ void visit_type_%(c_name)s(Visitor *v, const char *name,
> %(c_name)s **obj, Error
> }
> visit_check_struct(v, &err);
> out_obj:
> - visit_end_struct(v);
> + visit_end_struct(v, (void **)obj);
> if (err && visit_is_input(v)) {
> qapi_free_%(c_name)s(*obj);
> *obj = NULL;
> diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> index 838e5d5..f339dc2 100644
> --- a/qapi/qapi-visit-core.c
> +++ b/qapi/qapi-visit-core.c
> @@ -43,9 +43,9 @@ void visit_check_struct(Visitor *v, Error **errp)
> }
> }
>
> -void visit_end_struct(Visitor *v)
> +void visit_end_struct(Visitor *v, void **obj)
> {
> - v->end_struct(v);
> + v->end_struct(v, obj);
> }
>
> void visit_start_list(Visitor *v, const char *name, GenericList **list,
> @@ -67,9 +67,9 @@ GenericList *visit_next_list(Visitor *v, GenericList *tail,
> size_t size)
> return v->next_list(v, tail, size);
> }
>
> -void visit_end_list(Visitor *v)
> +void visit_end_list(Visitor *v, void **obj)
> {
> - v->end_list(v);
> + v->end_list(v, obj);
> }
>
> void visit_start_alternate(Visitor *v, const char *name,
> @@ -89,10 +89,10 @@ void visit_start_alternate(Visitor *v, const char *name,
> error_propagate(errp, err);
> }
>
> -void visit_end_alternate(Visitor *v)
> +void visit_end_alternate(Visitor *v, void **obj)
> {
> if (v->end_alternate) {
> - v->end_alternate(v);
> + v->end_alternate(v, obj);
> }
> }
>
> diff --git a/block/crypto.c b/block/crypto.c
> index 2424a4c..d2e710a 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -222,7 +222,7 @@ block_crypto_open_opts_init(QCryptoBlockFormat format,
> visit_check_struct(opts_get_visitor(ov), &local_err);
> }
>
> - visit_end_struct(opts_get_visitor(ov));
> + visit_end_struct(opts_get_visitor(ov), NULL);
>
> out:
> if (local_err) {
> @@ -269,7 +269,7 @@ block_crypto_create_opts_init(QCryptoBlockFormat format,
> visit_check_struct(opts_get_visitor(ov), &local_err);
> }
>
> - visit_end_struct(opts_get_visitor(ov));
> + visit_end_struct(opts_get_visitor(ov), NULL);
>
> out:
> if (local_err) {
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 94c875d..c1da698 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -298,7 +298,7 @@ static void prop_get_fdt(Object *obj, Visitor *v, const
> char *name,
> /* shouldn't ever see an FDT_END_NODE before FDT_BEGIN_NODE */
> g_assert(fdt_depth > 0);
> visit_check_struct(v, &err);
> - visit_end_struct(v);
> + visit_end_struct(v, NULL);
> if (err) {
> error_propagate(errp, err);
> return;
> @@ -321,7 +321,7 @@ static void prop_get_fdt(Object *obj, Visitor *v, const
> char *name,
> return;
> }
> }
> - visit_end_list(v);
> + visit_end_list(v, NULL);
> break;
> }
> default:
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index 8c15e09..3c1d03d 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -143,13 +143,13 @@ static void balloon_stats_get_all(Object *obj, Visitor
> *v, const char *name,
> }
> visit_check_struct(v, &err);
> out_nested:
> - visit_end_struct(v);
> + visit_end_struct(v, NULL);
>
> if (!err) {
> visit_check_struct(v, &err);
> }
> out_end:
> - visit_end_struct(v);
> + visit_end_struct(v, NULL);
> out:
> error_propagate(errp, err);
> }
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 8cb3af9..ff6b192 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -665,8 +665,8 @@ static void vmstate_save_old_style(QEMUFile *f,
> SaveStateEntry *se,
> tmp = "buffer";
> visit_type_str(vmdesc, "type", (char **)&tmp, &error_abort);
> visit_check_struct(vmdesc, &error_abort);
> - visit_end_struct(vmdesc);
> - visit_end_list(vmdesc);
> + visit_end_struct(vmdesc, NULL);
> + visit_end_list(vmdesc, NULL);
> }
> }
>
> @@ -1112,7 +1112,7 @@ void qemu_savevm_state_complete_precopy(QEMUFile *f,
> bool iterable_only)
> save_section_footer(f, se);
>
> visit_check_struct(vmdesc, &error_abort);
> - visit_end_struct(vmdesc);
> + visit_end_struct(vmdesc, NULL);
> }
>
> if (!in_postcopy) {
> @@ -1120,9 +1120,9 @@ void qemu_savevm_state_complete_precopy(QEMUFile *f,
> bool iterable_only)
> qemu_put_byte(f, QEMU_VM_EOF);
> }
>
> - visit_end_list(vmdesc);
> + visit_end_list(vmdesc, NULL);
> visit_check_struct(vmdesc, &error_abort);
> - visit_end_struct(vmdesc);
> + visit_end_struct(vmdesc, NULL);
> vmdesc_str = json_output_get_string(vmdesc_jov);
> vmdesc_len = strlen(vmdesc_str);
>
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index e7f894c..8c789a3 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -284,13 +284,13 @@ static void vmsd_desc_field_end(const
> VMStateDescription *vmsd,
> if (field->flags & VMS_STRUCT) {
> /* We printed a struct in between, close its child object */
> visit_check_struct(vmdesc, &error_abort);
> - visit_end_struct(vmdesc);
> + visit_end_struct(vmdesc, NULL);
> }
>
> tmp = size;
> visit_type_int(vmdesc, "size", &tmp, &error_abort);
> visit_check_struct(vmdesc, &error_abort);
> - visit_end_struct(vmdesc);
> + visit_end_struct(vmdesc, NULL);
> }
>
>
> @@ -364,7 +364,7 @@ void vmstate_save_state(QEMUFile *f, const
> VMStateDescription *vmsd,
> }
>
> if (vmdesc) {
> - visit_end_list(vmdesc);
> + visit_end_list(vmdesc, NULL);
> }
>
> vmstate_subsection_save(f, vmsd, opaque, vmdesc);
> @@ -464,14 +464,14 @@ static void vmstate_subsection_save(QEMUFile *f, const
> VMStateDescription *vmsd,
>
> if (vmdesc) {
> visit_check_struct(vmdesc, &error_abort);
> - visit_end_struct(vmdesc);
> + visit_end_struct(vmdesc, NULL);
> }
> }
> sub++;
> }
>
> if (vmdesc && subsection_found) {
> - visit_end_list(vmdesc);
> + visit_end_list(vmdesc, NULL);
> }
> }
>
> diff --git a/qapi/json-output-visitor.c b/qapi/json-output-visitor.c
> index f479034..0d67337 100644
> --- a/qapi/json-output-visitor.c
> +++ b/qapi/json-output-visitor.c
> @@ -58,7 +58,7 @@ static void json_output_start_struct(Visitor *v, const char
> *name, void **obj,
> jov->depth++;
> }
>
> -static void json_output_end_struct(Visitor *v)
> +static void json_output_end_struct(Visitor *v, void **obj)
> {
> JsonOutputVisitor *jov = to_jov(v);
> assert(jov->depth);
> @@ -87,7 +87,7 @@ static GenericList *json_output_next_list(Visitor *v,
> GenericList *tail,
> return tail->next;
> }
>
> -static void json_output_end_list(Visitor *v)
> +static void json_output_end_list(Visitor *v, void **obj)
> {
> JsonOutputVisitor *jov = to_jov(v);
> assert(jov->depth);
> diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
> index 4cf1cf8..dcfbf92 100644
> --- a/qapi/opts-visitor.c
> +++ b/qapi/opts-visitor.c
> @@ -180,7 +180,7 @@ opts_check_struct(Visitor *v, Error **errp)
>
>
> static void
> -opts_end_struct(Visitor *v)
> +opts_end_struct(Visitor *v, void **obj)
> {
> OptsVisitor *ov = to_ov(v);
>
> @@ -273,7 +273,7 @@ opts_next_list(Visitor *v, GenericList *tail, size_t size)
>
>
> static void
> -opts_end_list(Visitor *v)
> +opts_end_list(Visitor *v, void **obj)
> {
> OptsVisitor *ov = to_ov(v);
>
> diff --git a/qapi/qapi-clone-visitor.c b/qapi/qapi-clone-visitor.c
> index 42384d3..92771e3 100644
> --- a/qapi/qapi-clone-visitor.c
> +++ b/qapi/qapi-clone-visitor.c
> @@ -29,11 +29,8 @@ static void qapi_clone_start_struct(Visitor *v, const char
> *name, void **obj,
> QapiCloneVisitor *qcv = to_qcv(v);
>
> if (!obj) {
> - /* Nothing to allocate on the virtual walk during an
> - * alternate, but we still have to push depth.
> - * FIXME: passing obj to visit_end_struct would make this easier */
> + /* Nothing to allocate on the virtual walk */
> assert(qcv->depth);
> - qcv->depth++;
> return;
> }
>
Why can we elide qcv->depth++? Do the assert(qcv->qdepth) still hold?
> @@ -41,11 +38,13 @@ static void qapi_clone_start_struct(Visitor *v, const
> char *name, void **obj,
> qcv->depth++;
> }
>
> -static void qapi_clone_end(Visitor *v)
> +static void qapi_clone_end(Visitor *v, void **obj)
> {
> QapiCloneVisitor *qcv = to_qcv(v);
> assert(qcv->depth);
> - qcv->depth--;
> + if (obj) {
> + qcv->depth--;
> + }
> }
>
> static void qapi_clone_start_list(Visitor *v, const char *name,
> diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
> index cd68b55..9391dea 100644
> --- a/qapi/qapi-dealloc-visitor.c
> +++ b/qapi/qapi-dealloc-visitor.c
> @@ -19,53 +19,18 @@
> #include "qapi/qmp/types.h"
> #include "qapi/visitor-impl.h"
>
> -typedef struct StackEntry
> -{
> - void *value;
> - QTAILQ_ENTRY(StackEntry) node;
> -} StackEntry;
> -
> struct QapiDeallocVisitor
> {
> Visitor visitor;
> - QTAILQ_HEAD(, StackEntry) stack;
> };
>
> -static QapiDeallocVisitor *to_qov(Visitor *v)
> -{
> - return container_of(v, QapiDeallocVisitor, visitor);
> -}
> -
> -static void qapi_dealloc_push(QapiDeallocVisitor *qov, void *value)
> -{
> - StackEntry *e = g_malloc0(sizeof(*e));
> -
> - e->value = value;
> -
> - QTAILQ_INSERT_HEAD(&qov->stack, e, node);
> -}
> -
> -static void *qapi_dealloc_pop(QapiDeallocVisitor *qov)
> -{
> - StackEntry *e = QTAILQ_FIRST(&qov->stack);
> - QObject *value;
> - QTAILQ_REMOVE(&qov->stack, e, node);
> - value = e->value;
> - g_free(e);
> - return value;
> -}
> -
> static void qapi_dealloc_start_struct(Visitor *v, const char *name, void
> **obj,
> size_t unused, Error **errp)
> {
> - QapiDeallocVisitor *qov = to_qov(v);
> - qapi_dealloc_push(qov, obj);
> }
>
> -static void qapi_dealloc_end_struct(Visitor *v)
> +static void qapi_dealloc_end_struct(Visitor *v, void **obj)
> {
> - QapiDeallocVisitor *qov = to_qov(v);
> - void **obj = qapi_dealloc_pop(qov);
> if (obj) {
> g_free(*obj);
> }
> @@ -75,14 +40,10 @@ static void qapi_dealloc_start_alternate(Visitor *v,
> const char *name,
> GenericAlternate **obj, size_t size,
> bool promote_int, Error **errp)
> {
> - QapiDeallocVisitor *qov = to_qov(v);
> - qapi_dealloc_push(qov, obj);
> }
>
> -static void qapi_dealloc_end_alternate(Visitor *v)
> +static void qapi_dealloc_end_alternate(Visitor *v, void **obj)
> {
> - QapiDeallocVisitor *qov = to_qov(v);
> - void **obj = qapi_dealloc_pop(qov);
> if (obj) {
> g_free(*obj);
> }
> @@ -102,7 +63,7 @@ static GenericList *qapi_dealloc_next_list(Visitor *v,
> GenericList *tail,
> return next;
> }
>
> -static void qapi_dealloc_end_list(Visitor *v)
> +static void qapi_dealloc_end_list(Visitor *v, void **obj)
> {
> }
>
> @@ -178,7 +139,5 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
> v->visitor.type_any = qapi_dealloc_type_anything;
> v->visitor.type_null = qapi_dealloc_type_null;
>
> - QTAILQ_INIT(&v->stack);
> -
> return v;
> }
Much nicer.
> diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
> index aea90a1..84f32fc 100644
> --- a/qapi/qmp-input-visitor.c
> +++ b/qapi/qmp-input-visitor.c
> @@ -145,7 +145,7 @@ static void qmp_input_check_struct(Visitor *v, Error
> **errp)
> }
> }
>
> -static void qmp_input_pop(Visitor *v)
> +static void qmp_input_pop(Visitor *v, void **obj)
> {
> QmpInputVisitor *qiv = to_qiv(v);
> StackObject *tos = &qiv->stack[qiv->nb_stack - 1];
You could assert @obj matches tos->obj. Same for the other visitors
that still need a stack.
> diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
> index ef9f62c..4bdc00d 100644
> --- a/qapi/qmp-output-visitor.c
> +++ b/qapi/qmp-output-visitor.c
> @@ -107,7 +107,7 @@ static void qmp_output_start_struct(Visitor *v, const
> char *name, void **obj,
> qmp_output_push(qov, dict);
> }
>
> -static void qmp_output_end_struct(Visitor *v)
> +static void qmp_output_end_struct(Visitor *v, void **obj)
> {
> QmpOutputVisitor *qov = to_qov(v);
> QObject *value = qmp_output_pop(qov);
> @@ -131,7 +131,7 @@ static GenericList *qmp_output_next_list(Visitor *v,
> GenericList *tail,
> return tail->next;
> }
>
> -static void qmp_output_end_list(Visitor *v)
> +static void qmp_output_end_list(Visitor *v, void **end)
> {
> QmpOutputVisitor *qov = to_qov(v);
> QObject *value = qmp_output_pop(qov);
> diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> index 30b5879..1c66d6a 100644
> --- a/qapi/string-input-visitor.c
> +++ b/qapi/string-input-visitor.c
> @@ -181,7 +181,7 @@ static GenericList *next_list(Visitor *v, GenericList
> *tail, size_t size)
> return tail->next;
> }
>
> -static void end_list(Visitor *v)
> +static void end_list(Visitor *v, void **obj)
> {
> }
>
> diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
> index d013196..1504a89 100644
> --- a/qapi/string-output-visitor.c
> +++ b/qapi/string-output-visitor.c
> @@ -291,7 +291,7 @@ static GenericList *next_list(Visitor *v, GenericList
> *tail, size_t size)
> return ret;
> }
>
> -static void end_list(Visitor *v)
> +static void end_list(Visitor *v, void **obj)
> {
> StringOutputVisitor *sov = to_sov(v);
>
> diff --git a/qom/object.c b/qom/object.c
> index 3bc8a00..1562f7e 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -2038,7 +2038,7 @@ static void property_get_tm(Object *obj, Visitor *v,
> const char *name,
> }
> visit_check_struct(v, &err);
> out_end:
> - visit_end_struct(v);
> + visit_end_struct(v, NULL);
> out:
> error_propagate(errp, err);
>
> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
> index 51e62e2..926ec68 100644
> --- a/qom/object_interfaces.c
> +++ b/qom/object_interfaces.c
> @@ -71,7 +71,7 @@ Object *user_creatable_add(const QDict *qdict,
> obj = user_creatable_add_type(type, id, pdict, v, &local_err);
>
> out_visit:
> - visit_end_struct(v);
> + visit_end_struct(v, NULL);
>
> out:
> QDECREF(pdict);
> @@ -127,7 +127,7 @@ Object *user_creatable_add_type(const char *type, const
> char *id,
> if (!local_err) {
> visit_check_struct(v, &local_err);
> }
> - visit_end_struct(v);
> + visit_end_struct(v, NULL);
> if (local_err) {
> goto out;
> }
> diff --git a/tests/test-json-output-visitor.c
> b/tests/test-json-output-visitor.c
> index 5bf6fc5..5e8a5c8 100644
> --- a/tests/test-json-output-visitor.c
> +++ b/tests/test-json-output-visitor.c
> @@ -324,7 +324,7 @@ static void test_visitor_out_any(TestOutputVisitorData
> *data,
> qobj = QOBJECT(qint_from_int(-42));
> visit_start_list(data->ov, NULL, NULL, 0, &error_abort);
> visit_type_any(data->ov, NULL, &qobj, &error_abort);
> - visit_end_list(data->ov);
> + visit_end_list(data->ov, NULL);
> out = json_output_get_string(data->jov);
> if (*pretty) {
> g_assert_cmpstr(out, ==, "[\n -42\n]");
> @@ -341,7 +341,7 @@ static void test_visitor_out_any(TestOutputVisitorData
> *data,
> qobj = QOBJECT(qdict);
> visit_start_list(data->ov, NULL, NULL, 0, &error_abort);
> visit_type_any(data->ov, NULL, &qobj, &error_abort);
> - visit_end_list(data->ov);
> + visit_end_list(data->ov, NULL);
> qobject_decref(qobj);
> out = json_output_get_string(data->jov);
> if (*pretty) {
> diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
> index d02306c..56ee943 100644
> --- a/tests/test-qmp-input-visitor.c
> +++ b/tests/test-qmp-input-visitor.c
> @@ -305,7 +305,7 @@ static void test_visitor_in_null(TestInputVisitorData
> *data,
> visit_type_null(v, "b", &err);
> error_free_or_abort(&err);
> visit_check_struct(v, &error_abort);
> - visit_end_struct(v);
> + visit_end_struct(v, NULL);
> }
>
> static void test_visitor_in_union_flat(TestInputVisitorData *data,
> diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
> index 050d65b..e2aa494 100644
> --- a/tests/test-qmp-output-visitor.c
> +++ b/tests/test-qmp-output-visitor.c
> @@ -494,7 +494,7 @@ static void test_visitor_out_null(TestOutputVisitorData
> *data,
> visit_start_struct(data->ov, NULL, NULL, 0, &error_abort);
> visit_type_null(data->ov, "a", &error_abort);
> visit_check_struct(data->ov, &error_abort);
> - visit_end_struct(data->ov);
> + visit_end_struct(data->ov, NULL);
> arg = qmp_output_get_qobject(data->qov);
> g_assert(qobject_type(arg) == QTYPE_QDICT);
> qdict = qobject_to_qdict(arg);
> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
> index 92fbb0e..48436aa 100644
> --- a/docs/qapi-code-gen.txt
> +++ b/docs/qapi-code-gen.txt
> @@ -942,7 +942,7 @@ Example:
> }
> visit_check_struct(v, &err);
> out_obj:
> - visit_end_struct(v);
> + visit_end_struct(v, (void **)obj);
> if (err && visit_is_input(v)) {
> qapi_free_UserDefOne(*obj);
> *obj = NULL;
> @@ -970,7 +970,7 @@ Example:
> }
> }
>
> - visit_end_list(v);
> + visit_end_list(v, (void **)obj);
> if (err && visit_is_input(v)) {
> qapi_free_UserDefOneList(*obj);
> *obj = NULL;
- Re: [Qemu-ppc] [Qemu-devel] [PATCH v3 18/18] qapi: Add parameter to visit_end_*,
Markus Armbruster <=