Re: [Qemu-devel] [PATCH v14 07/21] qapi: permit scalar type conversions in QObjectInputVisitor

[Markus Armbruster]

"Daniel P. Berrange" <address@hidden> writes:

> Currently the QObjectInputVisitor assumes that all scalar
> values are directly represented as the final types declared
> by the thing being visited. ie it assumes an 'int' is using

i.e.

> QInt, and a 'bool' is using QBool, etc.  This is good when
> QObjectInputVisitor is fed a QObject that came from a JSON
> document on the QMP monitor, as it will strictly validate
> correctness.
>
> To allow QObjectInputVisitor to be reused for visiting
> a QObject originating from QemuOpts, an alternative mode
> is needed where all the scalars types are represented as
> QString and converted on the fly to the final desired
> type.
>
> Reviewed-by: Kevin Wolf <address@hidden>
> Reviewed-by: Marc-André Lureau <address@hidden>
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
>  include/qapi/qobject-input-visitor.h |  32 +++++-
>  qapi/qobject-input-visitor.c         | 132 ++++++++++++++++++++++++
>  tests/test-qobject-input-visitor.c   | 194 
> ++++++++++++++++++++++++++++++++++-
>  3 files changed, 350 insertions(+), 8 deletions(-)

Quite some code.  Here's how I'm persuading myself to like it.

We need to parse two and a half different languages into QAPI objects:
JSON, command line key=value,... (QemuOpts), and just value (same syntax
as in QemuOpts).

Two language differences stand out:

1. JSON is recursive, key=value,... is flat (but see below), and

2. JSON values have syntactically obvious JSON types, key=value values
   are just strings.

To parse JSON into a QAPI object, we first parse it into a QObject, then
use the QObject input visitor to convert the QObject into a QAPI object.

To parse key=value, we first parse it into a QemuOpts, then use either
the options visitor to convert the QemuOpts into a QAPI object.  We can
also use the string input visitor to convert just an individual key's
value.  Or any string for that matter.

Note that we always use the QemuOpts *string* value, never the integer
or boolean value QemuOpts additionally provides when the key has been
specified by a QemuOptDesc with a non-string type.  Such QemuOptDesc are
best avoided when we parse the string again with a visitor.  For when it
isn't avoided, the visitors' parsers need to match the QemuOpts parser.
Yes, this is a mess.

The flatness of key=value,... has become overly limiting.  We've grown
workarounds for lists in both the options and the string input visitor.
Independent ones *sigh*.  More recently, the block layer's dotted key
convention has provided a way to overcome flatness completely.

We could implement this convention in the options visitor.  Instead,
you're picking a different route: you reuse existing
qemu_opts_to_qdict() to go from QemuOpts to flat qdict, create
qdict_crumple() to unflatten using dotted key convention, then reuse the
existing QObject input visitor to go to QAPI object.  This decomposes
the task into smaller ones, some of them solved already, at a tolerable
cost in efficiency.

Except the QObject input visitor won't do as is, because of language
difference 2.  This patch makes it cope.

The need to do this negates much of the code reuse advantage.  Doesn't
look so hot by itself.  However, you then do more work and replace the
options visitor, with an obvious path to replacing the string input
visitor as well.

> diff --git a/include/qapi/qobject-input-visitor.h 
> b/include/qapi/qobject-input-visitor.h
> index cde328d..5022297 100644
> --- a/include/qapi/qobject-input-visitor.h
> +++ b/include/qapi/qobject-input-visitor.h
> @@ -19,12 +19,36 @@
>  
>  typedef struct QObjectInputVisitor QObjectInputVisitor;
>  
> -/*
> - * Return a new input visitor that converts a QObject to a QAPI object.
> +/**
> + * Create a new input visitor that converts @obj to a QAPI object.
> + *
> + * Any scalar values in the @obj input data structure should be in the
> + * required type already. i.e. if visiting a bool, the value should
> + * already be a QBool instance.
>   *
> - * Set @strict to reject a parse that doesn't consume all keys of a
> - * dictionary; otherwise excess input is ignored.
> + * If @strict is set to true, then an error will be reported if any
> + * dict keys are not consumed during visitation. If @strict is false
> + * then extra dict keys are silently ignored.

Aside: I want to get rid of non-strict, badly.  Quoting commit 240f64b
"qapi: Use strict QMP input visitor in more places":

    The only remaining uses of non-strict input visits are:
    
    - QMP 'qom-set' (which eventually executes
    object_property_set_qobject()) - mark it as something to revisit
    in the future (I didn't want to spend any more time on this patch
    auditing if we have any QOM dictionary properties that might be
    impacted, and couldn't easily prove whether this code path is
    shared with anything else).
    
    - test-qmp-input-visitor: explicit tests of non-strict mode. If
    we later get rid of users that don't need strictness, then this
    test should be merged with test-qmp-input-strict

> + *
> + * The returned input visitor should be released by calling
> + * visit_free() when no longer required.
>   */
>  Visitor *qobject_input_visitor_new(QObject *obj, bool strict);
>  
> +/**
> + * Create a new input visitor that converts @obj to a QAPI object.
> + *
> + * Any scalar values in the @obj input data structure should always be
> + * represented as strings. i.e. if visiting a boolean, the value should
> + * be a QString whose contents represent a valid boolean.
> + *
> + * The visitor always operates in strict mode, requiring all dict keys
> + * to be consumed during visitation. An error will be reported if this
> + * does not happen.
> + *
> + * The returned input visitor should be released by calling
> + * visit_free() when no longer required.
> + */
> +Visitor *qobject_input_visitor_new_autocast(QObject *obj);

