[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v14 02/19] qapi-visit: Add visitor.type classifi
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v14 02/19] qapi-visit: Add visitor.type classification |
Date: |
Wed, 13 Apr 2016 15:49:51 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> We have three classes of QAPI visitors: input, output, and dealloc.
> Currently, all implementations of these visitors have one thing in
> common based on their visitor type: the implementation used for the
> visit_type_enum() callback. But since we plan to add more such
> common behavior, in relation to documenting and further refining
> the semantics, it makes more sense to have the visitor
> implementations advertise which class they belong to, so the common
> qapi-visit-core code can use that information in multiple places.
>
> For this patch, knowing the class of a visitor implementation lets
> us make input_type_enum() and output_type_enum() become static
> functions, by replacing the callback function Visitor.type_enum()
> with the simpler enum member Visitor.type. Share a common
> assertion in qapi-visit-core as part of the refactoring.
>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> v14: no change
> v13: no change
> v12: new patch
> ---
> include/qapi/visitor-impl.h | 21 ++++++++++++---------
> qapi/qapi-visit-core.c | 28 +++++++++++++++-------------
> qapi/opts-visitor.c | 12 ++----------
> qapi/qapi-dealloc-visitor.c | 7 +------
> qapi/qmp-input-visitor.c | 2 +-
> qapi/qmp-output-visitor.c | 2 +-
> qapi/string-input-visitor.c | 2 +-
> qapi/string-output-visitor.c | 2 +-
> 8 files changed, 34 insertions(+), 42 deletions(-)
>
> diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
> index 2bd8f29..228a2a6 100644
> --- a/include/qapi/visitor-impl.h
> +++ b/include/qapi/visitor-impl.h
> @@ -14,6 +14,15 @@
>
> #include "qapi/visitor.h"
>
> +/* There are three classes of visitors; setting the class determines
> + * how QAPI enums are visited, as well as what additional restrictions
> + * can be asserted. */
> +typedef enum VisitorType {
> + VISITOR_INPUT,
> + VISITOR_OUTPUT,
> + VISITOR_DEALLOC,
> +} VisitorType;
> +
> struct Visitor
> {
> /* Must be set */
I think we should explain what makes a visitor an input/output/dealloc
visitor. Not necessarily in this patch, and not necessarily in this
place, just somewhere. Right now, the information is scattered.
> @@ -36,10 +45,6 @@ struct 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);
> -
> - /* Must be set. */
> void (*type_int64)(Visitor *v, const char *name, int64_t *obj,
> Error **errp);
> /* Must be set. */
> @@ -58,11 +63,9 @@ struct Visitor
>
> /* May be NULL; most useful for input visitors. */
> void (*optional)(Visitor *v, const char *name, bool *present);
> +
> + /* Must be set. */
> + VisitorType type;
> };
>
> -void input_type_enum(Visitor *v, const char *name, int *obj,
> - const char *const strings[], Error **errp);
> -void output_type_enum(Visitor *v, const char *name, int *obj,
> - const char *const strings[], Error **errp);
> -
> #endif
> diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> index fa680c9..3cd7edc 100644
> --- a/qapi/qapi-visit-core.c
> +++ b/qapi/qapi-visit-core.c
> @@ -72,12 +72,6 @@ bool visit_optional(Visitor *v, const char *name, bool
> *present)
> return *present;
> }
>
> -void visit_type_enum(Visitor *v, const char *name, int *obj,
> - const char *const strings[], Error **errp)
> -{
> - v->type_enum(v, name, obj, strings, errp);
> -}
> -
> void visit_type_int(Visitor *v, const char *name, int64_t *obj, Error **errp)
> {
> v->type_int64(v, name, obj, errp);
> @@ -208,14 +202,13 @@ void visit_type_any(Visitor *v, const char *name,
> QObject **obj, Error **errp)
> v->type_any(v, name, obj, errp);
> }
>
> -void output_type_enum(Visitor *v, const char *name, int *obj,
> - const char *const strings[], Error **errp)
> +static void output_type_enum(Visitor *v, const char *name, int *obj,
> + const char *const strings[], Error **errp)
> {
> int i = 0;
> int value = *obj;
> char *enum_str;
>
> - assert(strings);
> while (strings[i++] != NULL);
> if (value < 0 || value >= i - 1) {
> error_setg(errp, QERR_INVALID_PARAMETER, name ? name : "null");
> @@ -226,15 +219,13 @@ void output_type_enum(Visitor *v, const char *name, int
> *obj,
> visit_type_str(v, name, &enum_str, errp);
> }
>
> -void input_type_enum(Visitor *v, const char *name, int *obj,
> - const char *const strings[], Error **errp)
> +static void input_type_enum(Visitor *v, const char *name, int *obj,
> + const char *const strings[], Error **errp)
> {
> Error *local_err = NULL;
> int64_t value = 0;
> char *enum_str;
>
> - assert(strings);
> -
> visit_type_str(v, name, &enum_str, &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> @@ -257,3 +248,14 @@ void input_type_enum(Visitor *v, const char *name, int
> *obj,
> g_free(enum_str);
> *obj = value;
> }
> +
> +void visit_type_enum(Visitor *v, const char *name, int *obj,
> + const char *const strings[], Error **errp)
> +{
> + assert(strings);
> + if (v->type == VISITOR_INPUT) {
> + input_type_enum(v, name, obj, strings, errp);
> + } else if (v->type == VISITOR_OUTPUT) {
> + output_type_enum(v, name, obj, strings, errp);
> + }
> +}
> diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
> index 602f260..f98cf2e 100644
> --- a/qapi/opts-visitor.c
> +++ b/qapi/opts-visitor.c
> @@ -507,6 +507,8 @@ opts_visitor_new(const QemuOpts *opts)
>
> ov = g_malloc0(sizeof *ov);
>
> + ov->visitor.type = VISITOR_INPUT;
> +
> ov->visitor.start_struct = &opts_start_struct;
> ov->visitor.end_struct = &opts_end_struct;
>
> @@ -514,16 +516,6 @@ opts_visitor_new(const QemuOpts *opts)
> ov->visitor.next_list = &opts_next_list;
> ov->visitor.end_list = &opts_end_list;
>
> - /* input_type_enum() covers both "normal" enums and union discriminators.
> - * The union discriminator field is always generated as "type"; it should
> - * match the "type" QemuOpt child of any QemuOpts.
> - *
> - * input_type_enum() will remove the looked-up key from the
> - * "unprocessed_opts" hash even if the lookup fails, because the removal
> is
> - * done earlier in opts_type_str(). This should be harmless.
> - */
> - ov->visitor.type_enum = &input_type_enum;
> -
Hmm, this comment doesn't look worthless. With its statement gone, I
guess it should move somewhere else. What do you think?
> ov->visitor.type_int64 = &opts_type_int64;
> ov->visitor.type_uint64 = &opts_type_uint64;
> ov->visitor.type_size = &opts_type_size;
> diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
> index 6922179..c19a459 100644
> --- a/qapi/qapi-dealloc-visitor.c
> +++ b/qapi/qapi-dealloc-visitor.c
> @@ -163,11 +163,6 @@ static void qapi_dealloc_type_anything(Visitor *v, const
> char *name,
> }
> }
>
> -static void qapi_dealloc_type_enum(Visitor *v, const char *name, int *obj,
> - const char * const strings[], Error
> **errp)
> -{
> -}
> -
> Visitor *qapi_dealloc_get_visitor(QapiDeallocVisitor *v)
> {
> return &v->visitor;
> @@ -184,6 +179,7 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
>
> v = g_malloc0(sizeof(*v));
>
> + v->visitor.type = VISITOR_DEALLOC;
> v->visitor.start_struct = qapi_dealloc_start_struct;
> v->visitor.end_struct = qapi_dealloc_end_struct;
> v->visitor.start_alternate = qapi_dealloc_start_alternate;
> @@ -191,7 +187,6 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
> v->visitor.start_list = qapi_dealloc_start_list;
> v->visitor.next_list = qapi_dealloc_next_list;
> v->visitor.end_list = qapi_dealloc_end_list;
> - v->visitor.type_enum = qapi_dealloc_type_enum;
> v->visitor.type_int64 = qapi_dealloc_type_int64;
> v->visitor.type_uint64 = qapi_dealloc_type_uint64;
> v->visitor.type_bool = qapi_dealloc_type_bool;
> diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
> index 7cd1b77..02d4233 100644
> --- a/qapi/qmp-input-visitor.c
> +++ b/qapi/qmp-input-visitor.c
> @@ -339,13 +339,13 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj)
>
> v = g_malloc0(sizeof(*v));
>
> + v->visitor.type = VISITOR_INPUT;
> v->visitor.start_struct = qmp_input_start_struct;
> v->visitor.end_struct = qmp_input_end_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;
> v->visitor.type_bool = qmp_input_type_bool;
> diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
> index d44c676..1f2a7ba 100644
> --- a/qapi/qmp-output-visitor.c
> +++ b/qapi/qmp-output-visitor.c
> @@ -234,12 +234,12 @@ QmpOutputVisitor *qmp_output_visitor_new(void)
>
> v = g_malloc0(sizeof(*v));
>
> + v->visitor.type = VISITOR_OUTPUT;
> v->visitor.start_struct = qmp_output_start_struct;
> v->visitor.end_struct = qmp_output_end_struct;
> v->visitor.start_list = qmp_output_start_list;
> v->visitor.next_list = qmp_output_next_list;
> v->visitor.end_list = qmp_output_end_list;
> - v->visitor.type_enum = output_type_enum;
> v->visitor.type_int64 = qmp_output_type_int64;
> v->visitor.type_uint64 = qmp_output_type_uint64;
> v->visitor.type_bool = qmp_output_type_bool;
> diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> index ab12953..d604575 100644
> --- a/qapi/string-input-visitor.c
> +++ b/qapi/string-input-visitor.c
> @@ -348,7 +348,7 @@ StringInputVisitor *string_input_visitor_new(const char
> *str)
>
> v = g_malloc0(sizeof(*v));
>
> - v->visitor.type_enum = input_type_enum;
> + v->visitor.type = VISITOR_INPUT;
> v->visitor.type_int64 = parse_type_int64;
> v->visitor.type_uint64 = parse_type_uint64;
> v->visitor.type_size = parse_type_size;
> diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
> index c2e5c5b..0d44d7e 100644
> --- a/qapi/string-output-visitor.c
> +++ b/qapi/string-output-visitor.c
> @@ -351,7 +351,7 @@ StringOutputVisitor *string_output_visitor_new(bool human)
>
> v->string = g_string_new(NULL);
> v->human = human;
> - v->visitor.type_enum = output_type_enum;
> + v->visitor.type = VISITOR_OUTPUT;
> v->visitor.type_int64 = print_type_int64;
> v->visitor.type_uint64 = print_type_uint64;
> v->visitor.type_size = print_type_size;
- Re: [Qemu-devel] [PATCH v14 06/19] qmp-input: Don't consume input when checking has_member, (continued)
[Qemu-devel] [PATCH v14 01/19] qapi: Consolidate object visitors, Eric Blake, 2016/04/08
[Qemu-devel] [PATCH v14 11/19] qmp: Support explicit null during visits, Eric Blake, 2016/04/08
[Qemu-devel] [PATCH v14 02/19] qapi-visit: Add visitor.type classification, Eric Blake, 2016/04/08
- Re: [Qemu-devel] [PATCH v14 02/19] qapi-visit: Add visitor.type classification,
Markus Armbruster <=
[Qemu-devel] [PATCH v14 12/19] spapr_drc: Expose 'null' in qom-get when there is no fdt, Eric Blake, 2016/04/08
[Qemu-devel] [PATCH v14 15/19] qapi-commands: Wrap argument visit in visit_start_struct, Eric Blake, 2016/04/08
[Qemu-devel] [PATCH v14 13/19] qmp: Tighten output visitor rules, Eric Blake, 2016/04/08