[Qemu-devel] [PATCH v14 09/21] qapi: permit auto-creating single element lists

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

When converting QemuOpts to a QObject, there is no information
about compound types available, so when visiting a list, the
corresponding QObject is not guaranteed to be a QList. We
therefore need to be able to auto-create a single element QList
from whatever type we find.

This mode should only be enabled if you have compatibility
requirements for

   -arg foo=hello,foo=world

to be treated as equivalent to the preferred syntax:

   -arg foo.0=hello,foo.1=world

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 include/qapi/qobject-input-visitor.h | 20 +++++++-
 qapi/qobject-input-visitor.c         | 27 +++++++++--
 tests/test-qobject-input-visitor.c   | 88 +++++++++++++++++++++++++++++++-----
 3 files changed, 117 insertions(+), 18 deletions(-)

diff --git a/include/qapi/qobject-input-visitor.h b/include/qapi/qobject-input-visitor.h
index f134d90..1809f48 100644
--- a/include/qapi/qobject-input-visitor.h
+++ b/include/qapi/qobject-input-visitor.h
@@ -42,6 +42,19 @@ Visitor *qobject_input_visitor_new(QObject *obj, bool strict);
  * represented as strings. i.e. if visiting a boolean, the value should
  * be a QString whose contents represent a valid boolean.
  *
+ * If @autocreate_list is true, then as an alternative to a normal QList,
+ * list values can be stored as a QString or QDict instead, which will
+ * be interpreted as representing single element lists. This should only
+ * by used if compatibility is required with the OptsVisitor which allowed
+ * repeated keys, without list indexes, to represent lists. e.g. set this
+ * to true if you have compatibility requirements for
+ *
+ *   -arg foo=hello,foo=world
+ *
+ * to be treated as equivalent to the preferred syntax:
+ *
+ *   -arg foo.0=hello,foo.1=world
+ *
  * 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.
@@ -49,7 +62,8 @@ Visitor *qobject_input_visitor_new(QObject *obj, bool strict);
  * The returned input visitor should be released by calling
  * visit_free() when no longer required.
  */
-Visitor *qobject_input_visitor_new_autocast(QObject *obj);
+Visitor *qobject_input_visitor_new_autocast(QObject *obj,
+                                            bool autocreate_list);
 
 
 /**
@@ -64,6 +78,8 @@ Visitor *qobject_input_visitor_new_autocast(QObject *obj);
  * The returned input visitor should be released by calling
  * visit_free() when no longer required.
  */
-Visitor *qobject_input_visitor_new_opts(const QemuOpts *opts, Error **errp);
+Visitor *qobject_input_visitor_new_opts(const QemuOpts *opts,
+                                        bool autocreate_list,
+                                        Error **errp);
 
 #endif
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index d9269c9..d88e9f9 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -48,6 +48,10 @@ struct QObjectInputVisitor
 
     /* True to reject parse in visit_end_struct() if unvisited keys remain. */
     bool strict;
+
+    /* Whether we can auto-create single element lists when
+     * encountering a non-QList type */
+    bool autocreate_list;
 };
 
 static QObjectInputVisitor *to_qiv(Visitor *v)
@@ -108,6 +112,7 @@ static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv,
     assert(obj);
     tos->obj = obj;
     tos->qapi = qapi;
+    qobject_incref(obj);
 
     if (qiv->strict && qobject_type(obj) == QTYPE_QDICT) {
         h = g_hash_table_new(g_str_hash, g_str_equal);
@@ -147,6 +152,7 @@ static void qobject_input_stack_object_free(StackObject *tos)
     if (tos->h) {
         g_hash_table_unref(tos->h);
     }
+    qobject_decref(tos->obj);
 
     g_free(tos);
 }