Not thrilled about _autocast.  It kind of suggests that scalar values
are converted as necessary.  They are, but the full truth is that all
scalar values must be strings.  _string_scalars is even longer, though.

> +
>  #endif
> diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
> index 5ff3db3..cf41df6 100644
> --- a/qapi/qobject-input-visitor.c
> +++ b/qapi/qobject-input-visitor.c
> @@ -20,6 +20,7 @@
>  #include "qemu-common.h"
>  #include "qapi/qmp/types.h"
>  #include "qapi/qmp/qerror.h"
> +#include "qemu/cutils.h"
>  
>  #define QIV_STACK_SIZE 1024
>  
> @@ -263,6 +264,28 @@ static void qobject_input_type_int64(Visitor *v, const 
> char *name, int64_t *obj,
>      *obj = qint_get_int(qint);
>  }
>  
> +
> +static void qobject_input_type_int64_autocast(Visitor *v, const char *name,
> +                                              int64_t *obj, Error **errp)
> +{
> +    QObjectInputVisitor *qiv = to_qiv(v);
> +    QString *qstr = qobject_to_qstring(qobject_input_get_object(qiv, name,
> +                                                                true));

Needs a rebase for commit 1382d4a.  Same elsewhere.

> +    int64_t ret;
> +
> +    if (!qstr || !qstr->string) {

I don't think !qstr->string can happen.  Same elsewhere.

> +        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
> +                   "string");
> +        return;
> +    }

So far, this is basically qobject_input_type_str() less the g_strdup().
Worth factoring out?

Now we're entering out parsing swamp:

> +
> +    if (qemu_strtoll(qstr->string, NULL, 0, &ret) < 0) {
> +        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name, "a number");
> +        return;
> +    }

To serve as replacement for the options visitor, this needs to parse
exactly like opts_type_int64().

It should also match the JSON parser as far as possible, to minimize
difference between the two QObject input visitor variants, and the
QemuOpts parser, for command line consistency.

opts_type_int64() uses strtoll() directly.  It carefully checks for no
conversion (both ways, EINVAL and endptr == str), range, and string not
fully consumed.

Your code uses qemu_strtoll().  Bug#1: qemu_strtoll() assumes long long
is exactly 64 bits.  If it's wider, we fail to diagnose overflow.  If
it's narrower, we can't parse all values.  Bug#2: your code fails to
check the string is fully consumed.

The JSON parser also uses strtoll() directly, in parse_literal().  When
we get there, we know that the string consists only of decimal digits,
possibly prefixed by a minus sign.  Therefore, strtoll() can only fail
with ERANGE, and parse_literal() handles that correctly.

QemuOpts doesn't do signed integers.

> +    *obj = ret;
> +}
> +
>  static void qobject_input_type_uint64(Visitor *v, const char *name,
>                                        uint64_t *obj, Error **errp)
>  {
> @@ -279,6 +302,27 @@ static void qobject_input_type_uint64(Visitor *v, const 
> char *name,
>      *obj = qint_get_int(qint);
>  }
>  
> +static void qobject_input_type_uint64_autocast(Visitor *v, const char *name,
> +                                               uint64_t *obj, Error **errp)
> +{
> +    QObjectInputVisitor *qiv = to_qiv(v);
> +    QString *qstr = qobject_to_qstring(qobject_input_get_object(qiv, name,
> +                                                                true));
> +    unsigned long long ret;
> +
> +    if (!qstr || !qstr->string) {
> +        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
> +                   "string");
> +        return;
> +    }
> +
> +    if (parse_uint_full(qstr->string, &ret, 0) < 0) {
> +        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name, "a number");
> +        return;
> +    }
> +    *obj = ret;

