[Qemu-devel] [PATCH v7 3/7] qapi: add a QmpInputVisitor that does string conversion

["Daniel P. Berrange" <berrange@redhat.com>]

Currently the QmpInputVisitor assumes that all scalar
values are directly represented as their final types.
ie it assumes an 'int' is using QInt, and a 'bool' is
using QBool.

This adds an alternative constructor for QmpInputVisitor
that will set it up such that it expects a QString for
all scalar types instead.

This makes it possible to use QmpInputVisitor with a
QDict produced from QemuOpts, where everything is in
string format.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 include/qapi/qmp-input-visitor.h |  42 +++++++++--
 qapi/qmp-input-visitor.c         | 105 +++++++++++++++++++++++++++
 tests/test-qmp-input-visitor.c   | 149 ++++++++++++++++++++++++++++++++++++++-
 3 files changed, 289 insertions(+), 7 deletions(-)

diff --git a/include/qapi/qmp-input-visitor.h b/include/qapi/qmp-input-visitor.h
index b0624d8..ed09a90 100644
--- a/include/qapi/qmp-input-visitor.h
+++ b/include/qapi/qmp-input-visitor.h
@@ -19,14 +19,48 @@
 
 typedef struct QmpInputVisitor QmpInputVisitor;
 
-/*
- * Return a new input visitor that converts QMP to QAPI.
+/**
+ * qmp_input_visitor_new:
+ * @obj: the input object to visit
+ * @strict: whether to require that all input keys are consumed
+ *
+ * Create a new input visitor that converts QMP to QAPI.
+ *
+ * Any scalar values in the @obj input data structure should be in the
+ * required type already. ie 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.
+ *
+ * The returned input visitor should be released by calling
+ * qmp_input_visitor_cleanup when no longer required.
+ *
+ * Returns: a new input visitor
  */
 QmpInputVisitor *qmp_input_visitor_new(QObject *obj, bool strict);
 
+/**
+ * qmp_string_input_visitor_new:
+ * @obj: the input object to visit
+ * @strict: whether to require that all input keys are consumed
+ *
+ * Create a new input visitor that converts QMP to QAPI.
+ *
+ * Any scalar values in the @obj input data structure should always be
+ * represented as strings. ie if visiting a boolean, the value should
+ * be a QString whose contents represent a valid boolean.
+ *
+ * If @strict is set to true, then an error will be reported if any
+ * dict keys are not consumed during visitation.
+ *
+ * The returned input visitor should be released by calling
+ * qmp_input_visitor_cleanup when no longer required.
+ *
+ * Returns: a new input visitor
+ */
+QmpInputVisitor *qmp_string_input_visitor_new(QObject *obj, bool strict);
+
 void qmp_input_visitor_cleanup(QmpInputVisitor *v);
 
 Visitor *qmp_input_get_visitor(QmpInputVisitor *v);
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index aea90a1..38de75d 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-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
 
@@ -265,6 +266,17 @@ static void qmp_input_type_int64(Visitor *v, const char *name, int64_t *obj,
     *obj = qint_get_int(qint);
 }
 
