[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v15 14/23] qapi: Add visit_type_null() visitor
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v15 14/23] qapi: Add visit_type_null() visitor |
Date: |
Thu, 28 Apr 2016 18:40:53 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> Right now, qmp-output-visitor happens to produce a QNull result
> if nothing is actually visited between the creation of the visitor
> and the request for the resulting QObject. A stronger protocol
> would require that a QMP output visit MUST visit something. But
> to still be able to produce a JSON 'null' output, we need a new
> visitor function that states our intentions. Yes, we could say
> that such a visit must go through visit_type_any(), but that
> feels clunky.
>
> So this patch introduces the new visit_type_null() interface and
> its no-op interface in the dealloc visitor, and stubs in the
> qmp visitors (the next patch will finish the implementation).
> For the visitors that will not implement the callback, document
> the situation. The code in qapi-visit-core unconditionally
> dereferences the callback pointer, so that a segfault will inform
> a developer if they need to implement the callback for their
> choice of visitor.
>
> Note that JSON has a primitive null type, with the single value
> null; likewise with the QNull type for QObject; but for QAPI,
> we just have the 'null' value without a null type. We may
> eventually want to add more support in QAPI for null (most likely,
> we'd use it via an alternate type that permits 'null' or an
> object); but we'll create that usage when we need it.
>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> v15: improve commit message, add stubs here
> v14: no change
> v13: no change
> v12: rebase to earlier changes, drop R-b due to better documentation
> [no v10, v11]
> v9: no change
> v8: rebase to 'name' motion
> v7: new patch, based on discussion about spapr_drc.c
> ---
> include/qapi/visitor.h | 12 ++++++++++++
> include/qapi/visitor-impl.h | 3 +++
> include/qapi/opts-visitor.h | 3 ++-
> include/qapi/string-input-visitor.h | 2 +-
> include/qapi/string-output-visitor.h | 2 +-
> qapi/qapi-visit-core.c | 5 +++++
> qapi/qapi-dealloc-visitor.c | 5 +++++
> qapi/qmp-input-visitor.c | 6 ++++++
> qapi/qmp-output-visitor.c | 7 +++++++
> 9 files changed, 42 insertions(+), 3 deletions(-)
>
> diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
> index 0e028ba..e79c09e 100644
> --- a/include/qapi/visitor.h
> +++ b/include/qapi/visitor.h
> @@ -520,4 +520,16 @@ void visit_type_number(Visitor *v, const char *name,
> double *obj,
> */
> void visit_type_any(Visitor *v, const char *name, QObject **obj, Error
> **errp);
>
> +/*
> + * Visit a JSON null value.
> + *
> + * @name expresses the relationship of the null value to its parent
> + * container; see the general description of @name above.
> + *
> + * Unlike all other visit_type_* functions, no obj parameter is
> + * needed; rather, this is a witness that an explicit null value is
> + * expected rather than any other type.
> + */
> +void visit_type_null(Visitor *v, const char *name, Error **errp);
> +
> #endif
> diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
> index 796d180..88d27d5 100644
> --- a/include/qapi/visitor-impl.h
> +++ b/include/qapi/visitor-impl.h
> @@ -90,6 +90,9 @@ struct Visitor
> void (*type_any)(Visitor *v, const char *name, QObject **obj,
> Error **errp);
>
> + /* Must be set to visit explicit null values. */
> + void (*type_null)(Visitor *v, const char *name, Error **errp);
> +
> /* Must be set for input visitors, optional otherwise. The core
> * takes care of the return type in the public interface. */
> void (*optional)(Visitor *v, const char *name, bool *present);
> diff --git a/include/qapi/opts-visitor.h b/include/qapi/opts-visitor.h
> index 633aa71..fe37ed9 100644
> --- a/include/qapi/opts-visitor.h
> +++ b/include/qapi/opts-visitor.h
> @@ -31,7 +31,8 @@ typedef struct OptsVisitor OptsVisitor;
> * - values above INT64_MAX or LLONG_MAX are rejected.
> *
> * The Opts input visitor does not implement support for visiting QAPI
> - * alternates, numbers (other than integers), or arbitrary QTypes.
> + * alternates, numbers (other than integers), null, or arbitrary
> + * QTypes.
> */
> OptsVisitor *opts_visitor_new(const QemuOpts *opts);
> void opts_visitor_cleanup(OptsVisitor *nv);
> diff --git a/include/qapi/string-input-visitor.h
> b/include/qapi/string-input-visitor.h
> index fdf33ae..a8d8f67 100644
> --- a/include/qapi/string-input-visitor.h
> +++ b/include/qapi/string-input-visitor.h
> @@ -19,7 +19,7 @@ typedef struct StringInputVisitor StringInputVisitor;
>
> /*
> * The string input visitor does not implement support for visiting
> - * QAPI structs, alternates, or arbitrary QTypes.
> + * QAPI structs, alternates, null, or arbitrary QTypes.
> */
> StringInputVisitor *string_input_visitor_new(const char *str);
> void string_input_visitor_cleanup(StringInputVisitor *v);
> diff --git a/include/qapi/string-output-visitor.h
> b/include/qapi/string-output-visitor.h
> index 3bb09af..89b7e4b 100644
> --- a/include/qapi/string-output-visitor.h
> +++ b/include/qapi/string-output-visitor.h
> @@ -19,7 +19,7 @@ typedef struct StringOutputVisitor StringOutputVisitor;
>
> /*
> * The string output visitor does not implement support for visiting
> - * QAPI structs, alternates, or arbitrary QTypes.
> + * QAPI structs, alternates, null, or arbitrary QTypes.
> */
> StringOutputVisitor *string_output_visitor_new(bool human);
> void string_output_visitor_cleanup(StringOutputVisitor *v);
> diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> index a8492cf..7eaa75c 100644
> --- a/qapi/qapi-visit-core.c
> +++ b/qapi/qapi-visit-core.c
> @@ -244,6 +244,11 @@ void visit_type_any(Visitor *v, const char *name,
> QObject **obj, Error **errp)
> error_propagate(errp, err);
> }
>
> +void visit_type_null(Visitor *v, const char *name, Error **errp)
> +{
> + v->type_null(v, name, errp);
> +}
> +
> static void output_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 c19a459..413d525 100644
> --- a/qapi/qapi-dealloc-visitor.c
> +++ b/qapi/qapi-dealloc-visitor.c
> @@ -163,6 +163,10 @@ static void qapi_dealloc_type_anything(Visitor *v, const
> char *name,
> }
> }
>
> +static void qapi_dealloc_type_null(Visitor *v, const char *name, Error
> **errp)
> +{
> +}
> +
> Visitor *qapi_dealloc_get_visitor(QapiDeallocVisitor *v)
> {
> return &v->visitor;
> @@ -193,6 +197,7 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
> v->visitor.type_str = qapi_dealloc_type_str;
> v->visitor.type_number = qapi_dealloc_type_number;
> v->visitor.type_any = qapi_dealloc_type_anything;
> + v->visitor.type_null = qapi_dealloc_type_null;
>
> QTAILQ_INIT(&v->stack);
>
> diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
> index eb77adc..739bbd5 100644
> --- a/qapi/qmp-input-visitor.c
> +++ b/qapi/qmp-input-visitor.c
> @@ -340,6 +340,11 @@ static void qmp_input_type_any(Visitor *v, const char
> *name, QObject **obj,
> *obj = qobj;
> }
>
> +static void qmp_input_type_null(Visitor *v, const char *name, Error **errp)
> +{
> + error_setg(errp, "FIXME: Implement");
> +}
> +
I'd simply abort(). Can do on commit.
> static void qmp_input_optional(Visitor *v, const char *name, bool *present)
> {
> QmpInputVisitor *qiv = to_qiv(v);
> @@ -383,6 +388,7 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj, bool
> strict)
> v->visitor.type_str = qmp_input_type_str;
> v->visitor.type_number = qmp_input_type_number;
> v->visitor.type_any = qmp_input_type_any;
> + v->visitor.type_null = qmp_input_type_null;
> v->visitor.optional = qmp_input_optional;
> v->strict = strict;
>
> diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
> index 1f2a7ba..a67e3e8 100644
> --- a/qapi/qmp-output-visitor.c
> +++ b/qapi/qmp-output-visitor.c
> @@ -18,6 +18,7 @@
> #include "qemu/queue.h"
> #include "qemu-common.h"
> #include "qapi/qmp/types.h"
> +#include "qapi/error.h"
>
> typedef struct QStackEntry
> {
> @@ -196,6 +197,11 @@ static void qmp_output_type_any(Visitor *v, const char
> *name, QObject **obj,
> qmp_output_add_obj(qov, name, *obj);
> }
>
> +static void qmp_output_type_null(Visitor *v, const char *name, Error **errp)
> +{
> + error_setg(errp, "FIXME: Implement");
> +}
> +
Likewise.
> /* Finish building, and return the root object. Will not be NULL. */
> QObject *qmp_output_get_qobject(QmpOutputVisitor *qov)
> {
> @@ -246,6 +252,7 @@ QmpOutputVisitor *qmp_output_visitor_new(void)
> v->visitor.type_str = qmp_output_type_str;
> v->visitor.type_number = qmp_output_type_number;
> v->visitor.type_any = qmp_output_type_any;
> + v->visitor.type_null = qmp_output_type_null;
>
> QTAILQ_INIT(&v->stack);
- [Qemu-devel] [PATCH v15 04/23] qmp-input: Clean up stack handling, (continued)
- [Qemu-devel] [PATCH v15 04/23] qmp-input: Clean up stack handling, Eric Blake, 2016/04/27
- [Qemu-devel] [PATCH v15 11/23] qmp-input: Refactor when list is advanced, Eric Blake, 2016/04/27
- [Qemu-devel] [PATCH v15 13/23] tests: Add check-qnull, Eric Blake, 2016/04/27
- [Qemu-devel] [PATCH v15 02/23] qapi: Guarantee NULL obj on input visitor callback error, Eric Blake, 2016/04/27
- [Qemu-devel] [PATCH v15 02A/23] fixup! qapi: Guarantee NULL obj on input visitor callback error, Eric Blake, 2016/04/28
- [Qemu-devel] [PATCH v15 14/23] qapi: Add visit_type_null() visitor, Eric Blake, 2016/04/27
- Re: [Qemu-devel] [PATCH v15 14/23] qapi: Add visit_type_null() visitor,
Markus Armbruster <=
- [Qemu-devel] [PATCH v15 05/23] qapi: Use strict QMP input visitor in more places, Eric Blake, 2016/04/27
- [Qemu-devel] [PATCH v15 08/23] monitor: Let generated code validate arguments, Eric Blake, 2016/04/27
- Re: [Qemu-devel] [PATCH v15 08/23] monitor: Let generated code validate arguments, Eric Blake, 2016/04/28
[Qemu-devel] [PATCH v15 01/23] qapi-visit: Add visitor.type classification, Eric Blake, 2016/04/27