Differently wrong :)

To serve as replacement for the options visitor, this needs to parse
exactly like opts_type_uint64().

Again, this should also match the JSON parser and the QemuOpts parser as
far as possible.

opts_type_uint64() uses parse_uint().  It carefully checks for no
conversion (EINVAL; parse_uint() normalizes), range, and string not
fully consumed.

You use parse_uint_full().  You therefore don't have bug#2 here.  You do
have bug#1: you assume unsigned long long is exactly 64 bits.  If it's
wider, we fail to diagnose overflow.  If it's narrower, we can't parse
all values.

There's a discrepancy with the JSON parser, but it's not your fault: the
JSON parser represents JSON numbers that fit into int64_t to QInt, and
any others as QFloat.  In particular, the integers between INT64_MAX+1
and UINT64_MAX are represented as QFloat.  qmp_input_type_uint64()
accepts only QInt.  You can declare things as uint64 in the schema, but
you're still limited to 0..UINT64_MAX in QMP.

QemuOpts uses strtoull() directly, in parse_option_number().  Bug (not
yours): it happily ignores errors other than "string not fully
consumed".

> +}
> +
>  static void qobject_input_type_bool(Visitor *v, const char *name, bool *obj,
>                                      Error **errp)
>  {
> @@ -294,6 +338,22 @@ static void qobject_input_type_bool(Visitor *v, const 
> char *name, bool *obj,
>      *obj = qbool_get_bool(qbool);
>  }
>  
> +static void qobject_input_type_bool_autocast(Visitor *v, const char *name,
> +                                             bool *obj, Error **errp)
> +{
> +    QObjectInputVisitor *qiv = to_qiv(v);
> +    QString *qstr = qobject_to_qstring(qobject_input_get_object(qiv, name,
> +                                                                true));
> +
> +    if (!qstr || !qstr->string) {
> +        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
> +                   "string");
> +        return;
> +    }
> +
> +    parse_option_bool(name, qstr->string, obj, errp);

To serve as replacement for the options visitor, this needs to parse
exactly like opts_type_bool().  Which parses with parse_option_bool(),
too.  Good.

QemuOpts parses the same way.

JSON syntax is different.

> +}
> +
>  static void qobject_input_type_str(Visitor *v, const char *name, char **obj,
>                                     Error **errp)
>  {
> @@ -335,6 +395,30 @@ static void qobject_input_type_number(Visitor *v, const 
> char *name, double *obj,
>                 "number");
>  }
>  
> +static void qobject_input_type_number_autocast(Visitor *v, const char *name,
> +                                               double *obj, Error **errp)
> +{
> +    QObjectInputVisitor *qiv = to_qiv(v);
> +    QString *qstr = qobject_to_qstring(qobject_input_get_object(qiv, name,
> +                                                                true));
> +    char *endp;
> +
> +    if (!qstr || !qstr->string) {
> +        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
> +                   "string");
> +        return;
> +    }
> +
> +    errno = 0;
> +    *obj = strtod(qstr->string, &endp);
> +    if (errno == 0 && endp != qstr->string && *endp == '\0') {
> +        return;
> +    }
> +
> +    error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
> +               "number");
> +}
> +

The options visitor does not implement this method.

Your error checking looks good.

The JSON parser also uses strtod(), in parse_literal().  When we get
there, we know that the string is a valid JSON number.  Therefore,
strtod() can only fail with ERANGE.  Bug (not yours): we ignore that.

QemuOpts doesn't do floating-point.

>  static void qobject_input_type_any(Visitor *v, const char *name, QObject 
> **obj,
>                                     Error **errp)
>  {
> @@ -356,6 +440,22 @@ static void qobject_input_type_null(Visitor *v, const 
> char *name, Error **errp)
>      }
>  }
>  
> +static void qobject_input_type_size_autocast(Visitor *v, const char *name,
> +                                             uint64_t *obj, Error **errp)
> +{
> +    QObjectInputVisitor *qiv = to_qiv(v);
> +    QString *qstr = qobject_to_qstring(qobject_input_get_object(qiv, name,
> +                                                                true));
> +
> +    if (!qstr || !qstr->string) {
> +        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
> +                   "string");
> +        return;
> +    }
> +
> +    parse_option_size(name, qstr->string, obj, errp);

