Re: [Qemu-devel] [PATCH v3 18/18] qapi: Add parameter to visit_end_*

[Markus Armbruster]

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;