[Qemu-devel] [PATCH v14 13/21] qdict: allow qdict_crumple to accept compound types as values

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

Currently qdict_crumple requires a totally flat QDict as its
input. i.e. all values in the QDict must be scalar types.

In order to have backwards compatibility with the OptsVisitor,
qemu_opt_to_qdict() has a new mode where it may return a QList
for values in the QDict, if there was a repeated key. We thus
need to allow compound types to appear as values in the input
dict given to qdict_crumple().

To avoid confusion, we sanity check that the user has not mixed
the old and new syntax at the same time. e.g. these are allowed

   foo=hello,foo=world,foo=wibble
   foo.0=hello,foo.1=world,foo.2=wibble

but this is forbidden

   foo=hello,foo=world,foo.2=wibble

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 qobject/qdict.c     |  45 +++++++++-----
 tests/check-qdict.c | 175 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 204 insertions(+), 16 deletions(-)

diff --git a/qobject/qdict.c b/qobject/qdict.c
index c38e90e..83384b2 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -814,15 +814,16 @@ static int qdict_is_list(QDict *maybe_list, Error **errp)
 
 /**
  * qdict_crumple:
- * @src: the original flat dictionary (only scalar values) to crumple
+ * @src: the original dictionary to crumple
  * @recursive: true to recursively crumple nested dictionaries
  *
- * Takes a flat dictionary whose keys use '.' separator to indicate
- * nesting, and values are scalars, and crumples it into a nested
- * structure. If the @recursive parameter is false, then only the
- * first level of structure implied by the keys will be crumpled. If
- * @recursive is true, then the input will be recursively crumpled to
- * expand all levels of structure in the keys.
+ * Takes a dictionary whose keys use '.' separator to indicate
+ * nesting, crumples it into a nested structure. The values in
+ * @src are permitted to be scalars or compound typee. If the
+ * @recursive parameter is false, then only the first level of
+ * structure implied by the keys will be crumpled. If @recursive
+ * is true, then the input will be recursively crumpled to expand
+ * all levels of structure in the keys.
  *
  * To include a literal '.' in a key name, it must be escaped as '..'
  *
@@ -843,7 +844,10 @@ static int qdict_is_list(QDict *maybe_list, Error **errp)
  * The following scenarios in the input dict will result in an
  * error being returned:
  *
- *  - Any values in @src are non-scalar types
+ *  - If a key in @src has a value that is non-scalar, and
+ *    a later key implies creation of a compound type. e.g.
+ *    if 'foo' point to a 'QList' or 'QDict', then it is
+ *    not permitted to also have 'foo.NNN' keys later.
  *  - If keys in @src imply that a particular level is both a
  *    list and a dict. e.g., "foo.0.bar" and "foo.eek.bar".
  *  - If keys in @src imply that a particular level is a list,
@@ -864,21 +868,26 @@ QObject *qdict_crumple(const QDict *src, bool recursive, Error **errp)
     char *prefix = NULL;
     const char *suffix = NULL;
     int is_list;
+    QDict *compound;
 
+    compound = qdict_new();
     two_level = qdict_new();
 
     /* Step 1: split our totally flat dict into a two level dict */
     for (ent = qdict_first(src); ent != NULL; ent = qdict_next(src, ent)) {
-        if (qobject_type(ent->value) == QTYPE_QDICT ||
-            qobject_type(ent->value) == QTYPE_QLIST) {
-            error_setg(errp, "Value %s is not a scalar",
-                       ent->key);
-            goto error;
-        }
-
         qdict_split_flat_key(ent->key, &prefix, &suffix);
 
         child = qdict_get(two_level, prefix);
+
+        if (qdict_haskey(compound, prefix)) {
+            error_setg(errp, "Key %s is already set as a list/dict", prefix);
+            goto error;
+        }
+        if (qobject_type(ent->value) == QTYPE_QLIST ||
+            qobject_type(ent->value) == QTYPE_QDICT) {
+            qobject_incref(ent->value);
+            qdict_put_obj(compound, prefix, ent->value);
+        }
         if (suffix) {
             if (child) {
                 if (qobject_type(child) != QTYPE_QDICT) {
@@ -913,7 +922,8 @@ QObject *qdict_crumple(const QDict *src, bool recursive, Error **errp)
         for (ent = qdict_first(two_level); ent != NULL;
              ent = qdict_next(two_level, ent)) {
 
-            if (qobject_type(ent->value) == QTYPE_QDICT) {
+            if (qobject_type(ent->value) == QTYPE_QDICT &&
+                !qdict_haskey(compound, ent->key)) {
                 child = qdict_crumple(qobject_to_qdict(ent->value),
                                       recursive, errp);
                 if (!child) {
@@ -961,10 +971,13 @@ QObject *qdict_crumple(const QDict *src, bool recursive, Error **errp)
         dst = QOBJECT(multi_level);
     }
 
+    QDECREF(compound);
+
     return dst;
 
  error:
     g_free(prefix);
+    QDECREF(compound);
     QDECREF(multi_level);
     QDECREF(two_level);
     qobject_decref(dst);
diff --git a/tests/check-qdict.c b/tests/check-qdict.c
index 64c33ab..29d7362 100644
--- a/tests/check-qdict.c
+++ b/tests/check-qdict.c
@@ -844,6 +844,173 @@ static void qdict_crumple_test_bad_inputs(void)
     QDECREF(src);
 }
 
+static void qdict_crumple_test_non_flat_list_ok(void)
+{
+    QDict *src, *dst, *vnc, *listen;
+    QObject *child, *res;
+    QList *list;
+    QString *match;
+
+    src = qdict_new();
+    list = qlist_new();
+    qlist_append(list, qstring_from_str("fred"));
+    qlist_append(list, qstring_from_str("bob"));
+
+    qdict_put(src, "vnc.listen.addr", qstring_from_str("127.0.0.1"));
+    qdict_put(src, "vnc.listen.port", qstring_from_str("5901"));
+    qdict_put(src, "match", list);
+
+    res = qdict_crumple(src, true, &error_abort);
+
+    g_assert_cmpint(qobject_type(res), ==, QTYPE_QDICT);
+
+    dst = qobject_to_qdict(res);
+
+    g_assert_cmpint(qdict_size(dst), ==, 2);
+
+    child = qdict_get(dst, "vnc");
+    g_assert_cmpint(qobject_type(child), ==, QTYPE_QDICT);
+    vnc = qobject_to_qdict(child);
+
+    child = qdict_get(vnc, "listen");
+    g_assert_cmpint(qobject_type(child), ==, QTYPE_QDICT);
+    listen = qobject_to_qdict(child);
+    g_assert_cmpstr("127.0.0.1", ==, qdict_get_str(listen, "addr"));
+    g_assert_cmpstr("5901", ==, qdict_get_str(listen, "port"));
+
+    child = qdict_get(dst, "match");
+    g_assert_cmpint(qobject_type(child), ==, QTYPE_QLIST);
+    list = qobject_to_qlist(child);
+
+    g_assert_cmpint(qlist_size(list), ==, 2);
+
+    child = qlist_pop(list);
+    g_assert(child);
+    match = qobject_to_qstring(child);
+    g_assert_cmpstr("fred", ==, qstring_get_str(match));
+    qobject_decref(child);
+
+    child = qlist_pop(list);
+    g_assert(child);
+    match = qobject_to_qstring(child);
+    g_assert_cmpstr("bob", ==, qstring_get_str(match));
+    qobject_decref(child);
+
+    child = qlist_pop(list);
+    g_assert(!child);
+
+    QDECREF(src);
+    QDECREF(dst);
+}
+
+
+static void qdict_crumple_test_non_flat_list_bad(void)
+{
+    QDict *src;
+    QObject *res;
+    QList *list;
+    Error *err = NULL;
+
+    src = qdict_new();
+    list = qlist_new();
+    qlist_append(list, qstring_from_str("fred"));
+    qlist_append(list, qstring_from_str("bob"));
+
+    qdict_put(src, "vnc.listen.addr", qstring_from_str("127.0.0.1"));
+    qdict_put(src, "vnc.listen.port", qstring_from_str("5901"));
+    qdict_put(src, "match.0", qstring_from_str("frank"));
+    qdict_put(src, "match", list);
+
+    res = qdict_crumple(src, false, &err);
+    g_assert(!res);
+    error_free_or_abort(&err);
+
+    QDECREF(src);
+}
+
+
+static void qdict_crumple_test_non_flat_dict_ok(void)
+{
+    QDict *src, *dst, *rule, *vnc, *acl;
+    QObject *child, *res;
+    QList *rules;
+
+    vnc = qdict_new();
+    qdict_put(vnc, "listen.addr", qstring_from_str("127.0.0.1"));
+    qdict_put(vnc, "listen.port", qstring_from_str("5901"));
+
+    src = qdict_new();
+    qdict_put(src, "vnc", vnc);
+    qdict_put(src, "acl.rules.0.match", qstring_from_str("fred"));
+    qdict_put(src, "acl.rules.0.policy", qstring_from_str("allow"));
+    qdict_put(src, "acl.rules.1.match", qstring_from_str("bob"));
+    qdict_put(src, "acl.rules.1.policy", qstring_from_str("deny"));
+
+    res = qdict_crumple(src, true, &error_abort);
+
+    g_assert_cmpint(qobject_type(res), ==, QTYPE_QDICT);
+
+    dst = qobject_to_qdict(res);
+
+    g_assert_cmpint(qdict_size(dst), ==, 2);
+
+    child = qdict_get(dst, "vnc");
+    g_assert_cmpint(qobject_type(child), ==, QTYPE_QDICT);
+    vnc = qobject_to_qdict(child);
+
+    g_assert_cmpstr("127.0.0.1", ==, qdict_get_str(vnc, "listen.addr"));
+    g_assert_cmpstr("5901", ==, qdict_get_str(vnc, "listen.port"));
+
+    child = qdict_get(dst, "acl");
+    g_assert_cmpint(qobject_type(child), ==, QTYPE_QDICT);
+    acl = qobject_to_qdict(child);
+
+    child = qdict_get(acl, "rules");
+    g_assert_cmpint(qobject_type(child), ==, QTYPE_QLIST);
+    rules = qobject_to_qlist(child);
+    g_assert_cmpint(qlist_size(rules), ==, 2);
+
+    rule = qobject_to_qdict(qlist_pop(rules));
+    g_assert_cmpint(qdict_size(rule), ==, 2);
+    g_assert_cmpstr("fred", ==, qdict_get_str(rule, "match"));
+    g_assert_cmpstr("allow", ==, qdict_get_str(rule, "policy"));
+    QDECREF(rule);
+
+    rule = qobject_to_qdict(qlist_pop(rules));
+    g_assert_cmpint(qdict_size(rule), ==, 2);
+    g_assert_cmpstr("bob", ==, qdict_get_str(rule, "match"));
+    g_assert_cmpstr("deny", ==, qdict_get_str(rule, "policy"));
+    QDECREF(rule);
+
+    QDECREF(src);
+    QDECREF(dst);
+}
+
+
+static void qdict_crumple_test_non_flat_dict_bad(void)
+{
+    QDict *src, *vnc;
+    QObject *res;
+    Error *err = NULL;
+
+    vnc = qdict_new();
+    qdict_put(vnc, "listen.addr", qstring_from_str("127.0.0.1"));
+
+    src = qdict_new();
+    qdict_put(src, "vnc", vnc);
+    qdict_put(src, "vnc.listen.port", qstring_from_str("5901"));
+    qdict_put(src, "acl.rules.0.match", qstring_from_str("fred"));
+    qdict_put(src, "acl.rules.0.policy", qstring_from_str("allow"));
+    qdict_put(src, "acl.rules.1.match", qstring_from_str("bob"));
+    qdict_put(src, "acl.rules.1.policy", qstring_from_str("deny"));
+
+    res = qdict_crumple(src, true, &err);
+    g_assert(!res);
+    error_free_or_abort(&err);
+
+    QDECREF(src);
+}
+
 
 /*
  * Errors test-cases
@@ -1002,6 +1169,14 @@ int main(int argc, char **argv)
                     qdict_crumple_test_empty);
     g_test_add_func("/public/crumple/bad_inputs",
                     qdict_crumple_test_bad_inputs);
+    g_test_add_func("/public/crumple/non-flat-list-ok",
+                    qdict_crumple_test_non_flat_list_ok);
+    g_test_add_func("/public/crumple/non-flat-list-bad",
+                    qdict_crumple_test_non_flat_list_bad);
+    g_test_add_func("/public/crumple/non-flat-dict-ok",
+                    qdict_crumple_test_non_flat_dict_ok);
+    g_test_add_func("/public/crumple/non-flat-dict-bad",
+                    qdict_crumple_test_non_flat_dict_bad);
 
     /* The Big one */
     if (g_test_slow()) {
-- 
2.7.4