The options visitor uses qemu_strtosz_suffix().  Are you sure
parse_option_size() is a compatible replacement?  (I'm getting tired...)

> +}
> +
>  static void qobject_input_optional(Visitor *v, const char *name, bool 
> *present)
>  {
>      QObjectInputVisitor *qiv = to_qiv(v);
> @@ -413,3 +513,35 @@ Visitor *qobject_input_visitor_new(QObject *obj, bool 
> strict)
>  
>      return &v->visitor;
>  }
> +
> +Visitor *qobject_input_visitor_new_autocast(QObject *obj)
> +{
> +    QObjectInputVisitor *v;
> +
> +    v = g_malloc0(sizeof(*v));
> +
> +    v->visitor.type = VISITOR_INPUT;
> +    v->visitor.start_struct = qobject_input_start_struct;
> +    v->visitor.check_struct = qobject_input_check_struct;
> +    v->visitor.end_struct = qobject_input_pop;
> +    v->visitor.start_list = qobject_input_start_list;
> +    v->visitor.next_list = qobject_input_next_list;
> +    v->visitor.end_list = qobject_input_pop;
> +    v->visitor.start_alternate = qobject_input_start_alternate;
> +    v->visitor.type_int64 = qobject_input_type_int64_autocast;
> +    v->visitor.type_uint64 = qobject_input_type_uint64_autocast;
> +    v->visitor.type_bool = qobject_input_type_bool_autocast;
> +    v->visitor.type_str = qobject_input_type_str;
> +    v->visitor.type_number = qobject_input_type_number_autocast;
> +    v->visitor.type_any = qobject_input_type_any;
> +    v->visitor.type_null = qobject_input_type_null;
> +    v->visitor.type_size = qobject_input_type_size_autocast;
> +    v->visitor.optional = qobject_input_optional;
> +    v->visitor.free = qobject_input_free;
> +    v->strict = true;
> +
> +    v->root = obj;
> +    qobject_incref(obj);
> +
> +    return &v->visitor;
> +}
> diff --git a/tests/test-qobject-input-visitor.c 
> b/tests/test-qobject-input-visitor.c
> index 26c5012..d5b8044 100644
> --- a/tests/test-qobject-input-visitor.c
> +++ b/tests/test-qobject-input-visitor.c
> @@ -41,6 +41,7 @@ static void visitor_input_teardown(TestInputVisitorData 
> *data,
>     function so that the JSON string used by the tests are kept in the test
>     functions (and not in main()). */
>  static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data,
> +                                                 bool strict, bool autocast,
>                                                   const char *json_string,
>                                                   va_list *ap)
>  {
> @@ -49,11 +50,31 @@ static Visitor 
> *visitor_input_test_init_internal(TestInputVisitorData *data,
>      data->obj = qobject_from_jsonv(json_string, ap);
>      g_assert(data->obj);
>  
> -    data->qiv = qobject_input_visitor_new(data->obj, false);
> +    if (autocast) {
> +        assert(strict);
> +        data->qiv = qobject_input_visitor_new_autocast(data->obj);
> +    } else {
> +        data->qiv = qobject_input_visitor_new(data->obj, strict);
> +    }
>      g_assert(data->qiv);
>      return data->qiv;
>  }
>  
> +static GCC_FMT_ATTR(4, 5)
> +Visitor *visitor_input_test_init_full(TestInputVisitorData *data,
> +                                      bool strict, bool autocast,
> +                                      const char *json_string, ...)
> +{
> +    Visitor *v;
> +    va_list ap;
> +
> +    va_start(ap, json_string);
> +    v = visitor_input_test_init_internal(data, strict, autocast,
> +                                         json_string, &ap);
> +    va_end(ap);
> +    return v;
> +}
> +
>  static GCC_FMT_ATTR(2, 3)
>  Visitor *visitor_input_test_init(TestInputVisitorData *data,
>                                   const char *json_string, ...)
> @@ -62,7 +83,8 @@ Visitor *visitor_input_test_init(TestInputVisitorData *data,
>      va_list ap;
>  
>      va_start(ap, json_string);
> -    v = visitor_input_test_init_internal(data, json_string, &ap);
> +    v = visitor_input_test_init_internal(data, true, false,
> +                                         json_string, &ap);
>      va_end(ap);
>      return v;
>  }
> @@ -77,7 +99,8 @@ Visitor *visitor_input_test_init(TestInputVisitorData *data,
>  static Visitor *visitor_input_test_init_raw(TestInputVisitorData *data,
>                                              const char *json_string)
>  {
> -    return visitor_input_test_init_internal(data, json_string, NULL);
> +    return visitor_input_test_init_internal(data, true, false,
> +                                            json_string, NULL);
>  }
>  
>  static void test_visitor_in_int(TestInputVisitorData *data,
> @@ -109,6 +132,45 @@ static void 
> test_visitor_in_int_overflow(TestInputVisitorData *data,
>      error_free_or_abort(&err);
>  }
>  
> +static void test_visitor_in_int_autocast(TestInputVisitorData *data,
> +                                         const void *unused)
> +{
> +    int64_t res = 0, value = -42;
> +    Error *err = NULL;
> +    Visitor *v;
> +
> +    v = visitor_input_test_init_full(data, true, true,
> +                                     "%" PRId64, value);
> +    visit_type_int(v, NULL, &res, &err);
> +    error_free_or_abort(&err);
> +}
> +
> +static void test_visitor_in_int_str_autocast(TestInputVisitorData *data,
> +                                             const void *unused)
> +{
> +    int64_t res = 0, value = -42;
> +    Visitor *v;
> +
> +    v = visitor_input_test_init_full(data, true, true,
> +                                     "\"-42\"");
> +
> +    visit_type_int(v, NULL, &res, &error_abort);
> +    g_assert_cmpint(res, ==, value);
> +}
> +
> +static void test_visitor_in_int_str_noautocast(TestInputVisitorData *data,
> +                                               const void *unused)
> +{
> +    int64_t res = 0;
> +    Visitor *v;
> +    Error *err = NULL;
> +
> +    v = visitor_input_test_init(data, "\"-42\"");
> +
> +    visit_type_int(v, NULL, &res, &err);
> +    error_free_or_abort(&err);
> +}
> +
>  static void test_visitor_in_bool(TestInputVisitorData *data,
>                                   const void *unused)
>  {
> @@ -121,6 +183,44 @@ static void test_visitor_in_bool(TestInputVisitorData 
> *data,
>      g_assert_cmpint(res, ==, true);
>  }
>  
> +static void test_visitor_in_bool_autocast(TestInputVisitorData *data,
> +                                          const void *unused)
> +{
> +    bool res = false;
> +    Error *err = NULL;
> +    Visitor *v;
> +
> +    v = visitor_input_test_init_full(data, true, true, "true");
> +
> +    visit_type_bool(v, NULL, &res, &err);
> +    error_free_or_abort(&err);
> +}
> +
> +static void test_visitor_in_bool_str_autocast(TestInputVisitorData *data,
> +                                              const void *unused)
> +{
> +    bool res = false;
> +    Visitor *v;
> +
> +    v = visitor_input_test_init_full(data, true, true, "\"yes\"");
> +
> +    visit_type_bool(v, NULL, &res, &error_abort);
> +    g_assert_cmpint(res, ==, true);
> +}
> +
> +static void test_visitor_in_bool_str_noautocast(TestInputVisitorData *data,
> +                                                const void *unused)
> +{
> +    bool res = false;
> +    Visitor *v;
> +    Error *err = NULL;
> +
> +    v = visitor_input_test_init(data, "\"true\"");
> +
> +    visit_type_bool(v, NULL, &res, &err);
> +    error_free_or_abort(&err);
> +}
> +
>  static void test_visitor_in_number(TestInputVisitorData *data,
>                                     const void *unused)
>  {
> @@ -133,6 +233,69 @@ static void test_visitor_in_number(TestInputVisitorData 
> *data,
>      g_assert_cmpfloat(res, ==, value);
>  }
>  
> +static void test_visitor_in_number_autocast(TestInputVisitorData *data,
> +                                            const void *unused)
> +{
> +    double res = 0, value = 3.14;
> +    Error *err = NULL;
> +    Visitor *v;
> +
> +    v = visitor_input_test_init_full(data, true, true, "%f", value);
> +
> +    visit_type_number(v, NULL, &res, &err);
> +    error_free_or_abort(&err);
> +}
> +
> +static void test_visitor_in_number_str_autocast(TestInputVisitorData *data,
> +                                                const void *unused)
> +{
> +    double res = 0, value = 3.14;
> +    Visitor *v;
> +
> +    v = visitor_input_test_init_full(data, true, true, "\"3.14\"");
> +
> +    visit_type_number(v, NULL, &res, &error_abort);
> +    g_assert_cmpfloat(res, ==, value);
> +}
> +
> +static void test_visitor_in_number_str_noautocast(TestInputVisitorData *data,
> +                                                  const void *unused)
> +{
> +    double res = 0;
> +    Visitor *v;
> +    Error *err = NULL;
> +
> +    v = visitor_input_test_init(data, "\"3.14\"");
> +
> +    visit_type_number(v, NULL, &res, &err);
> +    error_free_or_abort(&err);
> +}
> +
> +static void test_visitor_in_size_str_autocast(TestInputVisitorData *data,
> +                                              const void *unused)
> +{
> +    uint64_t res, value = 500 * 1024 * 1024;
> +    Visitor *v;
> +
> +    v = visitor_input_test_init_full(data, true, true, "\"500M\"");
> +
> +    visit_type_size(v, NULL, &res, &error_abort);
> +    g_assert_cmpfloat(res, ==, value);
> +}
> +
> +static void test_visitor_in_size_str_noautocast(TestInputVisitorData *data,
> +                                                const void *unused)
> +{
> +    uint64_t res = 0;
> +    Visitor *v;
> +    Error *err = NULL;
> +
> +    v = visitor_input_test_init(data, "\"500M\"");
> +
> +    visit_type_size(v, NULL, &res, &err);
> +    error_free_or_abort(&err);
> +}
> +
>  static void test_visitor_in_string(TestInputVisitorData *data,
>                                     const void *unused)
>  {
> @@ -289,7 +452,8 @@ static void test_visitor_in_null(TestInputVisitorData 
> *data,
>       * when input is not null.
>       */
>  
> -    v = visitor_input_test_init(data, "{ 'a': null, 'b': '' }");
> +    v = visitor_input_test_init_full(data, false, false,
> +                                     "{ 'a': null, 'b': '' }");
>      visit_start_struct(v, NULL, NULL, 0, &error_abort);
>      visit_type_null(v, "a", &error_abort);
>      visit_type_str(v, "a", &tmp, &err);
> @@ -840,10 +1004,32 @@ int main(int argc, char **argv)
>                             NULL, test_visitor_in_int);
>      input_visitor_test_add("/visitor/input/int_overflow",
>                             NULL, test_visitor_in_int_overflow);
> +    input_visitor_test_add("/visitor/input/int_autocast",
> +                           NULL, test_visitor_in_int_autocast);
> +    input_visitor_test_add("/visitor/input/int_str_autocast",
> +                           NULL, test_visitor_in_int_str_autocast);
> +    input_visitor_test_add("/visitor/input/int_str_noautocast",
> +                           NULL, test_visitor_in_int_str_noautocast);
>      input_visitor_test_add("/visitor/input/bool",
>                             NULL, test_visitor_in_bool);
> +    input_visitor_test_add("/visitor/input/bool_autocast",
> +                           NULL, test_visitor_in_bool_autocast);
> +    input_visitor_test_add("/visitor/input/bool_str_autocast",
> +                           NULL, test_visitor_in_bool_str_autocast);
> +    input_visitor_test_add("/visitor/input/bool_str_noautocast",
> +                           NULL, test_visitor_in_bool_str_noautocast);
>      input_visitor_test_add("/visitor/input/number",
>                             NULL, test_visitor_in_number);
> +    input_visitor_test_add("/visitor/input/number_autocast",
> +                           NULL, test_visitor_in_number_autocast);
> +    input_visitor_test_add("/visitor/input/number_str_autocast",
> +                           NULL, test_visitor_in_number_str_autocast);
> +    input_visitor_test_add("/visitor/input/number_str_noautocast",
> +                           NULL, test_visitor_in_number_str_noautocast);
> +    input_visitor_test_add("/visitor/input/size_str_autocast",
> +                           NULL, test_visitor_in_size_str_autocast);
> +    input_visitor_test_add("/visitor/input/size_str_noautocast",
> +                           NULL, test_visitor_in_size_str_noautocast);
>      input_visitor_test_add("/visitor/input/string",
>                             NULL, test_visitor_in_string);
>      input_visitor_test_add("/visitor/input/enum",

I don't like the test code duplication.  The alternative would be a
function that rewrites all the scalars in a QObject to strings.
Probably not worthwhile.  We'll live with the duplication then.