+static void qmp_input_type_int64_str(Visitor *v, const char *name, int64_t *obj,
+                                     Error **errp)
+{
+    QmpInputVisitor *qiv = to_qiv(v);
+    QString *qstr = qobject_to_qstring(qmp_input_get_object(qiv, name, true));
+    uint64_t ret;
+
+    parse_option_number(name, qstr ? qstr->string : NULL, &ret, errp);
+    *obj = ret;
+}
+
 static void qmp_input_type_uint64(Visitor *v, const char *name, uint64_t *obj,
                                   Error **errp)
 {
@@ -281,6 +293,15 @@ static void qmp_input_type_uint64(Visitor *v, const char *name, uint64_t *obj,
     *obj = qint_get_int(qint);
 }
 
+static void qmp_input_type_uint64_str(Visitor *v, const char *name,
+                                      uint64_t *obj, Error **errp)
+{
+    QmpInputVisitor *qiv = to_qiv(v);
+    QString *qstr = qobject_to_qstring(qmp_input_get_object(qiv, name, true));
+
+    parse_option_number(name, qstr ? qstr->string : NULL, obj, errp);
+}
+
 static void qmp_input_type_bool(Visitor *v, const char *name, bool *obj,
                                 Error **errp)
 {
@@ -296,6 +317,15 @@ static void qmp_input_type_bool(Visitor *v, const char *name, bool *obj,
     *obj = qbool_get_bool(qbool);
 }
 
+static void qmp_input_type_bool_str(Visitor *v, const char *name, bool *obj,
+                                    Error **errp)
+{
+    QmpInputVisitor *qiv = to_qiv(v);
+    QString *qstr = qobject_to_qstring(qmp_input_get_object(qiv, name, true));
+
+    parse_option_bool(name, qstr ? qstr->string : NULL, obj, errp);
+}
+
 static void qmp_input_type_str(Visitor *v, const char *name, char **obj,
                                Error **errp)
 {
@@ -336,6 +366,25 @@ static void qmp_input_type_number(Visitor *v, const char *name, double *obj,
                "number");
 }
 
+static void qmp_input_type_number_str(Visitor *v, const char *name, double *obj,
+                                      Error **errp)
+{
+    QmpInputVisitor *qiv = to_qiv(v);
+    QString *qstr = qobject_to_qstring(qmp_input_get_object(qiv, name, true));
+    char *endp;
+
+    if (qstr && qstr->string) {
+        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");
+}
+
 static void qmp_input_type_any(Visitor *v, const char *name, QObject **obj,
                                Error **errp)
 {
@@ -357,6 +406,31 @@ static void qmp_input_type_null(Visitor *v, const char *name, Error **errp)
     }
 }
 
+static void qmp_input_type_size_str(Visitor *v, const char *name, uint64_t *obj,
+                                    Error **errp)
+{
+    QmpInputVisitor *qiv = to_qiv(v);
+    QString *qstr = qobject_to_qstring(qmp_input_get_object(qiv, name, true));
+    int64_t val;
+    char *endptr;
+
+    if (qstr && qstr->string) {
+        val = qemu_strtosz_suffix(qstr->string, &endptr,
+                                  QEMU_STRTOSZ_DEFSUFFIX_B);
+        if (val < 0 || *endptr) {
+            error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
+                       "a size value representible as a non-negative int64");
+            return;
+        }
+
+        *obj = val;
+        return;
+    }
+
+    error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
+               "size");
+}
+
 static void qmp_input_optional(Visitor *v, const char *name, bool *present)
 {
     QmpInputVisitor *qiv = to_qiv(v);
@@ -410,3 +484,34 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj, bool strict)
 
     return v;
 }
+
+QmpInputVisitor *qmp_string_input_visitor_new(QObject *obj, bool strict)
+{
+    QmpInputVisitor *v;
+
+    v = g_malloc0(sizeof(*v));
+
+    v->visitor.type = VISITOR_INPUT;
+    v->visitor.start_struct = qmp_input_start_struct;
+    v->visitor.check_struct = qmp_input_check_struct;
+    v->visitor.end_struct = qmp_input_pop;
+    v->visitor.start_list = qmp_input_start_list;
+    v->visitor.next_list = qmp_input_next_list;
+    v->visitor.end_list = qmp_input_pop;
+    v->visitor.start_alternate = qmp_input_start_alternate;
+    v->visitor.type_int64 = qmp_input_type_int64_str;
+    v->visitor.type_uint64 = qmp_input_type_uint64_str;
+    v->visitor.type_bool = qmp_input_type_bool_str;
+    v->visitor.type_str = qmp_input_type_str;
+    v->visitor.type_number = qmp_input_type_number_str;
+    v->visitor.type_any = qmp_input_type_any;
+    v->visitor.type_null = qmp_input_type_null;
+    v->visitor.type_size = qmp_input_type_size_str;
+    v->visitor.optional = qmp_input_optional;
+    v->strict = strict;
+
+    v->root = obj;
+    qobject_incref(obj);
+
+    return v;
+}
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 3b6b39e..59bbb4a 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -40,6 +40,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)
 {
@@ -50,7 +51,11 @@ static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data,
     data->obj = qobject_from_jsonv(json_string, ap);
     g_assert(data->obj);
 
-    data->qiv = qmp_input_visitor_new(data->obj, false);
+    if (autocast) {
+        data->qiv = qmp_string_input_visitor_new(data->obj, strict);
+    } else {
+        data->qiv = qmp_input_visitor_new(data->obj, strict);
+    }
     g_assert(data->qiv);
 
     v = qmp_input_get_visitor(data->qiv);
@@ -59,6 +64,21 @@ static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data,
     return v;
 }
 