@@ -197,7 +203,7 @@ static void qobject_input_start_list(Visitor *v, const char *name,
     QObject *qobj = qobject_input_get_object(qiv, name, true);
     const QListEntry *entry;
 
-    if (!qobj || qobject_type(qobj) != QTYPE_QLIST) {
+    if (!qobj || (!qiv->autocreate_list && qobject_type(qobj) != QTYPE_QLIST)) {
         if (list) {
             *list = NULL;
         }
@@ -206,7 +212,16 @@ static void qobject_input_start_list(Visitor *v, const char *name,
         return;
     }
 
-    entry = qobject_input_push(qiv, qobj, list, errp);
+    if (qobject_type(qobj) != QTYPE_QLIST) {
+        QList *tmplist = qlist_new();
+        qlist_append_obj(tmplist, qobj);
+        qobject_incref(qobj);
+        entry = qobject_input_push(qiv, QOBJECT(tmplist), list, errp);
+        QDECREF(tmplist);
+    } else {
+        entry = qobject_input_push(qiv, qobj, list, errp);
+    }
+
     if (list) {
         if (entry) {
             *list = g_malloc0(size);
@@ -514,7 +529,8 @@ Visitor *qobject_input_visitor_new(QObject *obj, bool strict)
     return &v->visitor;
 }
 
-Visitor *qobject_input_visitor_new_autocast(QObject *obj)
+Visitor *qobject_input_visitor_new_autocast(QObject *obj,
+                                            bool autocreate_list)
 {
     QObjectInputVisitor *v;
 
@@ -539,6 +555,7 @@ Visitor *qobject_input_visitor_new_autocast(QObject *obj)
     v->visitor.optional = qobject_input_optional;
     v->visitor.free = qobject_input_free;
     v->strict = true;
+    v->autocreate_list = autocreate_list;
 
     v->root = obj;
     qobject_incref(obj);
@@ -548,6 +565,7 @@ Visitor *qobject_input_visitor_new_autocast(QObject *obj)
 
 
 Visitor *qobject_input_visitor_new_opts(const QemuOpts *opts,
+                                        bool autocreate_list,
                                         Error **errp)
 {
     QDict *pdict;
@@ -564,7 +582,8 @@ Visitor *qobject_input_visitor_new_opts(const QemuOpts *opts,
         goto cleanup;
     }
 
-    v = qobject_input_visitor_new_autocast(pobj);
+    v = qobject_input_visitor_new_autocast(pobj,
+                                           autocreate_list);
  cleanup:
     qobject_decref(pobj);
     QDECREF(pdict);
diff --git a/tests/test-qobject-input-visitor.c b/tests/test-qobject-input-visitor.c
index d5b8044..c227565 100644
--- a/tests/test-qobject-input-visitor.c
+++ b/tests/test-qobject-input-visitor.c
@@ -42,6 +42,7 @@ static void visitor_input_teardown(TestInputVisitorData *data,
    functions (and not in main()). */
 static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data,
                                                  bool strict, bool autocast,
+                                                 bool autocreate_list,
                                                  const char *json_string,
                                                  va_list *ap)
 {
@@ -52,17 +53,20 @@ static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data,
 
     if (autocast) {
         assert(strict);
-        data->qiv = qobject_input_visitor_new_autocast(data->obj);
+        data->qiv = qobject_input_visitor_new_autocast(data->obj,
+                                                       autocreate_list);
     } else {
+        assert(!autocreate_list);
         data->qiv = qobject_input_visitor_new(data->obj, strict);
     }
     g_assert(data->qiv);
     return data->qiv;
 }
 
-static GCC_FMT_ATTR(4, 5)
+static GCC_FMT_ATTR(5, 6)
 Visitor *visitor_input_test_init_full(TestInputVisitorData *data,
                                       bool strict, bool autocast,
+                                      bool autocreate_list,
                                       const char *json_string, ...)
 {
     Visitor *v;
@@ -70,6 +74,7 @@ Visitor *visitor_input_test_init_full(TestInputVisitorData *data,
 
     va_start(ap, json_string);
     v = visitor_input_test_init_internal(data, strict, autocast,
+                                         autocreate_list,
                                          json_string, &ap);
     va_end(ap);
     return v;
@@ -83,7 +88,7 @@ Visitor *visitor_input_test_init(TestInputVisitorData *data,
     va_list ap;
 
     va_start(ap, json_string);
-    v = visitor_input_test_init_internal(data, true, false,
+    v = visitor_input_test_init_internal(data, true, false, false,
                                          json_string, &ap);
     va_end(ap);
     return v;
@@ -99,7 +104,7 @@ 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, true, false,
+    return visitor_input_test_init_internal(data, true, false, false,
                                             json_string, NULL);
 }
 
@@ -139,7 +144,7 @@ static void test_visitor_in_int_autocast(TestInputVisitorData *data,
     Error *err = NULL;
     Visitor *v;
 
-    v = visitor_input_test_init_full(data, true, true,
+    v = visitor_input_test_init_full(data, true, true, false,
                                      "%" PRId64, value);
     visit_type_int(v, NULL, &res, &err);
     error_free_or_abort(&err);
@@ -151,7 +156,7 @@ static void test_visitor_in_int_str_autocast(TestInputVisitorData *data,
     int64_t res = 0, value = -42;
     Visitor *v;
 
-    v = visitor_input_test_init_full(data, true, true,
+    v = visitor_input_test_init_full(data, true, true, false,
                                      "\"-42\"");
 
     visit_type_int(v, NULL, &res, &error_abort);
@@ -190,7 +195,7 @@ static void test_visitor_in_bool_autocast(TestInputVisitorData *data,
     Error *err = NULL;
     Visitor *v;
 
-    v = visitor_input_test_init_full(data, true, true, "true");
+    v = visitor_input_test_init_full(data, true, true, false, "true");
 
     visit_type_bool(v, NULL, &res, &err);
     error_free_or_abort(&err);
@@ -202,7 +207,7 @@ static void test_visitor_in_bool_str_autocast(TestInputVisitorData *data,
     bool res = false;
     Visitor *v;
 
-    v = visitor_input_test_init_full(data, true, true, "\"yes\"");
+    v = visitor_input_test_init_full(data, true, true, false, "\"yes\"");
 
     visit_type_bool(v, NULL, &res, &error_abort);
     g_assert_cmpint(res, ==, true);
@@ -240,7 +245,7 @@ static void test_visitor_in_number_autocast(TestInputVisitorData *data,
     Error *err = NULL;
     Visitor *v;
 
-    v = visitor_input_test_init_full(data, true, true, "%f", value);
+    v = visitor_input_test_init_full(data, true, true, false, "%f", value);
 
     visit_type_number(v, NULL, &res, &err);
     error_free_or_abort(&err);
@@ -252,7 +257,7 @@ static void test_visitor_in_number_str_autocast(TestInputVisitorData *data,
     double res = 0, value = 3.14;
     Visitor *v;
 
-    v = visitor_input_test_init_full(data, true, true, "\"3.14\"");
+    v = visitor_input_test_init_full(data, true, true, false, "\"3.14\"");
 
     visit_type_number(v, NULL, &res, &error_abort);
     g_assert_cmpfloat(res, ==, value);
@@ -277,7 +282,7 @@ static void test_visitor_in_size_str_autocast(TestInputVisitorData *data,
     uint64_t res, value = 500 * 1024 * 1024;
     Visitor *v;
 
-    v = visitor_input_test_init_full(data, true, true, "\"500M\"");
+    v = visitor_input_test_init_full(data, true, true, false, "\"500M\"");
 
     visit_type_size(v, NULL, &res, &error_abort);
     g_assert_cmpfloat(res, ==, value);
@@ -396,6 +401,59 @@ static void test_visitor_in_list(TestInputVisitorData *data,
     g_assert(!head);
 }
 
+static void test_visitor_in_list_autocreate_none(TestInputVisitorData *data,
+                                                 const void *unused)
+{
+    UserDefOneList *head = NULL;
+    Visitor *v;
+    Error *err = NULL;
+
+    v = visitor_input_test_init_full(data, true, true, false,
+                                     "{ 'string': 'string0', 'integer': 42 }");
+
+    visit_type_UserDefOneList(v, NULL, &head, &err);
+    error_free_or_abort(&err);
+    g_assert(head == NULL);
+}
+
+static void test_visitor_in_list_autocreate_dict(TestInputVisitorData *data,
+                                                 const void *unused)
+{
+    UserDefOneList *head = NULL;
+    Visitor *v;
+
+    v = visitor_input_test_init_full(data, true, true, true,
+                                     "{ 'string': 'string0', 'integer': '42' }");
+
+    visit_type_UserDefOneList(v, NULL, &head, &error_abort);
+    g_assert(head != NULL);
+
+    g_assert_cmpstr(head->value->string, ==, "string0");
+    g_assert_cmpint(head->value->integer, ==, 42);
+    g_assert(head->next == NULL);
+
+    qapi_free_UserDefOneList(head);
+    head = NULL;
+}
+
+static void test_visitor_in_list_autocreate_int(TestInputVisitorData *data,
+                                                const void *unused)
+{
+    uint32List *head = NULL;
+    Visitor *v;
+
+    /* Verify that we auto-create a single element list from the int */
+    v = visitor_input_test_init_full(data, true, true, true, "'42'");
+
+    visit_type_uint32List(v, NULL, &head, &error_abort);
+    g_assert(head != NULL);
+
+    g_assert_cmpint(head->value, ==, 42);
+
+    qapi_free_uint32List(head);
+    head = NULL;
+}
+
 static void test_visitor_in_any(TestInputVisitorData *data,
                                 const void *unused)
 {
@@ -452,7 +510,7 @@ static void test_visitor_in_null(TestInputVisitorData *data,
      * when input is not null.
      */
 
-    v = visitor_input_test_init_full(data, false, false,
+    v = visitor_input_test_init_full(data, false, false, false,
                                      "{ 'a': null, 'b': '' }");
     visit_start_struct(v, NULL, NULL, 0, &error_abort);
     visit_type_null(v, "a", &error_abort);
@@ -1040,6 +1098,12 @@ int main(int argc, char **argv)
                            NULL, test_visitor_in_struct_nested);
     input_visitor_test_add("/visitor/input/list",
                            NULL, test_visitor_in_list);
+    input_visitor_test_add("/visitor/input/list-autocreate-noautocast",
+                           NULL, test_visitor_in_list_autocreate_none);
+    input_visitor_test_add("/visitor/input/list-autocreate-autocast",
+                           NULL, test_visitor_in_list_autocreate_dict);
+    input_visitor_test_add("/visitor/input/list-autocreate-int",
+                           NULL, test_visitor_in_list_autocreate_int);
     input_visitor_test_add("/visitor/input/any",
                            NULL, test_visitor_in_any);
     input_visitor_test_add("/visitor/input/null",
-- 
2.7.4