Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2016-05-12' into staging

QAPI patches for 2016-05-12

# gpg: Signature made Thu 12 May 2016 08:49:04 BST using RSA key ID EB918653
# gpg: Good signature from "Markus Armbruster <armbru@redhat.com>"
# gpg:                 aka "Markus Armbruster <armbru@pond.sub.org>"

* remotes/armbru/tags/pull-qapi-2016-05-12: (23 commits)
  qapi: Change visit_type_FOO() to no longer return partial objects
  qapi: Simplify semantics of visit_next_list()
  qapi: Fix string input visitor handling of invalid list
  tests/string-input-visitor: Add negative integer tests
  qapi: Split visit_end_struct() into pieces
  qmp: Tighten output visitor rules
  qmp: Don't reuse qmp visitor after grabbing output
  spapr_drc: Expose 'null' in qom-get when there is no fdt
  qmp: Support explicit null during visits
  qapi: Add visit_type_null() visitor
  tests: Add check-qnull
  qapi: Document visitor interfaces, add assertions
  qmp-input: Refactor when list is advanced
  qmp-input: Require struct push to visit members of top dict
  qom: Wrap prop visit in visit_start_struct
  qapi-commands: Wrap argument visit in visit_start_struct
  qmp-input: Don't consume input when checking has_member
  qapi: Use strict QMP input visitor in more places
  qapi: Consolidate QMP input visitor creation
  qmp-input: Clean up stack handling
  ...

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

9 files changed, 176 insertions, 44 deletions

diff --git a/tests/.gitignore b/tests/.gitignore
index 9eed22988b..a06a8ba26f 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -3,6 +3,7 @@ check-qfloat
 check-qint
 check-qjson
 check-qlist
+check-qnull
 check-qstring
 check-qom-interface
 check-qom-proplist
diff --git a/tests/Makefile b/tests/Makefile
index 9194f1850b..9dddde6589 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -16,6 +16,8 @@ check-unit-y += tests/check-qstring$(EXESUF)
 gcov-files-check-qstring-y = qobject/qstring.c
 check-unit-y += tests/check-qlist$(EXESUF)
 gcov-files-check-qlist-y = qobject/qlist.c
+check-unit-y += tests/check-qnull$(EXESUF)
+gcov-files-check-qnull-y = qobject/qnull.c
 check-unit-y += tests/check-qjson$(EXESUF)
 gcov-files-check-qjson-y = qobject/qjson.c
 check-unit-y += tests/test-qmp-output-visitor$(EXESUF)
@@ -382,7 +384,8 @@ GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h \
 	tests/test-qmp-introspect.h
 
 test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
-	tests/check-qlist.o tests/check-qfloat.o tests/check-qjson.o \
+	tests/check-qlist.o tests/check-qfloat.o tests/check-qnull.o \
+	tests/check-qjson.o \
 	tests/test-coroutine.o tests/test-string-output-visitor.o \
 	tests/test-string-input-visitor.o tests/test-qmp-output-visitor.o \
 	tests/test-qmp-input-visitor.o tests/test-qmp-input-strict.o \
@@ -410,6 +413,7 @@ tests/check-qstring$(EXESUF): tests/check-qstring.o $(test-util-obj-y)
 tests/check-qdict$(EXESUF): tests/check-qdict.o $(test-util-obj-y)
 tests/check-qlist$(EXESUF): tests/check-qlist.o $(test-util-obj-y)
 tests/check-qfloat$(EXESUF): tests/check-qfloat.o $(test-util-obj-y)
+tests/check-qnull$(EXESUF): tests/check-qnull.o $(test-util-obj-y)
 tests/check-qjson$(EXESUF): tests/check-qjson.o $(test-util-obj-y)
 tests/check-qom-interface$(EXESUF): tests/check-qom-interface.o $(test-qom-obj-y)
 tests/check-qom-proplist$(EXESUF): tests/check-qom-proplist.o $(test-qom-obj-y)