+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, ...)
@@ -67,7 +87,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, false, false,
+                                         json_string, &ap);
     va_end(ap);
     return v;
 }
@@ -82,7 +103,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, false, false,
+                                            json_string, NULL);
 }
 
 static void test_visitor_in_int(TestInputVisitorData *data,
@@ -114,6 +136,33 @@ 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;
+    Visitor *v;
+
+    v = visitor_input_test_init_full(data, false, true,
+                                     "\"-42\"");
+
+    visit_type_int(v, NULL, &res, &error_abort);
+    g_assert_cmpint(res, ==, value);
+}
+
+static void test_visitor_in_int_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);
+    g_assert(err != NULL);
+    error_free(err);
+}
+
 static void test_visitor_in_bool(TestInputVisitorData *data,
                                  const void *unused)
 {
@@ -126,6 +175,32 @@ 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;
+    Visitor *v;
+
+    v = visitor_input_test_init_full(data, false, true, "\"yes\"");
+
+    visit_type_bool(v, NULL, &res, &error_abort);
+    g_assert_cmpint(res, ==, true);
+}
+
+static void test_visitor_in_bool_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);
+    g_assert(err != NULL);
+    error_free(err);
+}
+
 static void test_visitor_in_number(TestInputVisitorData *data,
                                    const void *unused)
 {
@@ -138,6 +213,58 @@ 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;
+    Visitor *v;
+
+    v = visitor_input_test_init_full(data, false, true, "\"3.14\"");
+
+    visit_type_number(v, NULL, &res, &error_abort);
+    g_assert_cmpfloat(res, ==, value);
+}
+
+static void test_visitor_in_number_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);
+    g_assert(err != NULL);
+    error_free(err);
+}
+
+static void test_visitor_in_size_autocast(TestInputVisitorData *data,
+                                            const void *unused)
+{
+    uint64_t res, value = 500 * 1024 * 1024;
+    Visitor *v;
+
+    v = visitor_input_test_init_full(data, false, true, "\"500M\"");
+
+    visit_type_size(v, NULL, &res, &error_abort);
+    g_assert_cmpfloat(res, ==, value);
+}
+
+static void test_visitor_in_size_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);
+    g_assert(err != NULL);
+    error_free(err);
+}
+
 static void test_visitor_in_string(TestInputVisitorData *data,
                                    const void *unused)
 {
@@ -834,10 +961,26 @@ int main(int argc, char **argv)
                            &in_visitor_data, test_visitor_in_int);
     input_visitor_test_add("/visitor/input/int_overflow",
                            &in_visitor_data, test_visitor_in_int_overflow);
+    input_visitor_test_add("/visitor/input/int_autocast",
+                           &in_visitor_data, test_visitor_in_int_autocast);
+    input_visitor_test_add("/visitor/input/int_noautocast",
+                           &in_visitor_data, test_visitor_in_int_noautocast);
     input_visitor_test_add("/visitor/input/bool",
                            &in_visitor_data, test_visitor_in_bool);
+    input_visitor_test_add("/visitor/input/bool_autocast",
+                           &in_visitor_data, test_visitor_in_bool_autocast);
+    input_visitor_test_add("/visitor/input/bool_noautocast",
+                           &in_visitor_data, test_visitor_in_bool_noautocast);
     input_visitor_test_add("/visitor/input/number",
                            &in_visitor_data, test_visitor_in_number);
+    input_visitor_test_add("/visitor/input/number_autocast",
+                           &in_visitor_data, test_visitor_in_number_autocast);
+    input_visitor_test_add("/visitor/input/number_noautocast",
+                           &in_visitor_data, test_visitor_in_number_noautocast);
+    input_visitor_test_add("/visitor/input/size_autocast",
+                           &in_visitor_data, test_visitor_in_size_autocast);
+    input_visitor_test_add("/visitor/input/size_noautocast",
+                           &in_visitor_data, test_visitor_in_size_noautocast);
     input_visitor_test_add("/visitor/input/string",
                            &in_visitor_data, test_visitor_in_string);
     input_visitor_test_add("/visitor/input/enum",
-- 
2.7.4