Eric Blake <ebl...@redhat.com> writes:
> After recent changes, the only remaining use of
> visit_start_implicit_struct() is for allocating the space needed
> when visiting an alternate. Since the term 'implicit struct' is
> hard to explain, rename the function to its current usage. While
> at it, we can merge the functionality of visit_get_next_type()
> into the same function, making it more like visit_start_struct().
>
> Generated code is now slightly smaller:
>
> | {
> | Error *err = NULL;
> |
> |- visit_start_implicit_struct(v, (void**) obj, sizeof(BlockdevRef), &err);
> |+ visit_start_alternate(v, name, (GenericAlternate **)obj, sizeof(**obj),
> |+ true, &err);
> | if (err) {
> | goto out;
> | }
> |- visit_get_next_type(v, name, &(*obj)->type, true, &err);
> |- if (err) {
> |- goto out_obj;
> |- }
> | switch ((*obj)->type) {
> | case QTYPE_QDICT:
> | visit_type_alternate_BlockdevOptions(v, name,
> &(*obj)->u.definition, &err);
> | break;
> ...
> | }
> |-out_obj:
> |- visit_end_implicit_struct(v);
> |+ visit_end_alternate(v);
> | out:
> | error_propagate(errp, err);
> | }
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
>
> ---
> v10: new patch
> ---
> include/qapi/visitor.h | 50
> ++++++++++++++++++++++++++++++++++-----------
> include/qapi/visitor-impl.h | 17 +++++++--------
> scripts/qapi-visit.py | 10 +++------
> qapi/qapi-visit-core.c | 40 +++++++++++++++---------------------
> qapi/qapi-dealloc-visitor.c | 13 ++++++------
> qapi/qmp-input-visitor.c | 24 ++++++++--------------
> 6 files changed, 82 insertions(+), 72 deletions(-)
>
> diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
> index b8ae1b5..83cad74 100644
> --- a/include/qapi/visitor.h
> +++ b/include/qapi/visitor.h
> @@ -19,7 +19,6 @@
> #include "qapi/error.h"
> #include <stdlib.h>
>
> -
> /* This struct is layout-compatible with all other *List structs
> * created by the qapi generator. It is used as a typical
> * singly-linked list. */
> @@ -28,17 +27,52 @@ typedef struct GenericList {
> char padding[];
> } GenericList;
>
> +/* This struct is layout-compatible with all Alternate types
> + * created by the qapi generator. */
> +typedef struct GenericAlternate {
> + QType type;
> + char padding[];
> +} GenericAlternate;
> +
> void visit_start_struct(Visitor *v, const char *name, void **obj,
> size_t size, Error **errp);
> void visit_end_struct(Visitor *v, Error **errp);
> -void visit_start_implicit_struct(Visitor *v, void **obj, size_t size,
> - Error **errp);
> -void visit_end_implicit_struct(Visitor *v);
>
> void visit_start_list(Visitor *v, const char *name, Error **errp);
> GenericList *visit_next_list(Visitor *v, GenericList **list, size_t size);
> void visit_end_list(Visitor *v);
>
> +/*
> + * Start the visit of an alternate @obj with the given @size.
> + *
> + * @name specifies the relationship to the containing struct (ignored
> + * for a top level visit, the name of the key if this alternate is
> + * part of an object, or NULL if this alternate is part of a list).
> + *
> + * @obj must not be NULL. Input visitors will allocate @obj and
> + * determine the qtype of the next thing to be visited, stored in
> + * (*@obj)->type. Other visitors will leave @obj unchanged.
> + *
> + * If @promote_int, treat integers as QTYPE_FLOAT.
> + *
> + * If successful, this must be paired with visit_end_alternate(), even
> + * if visiting the contents of the alternate fails.
> + */
> +void visit_start_alternate(Visitor *v, const char *name,
> + GenericAlternate **obj, size_t size,
> + bool promote_int, Error **errp);
> +
> +/*
> + * Finish visiting an alternate type.
> + *
> + * Must be called after a successful visit_start_alternate(), even if
> + * an error occurred in the meantime.
> + *
> + * 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);
> +
> /**
> * Check if an optional member @name of an object needs visiting.
> * For input visitors, set *@present according to whether the
> @@ -47,14 +81,6 @@ void visit_end_list(Visitor *v);
> */
> bool visit_optional(Visitor *v, const char *name, bool *present);
>
> -/**
> - * Determine the qtype of the item @name in the current object visit.
> - * For input visitors, set *@type to the correct qtype of a qapi
> - * alternate type; for other visitors, leave *@type unchanged.
> - * If @promote_int, treat integers as QTYPE_FLOAT.
> - */
> -void visit_get_next_type(Visitor *v, const char *name, QType *type,
> - bool promote_int, Error **errp);
> void visit_type_enum(Visitor *v, const char *name, int *obj,
> const char *const strings[], Error **errp);
> void visit_type_int(Visitor *v, const char *name, int64_t *obj, Error
> **errp);
> diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
> index c4af3e0..6a1ddfb 100644
> --- a/include/qapi/visitor-impl.h
> +++ b/include/qapi/visitor-impl.h
> @@ -22,22 +22,23 @@ struct Visitor
> size_t size, Error **errp);
> void (*end_struct)(Visitor *v, Error **errp);
>
> - void (*start_implicit_struct)(Visitor *v, void **obj, size_t size,
> - Error **errp);
> - /* May be NULL */
> - void (*end_implicit_struct)(Visitor *v);
> -
> void (*start_list)(Visitor *v, const char *name, Error **errp);
> /* Must be set */
> GenericList *(*next_list)(Visitor *v, GenericList **list, size_t size);
> /* Must be set */
> void (*end_list)(Visitor *v);
>
> + /* Optional, needed for input and dealloc visitors. */
> + void (*start_alternate)(Visitor *v, const char *name,
> + GenericAlternate **obj, size_t size,
> + bool promote_int, Error **errp);
> +
> + /* Optional, needed for dealloc visitor. */
> + void (*end_alternate)(Visitor *v);
> +
> + /* Must be set. */
> void (*type_enum)(Visitor *v, const char *name, int *obj,
> const char *const strings[], Error **errp);
> - /* May be NULL; only needed for input visitors. */
> - void (*get_next_type)(Visitor *v, const char *name, QType *type,
> - bool promote_int, Error **errp);
>
> /* Must be set. */
> void (*type_int64)(Visitor *v, const char *name, int64_t *obj,
> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
> index 02f0122..2749331 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -175,14 +175,11 @@ void visit_type_%(c_name)s(Visitor *v, const char
> *name, %(c_name)s **obj, Error
> {
> Error *err = NULL;
>
> - visit_start_implicit_struct(v, (void**) obj, sizeof(%(c_name)s), &err);
> + visit_start_alternate(v, name, (GenericAlternate **)obj, sizeof(**obj),
> + %(promote_int)s, &err);
> if (err) {
> goto out;
> }
> - visit_get_next_type(v, name, &(*obj)->type, %(promote_int)s, &err);
> - if (err) {
> - goto out_obj;
> - }
> switch ((*obj)->type) {
> ''',
> c_name=c_name(name), promote_int=promote_int)
> @@ -205,8 +202,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");
> }
> -out_obj:
> - visit_end_implicit_struct(v);
> + visit_end_alternate(v);
> out:
> error_propagate(errp, err);
> }
> diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> index 976106e..973ab72 100644
> --- a/qapi/qapi-visit-core.c
> +++ b/qapi/qapi-visit-core.c
> @@ -30,21 +30,6 @@ void visit_end_struct(Visitor *v, Error **errp)
> v->end_struct(v, errp);
> }
>
> -void visit_start_implicit_struct(Visitor *v, void **obj, size_t size,
> - Error **errp)
> -{
> - if (v->start_implicit_struct) {
> - v->start_implicit_struct(v, obj, size, errp);
> - }
> -}
> -
> -void visit_end_implicit_struct(Visitor *v)
> -{
> - if (v->end_implicit_struct) {
> - v->end_implicit_struct(v);
> - }
> -}
> -
> void visit_start_list(Visitor *v, const char *name, Error **errp)
> {
> v->start_list(v, name, errp);
> @@ -60,6 +45,23 @@ void visit_end_list(Visitor *v)
> v->end_list(v);
> }
>
> +void visit_start_alternate(Visitor *v, const char *name,
> + GenericAlternate **obj, size_t size,
> + bool promote_int, Error **errp)
> +{
> + assert(obj && size >= sizeof(GenericAlternate));
> + if (v->start_alternate) {
> + v->start_alternate(v, name, obj, size, promote_int, errp);
> + }
> +}
> +
> +void visit_end_alternate(Visitor *v)
> +{
> + if (v->end_alternate) {
> + v->end_alternate(v);
> + }
> +}
> +
> bool visit_optional(Visitor *v, const char *name, bool *present)
> {
> if (v->optional) {
> @@ -68,14 +70,6 @@ bool visit_optional(Visitor *v, const char *name, bool
> *present)
> return *present;
> }
>
> -void visit_get_next_type(Visitor *v, const char *name, QType *type,
> - bool promote_int, Error **errp)
> -{
> - if (v->get_next_type) {
> - v->get_next_type(v, name, type, promote_int, errp);
> - }
> -}
> -
> void visit_type_enum(Visitor *v, const char *name, int *obj,
> const char *const strings[], Error **errp)
> {
> diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
> index 4eae555..6922179 100644
> --- a/qapi/qapi-dealloc-visitor.c
> +++ b/qapi/qapi-dealloc-visitor.c
> @@ -76,16 +76,15 @@ static void qapi_dealloc_end_struct(Visitor *v, Error
> **errp)
> }
> }
>
> -static void qapi_dealloc_start_implicit_struct(Visitor *v,
> - void **obj,
> - size_t size,
> - Error **errp)
> +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_implicit_struct(Visitor *v)
> +static void qapi_dealloc_end_alternate(Visitor *v)
> {
> QapiDeallocVisitor *qov = to_qov(v);
> void **obj = qapi_dealloc_pop(qov);
> @@ -187,8 +186,8 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
>
> v->visitor.start_struct = qapi_dealloc_start_struct;
> v->visitor.end_struct = qapi_dealloc_end_struct;
> - v->visitor.start_implicit_struct = qapi_dealloc_start_implicit_struct;
> - v->visitor.end_implicit_struct = qapi_dealloc_end_implicit_struct;
> + v->visitor.start_alternate = qapi_dealloc_start_alternate;
> + v->visitor.end_alternate = qapi_dealloc_end_alternate;
> v->visitor.start_list = qapi_dealloc_start_list;
> v->visitor.next_list = qapi_dealloc_next_list;
> v->visitor.end_list = qapi_dealloc_end_list;
> diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
> index 2621660..e659832 100644
> --- a/qapi/qmp-input-visitor.c
> +++ b/qapi/qmp-input-visitor.c
> @@ -143,14 +143,6 @@ static void qmp_input_end_struct(Visitor *v, Error
> **errp)
> qmp_input_pop(qiv, errp);
> }
>
> -static void qmp_input_start_implicit_struct(Visitor *v, void **obj,
> - size_t size, Error **errp)
> -{
> - if (obj) {
> - *obj = g_malloc0(size);
> - }
> -}
> -
> static void qmp_input_start_list(Visitor *v, const char *name, Error **errp)
> {
> QmpInputVisitor *qiv = to_qiv(v);
> @@ -202,19 +194,22 @@ static void qmp_input_end_list(Visitor *v)
> qmp_input_pop(qiv, &error_abort);
> }
>
> -static void qmp_input_get_next_type(Visitor *v, const char *name, QType
> *type,
> - bool promote_int, Error **errp)
> +static void qmp_input_start_alternate(Visitor *v, const char *name,
> + GenericAlternate **obj, size_t size,
> + bool promote_int, Error **errp)
> {
> QmpInputVisitor *qiv = to_qiv(v);
> QObject *qobj = qmp_input_get_object(qiv, name, false);
>
> if (!qobj) {
> + *obj = NULL;
> error_setg(errp, QERR_MISSING_PARAMETER, name ? name : "null");
> return;
> }
> - *type = qobject_type(qobj);
> - if (promote_int && *type == QTYPE_QINT) {
> - *type = QTYPE_QFLOAT;
> + *obj = g_malloc0(size);
> + (*obj)->type = qobject_type(qobj);
> + if (promote_int && (*obj)->type == QTYPE_QINT) {
> + (*obj)->type = QTYPE_QFLOAT;
> }
> }
>
> @@ -345,10 +340,10 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj)
>
> v->visitor.start_struct = qmp_input_start_struct;
> v->visitor.end_struct = qmp_input_end_struct;
> - v->visitor.start_implicit_struct = qmp_input_start_implicit_struct;
> v->visitor.start_list = qmp_input_start_list;
> v->visitor.next_list = qmp_input_next_list;
> v->visitor.end_list = qmp_input_end_list;
> + v->visitor.start_alternate = qmp_input_start_alternate;
> v->visitor.type_enum = input_type_enum;
> v->visitor.type_int64 = qmp_input_type_int64;
> v->visitor.type_uint64 = qmp_input_type_uint64;
> @@ -357,7 +352,6 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj)
> v->visitor.type_number = qmp_input_type_number;
> v->visitor.type_any = qmp_input_type_any;
> v->visitor.optional = qmp_input_optional;
> - v->visitor.get_next_type = qmp_input_get_next_type;
>
> qmp_input_push(v, obj, NULL);
> qobject_incref(obj);
Okay, this start_alternate stuff looks like it could actually have a
chance not to confuse me every time I run across it, unlike the
implicit_struct stuff it replaces.
