Eric Blake <ebl...@redhat.com> writes:
> Implement the new type_null() callback for the qmp input visitor.
> While we don't yet have a use for this in qapi (the generator
> will need some tweaks first), one usage is already envisioned:
> when changing blockdev parameters, it would be nice to have a
> difference between leaving a tuning parameter unchanged (omit
> that parameter from the struct) and to explicitly reset the
> parameter to its default without having to know what the default
> value is (specify the parameter with an explicit null value,
> which will require us to allow a qapi alternate that chooses
> between the normal value and an explicit null).
>
> At any rate, we can test this without the use of generated qapi
> by manually using visit_start_struct()/visit_end_struct().
Well, we test by calling visit_type_null() manually. We choose to wrap
it in a visit_start_struct() ... visit_end_struct() pair, but that's
detail. Actually, we do an unwrapped root visit first, and then a
struct-wrapped visit.
Suggest "by calling visit_type_null() manually."
> Signed-off-by: Eric Blake <ebl...@redhat.com>
>
> ---
> v9: new patch
> ---
> include/qapi/visitor-impl.h | 2 +-
> qapi/qmp-input-visitor.c | 14 ++++++++++++++
> tests/test-qmp-input-visitor.c | 26 ++++++++++++++++++++++++++
> 3 files changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
> index 8705136..95408a5 100644
> --- a/include/qapi/visitor-impl.h
> +++ b/include/qapi/visitor-impl.h
> @@ -76,7 +76,7 @@ struct Visitor
> void (*type_any)(Visitor *v, const char *name, QObject **obj,
> Error **errp);
> /* Must be provided to visit explicit null values (right now, only the
> - * dealloc visitor supports this). */
> + * dealloc and qmp-input visitors support this). */
Will need updating.
> void (*type_null)(Visitor *v, const char *name, Error **errp);
>
> /* May be NULL; most useful for input visitors. */
> diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
> index 597652c..ad23bec 100644
> --- a/qapi/qmp-input-visitor.c
> +++ b/qapi/qmp-input-visitor.c
> @@ -315,6 +315,19 @@ 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)
> +{
> + QmpInputVisitor *qiv = to_qiv(v);
> + QObject *qobj = qmp_input_get_object(qiv, name, true);
> +
> + if (qobject_type(qobj) == QTYPE_QNULL) {
> + return;
> + }
> +
> + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
> + "null");
Recommend to put the error in the conditional:
if (qobject_type(qobj) != QTYPE_QNULL) {
error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
"null");
}
> +}
> +
> static void qmp_input_optional(Visitor *v, const char *name, bool *present)
> {
> QmpInputVisitor *qiv = to_qiv(v);
> @@ -358,6 +371,7 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj)
> 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->visitor.get_next_type = qmp_input_get_next_type;
>
> diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
> index f6bd408..6489e4a 100644
> --- a/tests/test-qmp-input-visitor.c
> +++ b/tests/test-qmp-input-visitor.c
> @@ -278,6 +278,30 @@ static void test_visitor_in_any(TestInputVisitorData
> *data,
> qobject_decref(res);
> }
>
> +static void test_visitor_in_null(TestInputVisitorData *data,
> + const void *unused)
> +{
> + Visitor *v;
> + QObject *null;
> +
> + v = visitor_input_test_init(data, "null");
> + visit_type_null(v, NULL, &error_abort);
> +
> + v = visitor_input_test_init(data, "{ 'a': null }");
> + visit_start_struct(v, NULL, NULL, 0, &error_abort);
> + visit_type_null(v, "a", &error_abort);
> + visit_end_struct(v, &error_abort);
> +
> + /* Check that qnull reference counting is sane:
> + * 1 for global use, 1 for our qnull() use, and 1 still owned by 'v'
> + * until it is torn down */
> + null = qnull();
> + g_assert(null->refcnt == 3);
> + visitor_input_teardown(data, NULL);
> + g_assert(null->refcnt == 2);
> + qobject_decref(null);
For other kinds of QObject, we leave the testing of reference counting
to the check-qKIND.c, and don't bother with it when testing the
visitors. Any particular reason to do null differently?
> +}
> +
> static void test_visitor_in_union_flat(TestInputVisitorData *data,
> const void *unused)
> {
> @@ -792,6 +816,8 @@ int main(int argc, char **argv)
> &in_visitor_data, test_visitor_in_list);
> input_visitor_test_add("/visitor/input/any",
> &in_visitor_data, test_visitor_in_any);
> + input_visitor_test_add("/visitor/input/null",
> + &in_visitor_data, test_visitor_in_null);
> input_visitor_test_add("/visitor/input/union-flat",
> &in_visitor_data, test_visitor_in_union_flat);
> input_visitor_test_add("/visitor/input/alternate",