diff --git a/tests/check-qnull.c b/tests/check-qnull.c
new file mode 100644
index 0000000000..fd9c68f7e1
--- /dev/null
+++ b/tests/check-qnull.c
@@ -0,0 +1,75 @@
+/*
+ * QNull unit-tests.
+ *
+ * Copyright (C) 2016 Red Hat Inc.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ */
+#include "qemu/osdep.h"
+#include <glib.h>
+
+#include "qapi/qmp/qobject.h"
+#include "qemu-common.h"
+#include "qapi/qmp-input-visitor.h"
+#include "qapi/qmp-output-visitor.h"
+#include "qapi/error.h"
+
+/*
+ * Public Interface test-cases
+ *
+ * (with some violations to access 'private' data)
+ */
+
+static void qnull_ref_test(void)
+{
+    QObject *obj;
+
+    g_assert(qnull_.refcnt == 1);
+    obj = qnull();
+    g_assert(obj);
+    g_assert(obj == &qnull_);
+    g_assert(qnull_.refcnt == 2);
+    g_assert(qobject_type(obj) == QTYPE_QNULL);
+    qobject_decref(obj);
+    g_assert(qnull_.refcnt == 1);
+}
+
+static void qnull_visit_test(void)
+{
+    QObject *obj;
+    QmpOutputVisitor *qov;
+    QmpInputVisitor *qiv;
+
+    /*
+     * Most tests of interactions between QObject and visitors are in
+     * test-qmp-*-visitor; but these tests live here because they
+     * depend on layering violations to check qnull_ refcnt.
+     */
+
+    g_assert(qnull_.refcnt == 1);
+    obj = qnull();
+    qiv = qmp_input_visitor_new(obj, true);
+    qobject_decref(obj);
+    visit_type_null(qmp_input_get_visitor(qiv), NULL, &error_abort);
+    qmp_input_visitor_cleanup(qiv);
+
+    qov = qmp_output_visitor_new();
+    visit_type_null(qmp_output_get_visitor(qov), NULL, &error_abort);
+    obj = qmp_output_get_qobject(qov);
+    g_assert(obj == &qnull_);
+    qobject_decref(obj);
+    qmp_output_visitor_cleanup(qov);
+
+    g_assert(qnull_.refcnt == 1);
+}
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+
+    g_test_add_func("/public/qnull_ref", qnull_ref_test);
+    g_test_add_func("/public/qnull_visit", qnull_visit_test);
+
+    return g_test_run();
+}
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index 14a9ebbd5a..5c3edd753a 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -222,20 +222,19 @@ static void test_dealloc_partial(void)
         ud2_dict = qdict_new();
         qdict_put_obj(ud2_dict, "string0", QOBJECT(qstring_from_str(text)));
 
-        qiv = qmp_input_visitor_new(QOBJECT(ud2_dict));
+        qiv = qmp_input_visitor_new(QOBJECT(ud2_dict), true);
         visit_type_UserDefTwo(qmp_input_get_visitor(qiv), NULL, &ud2, &err);
         qmp_input_visitor_cleanup(qiv);
         QDECREF(ud2_dict);
     }
 
-    /* verify partial success */
-    assert(ud2 != NULL);
-    assert(ud2->string0 != NULL);
-    assert(strcmp(ud2->string0, text) == 0);
-    assert(ud2->dict1 == NULL);
-
-    /* confirm & release construction error */
+    /* verify that visit_type_XXX() cleans up properly on error */
     error_free_or_abort(&err);
+    assert(!ud2);
+
+    /* Manually create a partial object, leaving ud2->dict1 at NULL */
+    ud2 = g_new0(UserDefTwo, 1);
+    ud2->string0 = g_strdup(text);
 
     /* tear down partial object */
     qapi_free_UserDefTwo(ud2);
diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
index d71727e272..4602529ea0 100644
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qmp-input-strict.c
@@ -55,7 +55,7 @@ static Visitor *validate_test_init_internal(TestInputVisitorData *data,
     data->obj = qobject_from_jsonv(json_string, ap);
     g_assert(data->obj);
 
-    data->qiv = qmp_input_visitor_new_strict(data->obj);
+    data->qiv = qmp_input_visitor_new(data->obj, true);
     g_assert(data->qiv);
 
     v = qmp_input_get_visitor(data->qiv);
@@ -182,10 +182,7 @@ static void test_validate_fail_struct(TestInputVisitorData *data,
 
     visit_type_TestStruct(v, NULL, &p, &err);
     error_free_or_abort(&err);
-    if (p) {
-        g_free(p->string);
-    }
-    g_free(p);
+    g_assert(!p);
 }
 
 static void test_validate_fail_struct_nested(TestInputVisitorData *data,
@@ -199,7 +196,7 @@ static void test_validate_fail_struct_nested(TestInputVisitorData *data,
 
     visit_type_UserDefTwo(v, NULL, &udp, &err);
     error_free_or_abort(&err);
-    qapi_free_UserDefTwo(udp);
+    g_assert(!udp);
 }
 
 static void test_validate_fail_list(TestInputVisitorData *data,
@@ -213,7 +210,7 @@ static void test_validate_fail_list(TestInputVisitorData *data,
 
     visit_type_UserDefOneList(v, NULL, &head, &err);
     error_free_or_abort(&err);
-    qapi_free_UserDefOneList(head);
+    g_assert(!head);
 }
 
 static void test_validate_fail_union_native_list(TestInputVisitorData *data,
@@ -228,7 +225,7 @@ static void test_validate_fail_union_native_list(TestInputVisitorData *data,
 
     visit_type_UserDefNativeListUnion(v, NULL, &tmp, &err);
     error_free_or_abort(&err);
-    qapi_free_UserDefNativeListUnion(tmp);
+    g_assert(!tmp);
 }
 
 static void test_validate_fail_union_flat(TestInputVisitorData *data,
@@ -242,7 +239,7 @@ static void test_validate_fail_union_flat(TestInputVisitorData *data,
 
     visit_type_UserDefFlatUnion(v, NULL, &tmp, &err);
     error_free_or_abort(&err);
-    qapi_free_UserDefFlatUnion(tmp);
+    g_assert(!tmp);
 }
 
 static void test_validate_fail_union_flat_no_discrim(TestInputVisitorData *data,
@@ -257,13 +254,13 @@ static void test_validate_fail_union_flat_no_discrim(TestInputVisitorData *data,
 
     visit_type_UserDefFlatUnion2(v, NULL, &tmp, &err);
     error_free_or_abort(&err);
-    qapi_free_UserDefFlatUnion2(tmp);
+    g_assert(!tmp);
 }
 
 static void test_validate_fail_alternate(TestInputVisitorData *data,
                                          const void *unused)
 {
-    UserDefAlternate *tmp = NULL;
+    UserDefAlternate *tmp;
     Visitor *v;
     Error *err = NULL;
 
@@ -271,7 +268,7 @@ static void test_validate_fail_alternate(TestInputVisitorData *data,
 
     visit_type_UserDefAlternate(v, NULL, &tmp, &err);
     error_free_or_abort(&err);
-    qapi_free_UserDefAlternate(tmp);
+    g_assert(!tmp);
 }
 
 static void do_test_validate_qmp_introspect(TestInputVisitorData *data,
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 80527eb850..cee07ce8dd 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -51,7 +51,7 @@ 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);
+    data->qiv = qmp_input_visitor_new(data->obj, false);
     g_assert(data->qiv);
 
     v = qmp_input_get_visitor(data->qiv);
@@ -279,6 +279,34 @@ static void test_visitor_in_any(TestInputVisitorData *data,
     qobject_decref(res);
 }
 
+static void test_visitor_in_null(TestInputVisitorData *data,
+                                 const void *unused)
+{
+    Visitor *v;
+    Error *err = NULL;
+    char *tmp;
+
+    /*
+     * FIXME: Since QAPI doesn't know the 'null' type yet, we can't
+     * test visit_type_null() by reading into a QAPI struct then
+     * checking that it was populated correctly.  The best we can do
+     * for now is ensure that we consumed null from the input, proven
+     * by the fact that we can't re-read the key; and that we detect
+     * when input is not null.
+     */
+
+    v = visitor_input_test_init(data, "{ '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);
+    g_assert(!tmp);
+    error_free_or_abort(&err);
+    visit_type_null(v, "b", &err);
+    error_free_or_abort(&err);
+    visit_check_struct(v, &error_abort);
+    visit_end_struct(v);
+}
+
 static void test_visitor_in_union_flat(TestInputVisitorData *data,
                                        const void *unused)
 {
@@ -745,18 +773,12 @@ static void test_visitor_in_errors(TestInputVisitorData *data,
 
     visit_type_TestStruct(v, NULL, &p, &err);
     error_free_or_abort(&err);
-    /* FIXME - a failed parse should not leave a partially-allocated p
-     * for us to clean up; this could cause callers to leak memory. */
-    g_assert(p->string == NULL);
-
-    g_free(p->string);
-    g_free(p);
+    g_assert(!p);
 
     v = visitor_input_test_init(data, "[ '1', '2', false, '3' ]");
     visit_type_strList(v, NULL, &q, &err);
     error_free_or_abort(&err);
-    assert(q);
-    qapi_free_strList(q);
+    assert(!q);
 }
 
 static void test_visitor_in_wrong_type(TestInputVisitorData *data,
@@ -829,6 +851,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",
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index c70926793a..1f80e696ea 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -43,6 +43,12 @@ static void visitor_output_teardown(TestOutputVisitorData *data,
     data->ov = NULL;
 }
 
+static void visitor_reset(TestOutputVisitorData *data)
+{
+    visitor_output_teardown(data, NULL);
+    visitor_output_setup(data, NULL);
+}
+
 static void test_visitor_out_int(TestOutputVisitorData *data,
                                  const void *unused)
 {
@@ -139,6 +145,7 @@ static void test_visitor_out_enum(TestOutputVisitorData *data,
         g_assert_cmpstr(qstring_get_str(qobject_to_qstring(obj)), ==,
                         EnumOne_lookup[i]);
         qobject_decref(obj);
+        visitor_reset(data);
     }
 }
 
@@ -153,6 +160,7 @@ static void test_visitor_out_enum_errors(TestOutputVisitorData *data,
         visit_type_EnumOne(data->ov, "unused", &bad_values[i], &err);
         g_assert(err);
         error_free(err);
+        visitor_reset(data);
     }
 }
 
@@ -262,6 +270,7 @@ static void test_visitor_out_struct_errors(TestOutputVisitorData *data,
         visit_type_UserDefOne(data->ov, "unused", &pu, &err);
         g_assert(err);
         error_free(err);
+        visitor_reset(data);
     }
 }
 
@@ -366,6 +375,7 @@ static void test_visitor_out_any(TestOutputVisitorData *data,
     qobject_decref(obj);
     qobject_decref(qobj);
 
+    visitor_reset(data);
     qdict = qdict_new();
     qdict_put(qdict, "integer", qint_from_int(-42));
     qdict_put(qdict, "boolean", qbool_from_bool(true));
@@ -442,6 +452,7 @@ static void test_visitor_out_alternate(TestOutputVisitorData *data,
     qapi_free_UserDefAlternate(tmp);
     qobject_decref(arg);
 
+    visitor_reset(data);
     tmp = g_new0(UserDefAlternate, 1);
     tmp->type = QTYPE_QSTRING;
     tmp->u.s = g_strdup("hello");
@@ -455,6 +466,7 @@ static void test_visitor_out_alternate(TestOutputVisitorData *data,
     qapi_free_UserDefAlternate(tmp);
     qobject_decref(arg);
 
+    visitor_reset(data);
     tmp = g_new0(UserDefAlternate, 1);
     tmp->type = QTYPE_QDICT;
     tmp->u.udfu.integer = 1;
@@ -477,15 +489,24 @@ static void test_visitor_out_alternate(TestOutputVisitorData *data,
     qobject_decref(arg);
 }
 
-static void test_visitor_out_empty(TestOutputVisitorData *data,
-                                   const void *unused)
+static void test_visitor_out_null(TestOutputVisitorData *data,
+                                  const void *unused)
 {
     QObject *arg;
+    QDict *qdict;
+    QObject *nil;
 
+    visit_start_struct(data->ov, NULL, NULL, 0, &error_abort);
+    visit_type_null(data->ov, "a", &error_abort);
+    visit_check_struct(data->ov, &error_abort);
+    visit_end_struct(data->ov);
     arg = qmp_output_get_qobject(data->qov);
-    g_assert(qobject_type(arg) == QTYPE_QNULL);
-    /* Check that qnull reference counting is sane */
-    g_assert(arg->refcnt == 2);
+    g_assert(qobject_type(arg) == QTYPE_QDICT);
+    qdict = qobject_to_qdict(arg);
+    g_assert_cmpint(qdict_size(qdict), ==, 1);
+    nil = qdict_get(qdict, "a");
+    g_assert(nil);
+    g_assert(qobject_type(nil) == QTYPE_QNULL);
     qobject_decref(arg);
 }
 
@@ -839,8 +860,8 @@ int main(int argc, char **argv)
                             &out_visitor_data, test_visitor_out_union_flat);
     output_visitor_test_add("/visitor/output/alternate",
                             &out_visitor_data, test_visitor_out_alternate);
-    output_visitor_test_add("/visitor/output/empty",
-                            &out_visitor_data, test_visitor_out_empty);
+    output_visitor_test_add("/visitor/output/null",
+                            &out_visitor_data, test_visitor_out_null);
     output_visitor_test_add("/visitor/output/native_list/int",
                             &out_visitor_data,
                             test_visitor_out_native_list_int);
diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c
index 9e6906a567..5a56920222 100644
--- a/tests/test-string-input-visitor.c
+++ b/tests/test-string-input-visitor.c
@@ -63,6 +63,13 @@ static void test_visitor_in_int(TestInputVisitorData *data,
     visit_type_int(v, NULL, &res, &err);
     g_assert(!err);
     g_assert_cmpint(res, ==, value);
+
+    visitor_input_teardown(data, unused);
+
+    v = visitor_input_test_init(data, "not an int");
+
+    visit_type_int(v, NULL, &res, &err);
+    error_free_or_abort(&err);
 }
 
 static void test_visitor_in_intList(TestInputVisitorData *data,
@@ -70,6 +77,7 @@ static void test_visitor_in_intList(TestInputVisitorData *data,
 {
     int64_t value[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 20};
     int16List *res = NULL, *tmp;
+    Error *err = NULL;
     Visitor *v;
     int i = 0;
 
@@ -84,12 +92,15 @@ static void test_visitor_in_intList(TestInputVisitorData *data,
     }
     g_assert(!tmp);
 
-    tmp = res;
-    while (tmp) {
-        res = res->next;
-        g_free(tmp);
-        tmp = res;
-    }
+    qapi_free_int16List(res);
+
+    visitor_input_teardown(data, unused);
+
+    v = visitor_input_test_init(data, "not an int list");
+
+    visit_type_int16List(v, NULL, &res, &err);
+    error_free_or_abort(&err);
+    g_assert(!res);
 }
 
 static void test_visitor_in_bool(TestInputVisitorData *data,
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index 9adbc30a41..7b14b5a7af 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -1038,7 +1038,7 @@ static void qmp_deserialize(void **native_out, void *datap,
     obj = qobject_from_json(qstring_get_str(output_json));
 
     QDECREF(output_json);
-    d->qiv = qmp_input_visitor_new(obj);
+    d->qiv = qmp_input_visitor_new(obj, true);
     qobject_decref(obj_orig);
     qobject_decref(obj);
     visit(qmp_input_get_visitor(d->qiv), native_out, errp);