From ec95f6148ce0d48f098979ed2168708820abd9dd Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 3 Mar 2017 13:32:38 +0100 Subject: test-qobject-input-visitor: Use strict visitor The qobject input visitor comes in a strict and a non-strict variant. This test is the non-strict variant's last user. Turns out it relies on non-strict only in test_visitor_in_null(), and just out of laziness. We don't actually test the non-strict behavior. Clean up test_visitor_in_null(), and switch to the strict variant. The next commit will drop the non-strict variant. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <1488544368-30622-19-git-send-email-armbru@redhat.com> --- tests/test-qobject-input-visitor.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'tests/test-qobject-input-visitor.c') diff --git a/tests/test-qobject-input-visitor.c b/tests/test-qobject-input-visitor.c index 945404a9e0..125e34c101 100644 --- a/tests/test-qobject-input-visitor.c +++ b/tests/test-qobject-input-visitor.c @@ -49,7 +49,7 @@ 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); + data->qiv = qobject_input_visitor_new(data->obj, true); g_assert(data->qiv); return data->qiv; } @@ -290,14 +290,14 @@ 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(data, "{ 'a': null, 'b': '', 'c': null }"); 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_type_str(v, "c", &tmp, &err); + g_assert(!tmp); + error_free_or_abort(&err); visit_check_struct(v, &error_abort); visit_end_struct(v, NULL); } -- cgit 1.4.1 From 048abb7b20c9f822ad9d4b730bade73b3311a47a Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 3 Mar 2017 13:32:39 +0100 Subject: qapi: Drop unused non-strict qobject input visitor The split between tests/test-qobject-input-visitor.c and tests/test-qobject-input-strict.c now makes less sense than ever. The next commit will take care of that. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <1488544368-30622-20-git-send-email-armbru@redhat.com> --- block/nbd.c | 2 +- block/nfs.c | 2 +- block/ssh.c | 2 +- docs/qapi-code-gen.txt | 2 +- include/qapi/qobject-input-visitor.h | 5 +---- qapi/qobject-input-visitor.c | 30 +++++++++++------------------- qmp.c | 2 +- qom/qom-qobject.c | 2 +- scripts/qapi-commands.py | 2 +- target/s390x/cpu_models.c | 2 +- tests/check-qnull.c | 2 +- tests/qmp-test.c | 2 +- tests/test-qmp-commands.c | 2 +- tests/test-qobject-input-strict.c | 2 +- tests/test-qobject-input-visitor.c | 2 +- tests/test-visitor-serialization.c | 2 +- 16 files changed, 26 insertions(+), 37 deletions(-) (limited to 'tests/test-qobject-input-visitor.c') diff --git a/block/nbd.c b/block/nbd.c index a7f9108fe5..f478f80b4a 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -278,7 +278,7 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QDict *options, Error **errp) goto done; } - iv = qobject_input_visitor_new(crumpled_addr, true); + iv = qobject_input_visitor_new(crumpled_addr); visit_type_SocketAddress(iv, NULL, &saddr, &local_err); if (local_err) { error_propagate(errp, local_err); diff --git a/block/nfs.c b/block/nfs.c index 890d5d4aff..3f43f6e26a 100644 --- a/block/nfs.c +++ b/block/nfs.c @@ -474,7 +474,7 @@ static NFSServer *nfs_config(QDict *options, Error **errp) goto out; } - iv = qobject_input_visitor_new(crumpled_addr, true); + iv = qobject_input_visitor_new(crumpled_addr); visit_type_NFSServer(iv, NULL, &server, &local_error); if (local_error) { error_propagate(errp, local_error); diff --git a/block/ssh.c b/block/ssh.c index 835932e6a4..278e66faa6 100644 --- a/block/ssh.c +++ b/block/ssh.c @@ -601,7 +601,7 @@ static InetSocketAddress *ssh_config(QDict *options, Error **errp) goto out; } - iv = qobject_input_visitor_new(crumpled_addr, true); + iv = qobject_input_visitor_new(crumpled_addr); visit_type_InetSocketAddress(iv, NULL, &inet, &local_error); if (local_error) { error_propagate(errp, local_error); diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index 7eb7be12ab..6746c1052c 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -1138,7 +1138,7 @@ Example: Visitor *v; UserDefOneList *arg1 = NULL; - v = qobject_input_visitor_new(QOBJECT(args), true); + v = qobject_input_visitor_new(QOBJECT(args)); visit_start_struct(v, NULL, NULL, 0, &err); if (err) { goto out; diff --git a/include/qapi/qobject-input-visitor.h b/include/qapi/qobject-input-visitor.h index cde328da9f..21db9c4b4a 100644 --- a/include/qapi/qobject-input-visitor.h +++ b/include/qapi/qobject-input-visitor.h @@ -21,10 +21,7 @@ typedef struct QObjectInputVisitor QObjectInputVisitor; /* * Return a new input visitor that converts a QObject to a QAPI object. - * - * Set @strict to reject a parse that doesn't consume all keys of a - * dictionary; otherwise excess input is ignored. */ -Visitor *qobject_input_visitor_new(QObject *obj, bool strict); +Visitor *qobject_input_visitor_new(QObject *obj); #endif diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c index 8015a986b0..eafcdf4625 100644 --- a/qapi/qobject-input-visitor.c +++ b/qapi/qobject-input-visitor.c @@ -43,9 +43,6 @@ struct QObjectInputVisitor { * QDict or QList). */ QSLIST_HEAD(, StackObject) stack; - /* True to reject parse in visit_end_struct() if unvisited keys remain. */ - bool strict; - GString *errname; /* Accumulator for full_name() */ }; @@ -157,11 +154,12 @@ static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv, tos->obj = obj; tos->qapi = qapi; - if (qiv->strict && qobject_type(obj) == QTYPE_QDICT) { + if (qobject_type(obj) == QTYPE_QDICT) { h = g_hash_table_new(g_str_hash, g_str_equal); qdict_iter(qobject_to_qdict(obj), qdict_add_key, h); tos->h = h; - } else if (qobject_type(obj) == QTYPE_QLIST) { + } else { + assert(qobject_type(obj) == QTYPE_QLIST); tos->entry = qlist_first(qobject_to_qlist(obj)); tos->index = -1; } @@ -175,20 +173,15 @@ static void qobject_input_check_struct(Visitor *v, Error **errp) { QObjectInputVisitor *qiv = to_qiv(v); StackObject *tos = QSLIST_FIRST(&qiv->stack); + GHashTableIter iter; + const char *key; assert(tos && !tos->entry); - if (qiv->strict) { - GHashTable *const top_ht = tos->h; - if (top_ht) { - GHashTableIter iter; - const char *key; - - g_hash_table_iter_init(&iter, top_ht); - if (g_hash_table_iter_next(&iter, (void **)&key, NULL)) { - error_setg(errp, "Parameter '%s' is unexpected", - full_name(qiv, key)); - } - } + + g_hash_table_iter_init(&iter, tos->h); + if (g_hash_table_iter_next(&iter, (void **)&key, NULL)) { + error_setg(errp, "Parameter '%s' is unexpected", + full_name(qiv, key)); } } @@ -465,7 +458,7 @@ static void qobject_input_free(Visitor *v) g_free(qiv); } -Visitor *qobject_input_visitor_new(QObject *obj, bool strict) +Visitor *qobject_input_visitor_new(QObject *obj) { QObjectInputVisitor *v; @@ -489,7 +482,6 @@ Visitor *qobject_input_visitor_new(QObject *obj, bool strict) v->visitor.type_null = qobject_input_type_null; v->visitor.optional = qobject_input_optional; v->visitor.free = qobject_input_free; - v->strict = strict; v->root = obj; qobject_incref(obj); diff --git a/qmp.c b/qmp.c index dfaabac1a6..fa82b598c6 100644 --- a/qmp.c +++ b/qmp.c @@ -675,7 +675,7 @@ void qmp_object_add(const char *type, const char *id, pdict = qdict_new(); } - v = qobject_input_visitor_new(QOBJECT(pdict), true); + v = qobject_input_visitor_new(QOBJECT(pdict)); obj = user_creatable_add_type(type, id, pdict, v, errp); visit_free(v); if (obj) { diff --git a/qom/qom-qobject.c b/qom/qom-qobject.c index bbdedda74a..4aec20d73c 100644 --- a/qom/qom-qobject.c +++ b/qom/qom-qobject.c @@ -23,7 +23,7 @@ void object_property_set_qobject(Object *obj, QObject *value, { Visitor *v; - v = qobject_input_visitor_new(value, true); + v = qobject_input_visitor_new(value); object_property_set(obj, v, name, errp); visit_free(v); } diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 2b062ad1fd..0c05449cb6 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -130,7 +130,7 @@ def gen_marshal(name, arg_type, boxed, ret_type): push_indent() ret += mcgen(''' - v = qobject_input_visitor_new(QOBJECT(args), true); + v = qobject_input_visitor_new(QOBJECT(args)); visit_start_struct(v, NULL, NULL, 0, &err); if (err) { goto out; diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c index 2a894eec65..4ea3a2de80 100644 --- a/target/s390x/cpu_models.c +++ b/target/s390x/cpu_models.c @@ -346,7 +346,7 @@ static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo *info, } if (qdict) { - visitor = qobject_input_visitor_new(info->props, true); + visitor = qobject_input_visitor_new(info->props); visit_start_struct(visitor, NULL, NULL, 0, errp); if (*errp) { object_unref(obj); diff --git a/tests/check-qnull.c b/tests/check-qnull.c index b50bb8a81a..8dd1c9686f 100644 --- a/tests/check-qnull.c +++ b/tests/check-qnull.c @@ -47,7 +47,7 @@ static void qnull_visit_test(void) g_assert(qnull_.refcnt == 1); obj = qnull(); - v = qobject_input_visitor_new(obj, true); + v = qobject_input_visitor_new(obj); qobject_decref(obj); visit_type_null(v, NULL, &error_abort); visit_free(v); diff --git a/tests/qmp-test.c b/tests/qmp-test.c index 405e49ec34..5d0260b2be 100644 --- a/tests/qmp-test.c +++ b/tests/qmp-test.c @@ -34,7 +34,7 @@ static void test_version(QObject *version) VersionInfo *vinfo; g_assert(version); - v = qobject_input_visitor_new(version, true); + v = qobject_input_visitor_new(version); visit_type_VersionInfo(v, "version", &vinfo, &error_abort); qapi_free_VersionInfo(vinfo); visit_free(v); diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c index 1cf413bc39..0f81a98be2 100644 --- a/tests/test-qmp-commands.c +++ b/tests/test-qmp-commands.c @@ -246,7 +246,7 @@ static void test_dealloc_partial(void) ud2_dict = qdict_new(); qdict_put_obj(ud2_dict, "string0", QOBJECT(qstring_from_str(text))); - v = qobject_input_visitor_new(QOBJECT(ud2_dict), true); + v = qobject_input_visitor_new(QOBJECT(ud2_dict)); visit_type_UserDefTwo(v, NULL, &ud2, &err); visit_free(v); QDECREF(ud2_dict); diff --git a/tests/test-qobject-input-strict.c b/tests/test-qobject-input-strict.c index 4087ea3dd3..7d26113013 100644 --- a/tests/test-qobject-input-strict.c +++ b/tests/test-qobject-input-strict.c @@ -53,7 +53,7 @@ static Visitor *validate_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, true); + data->qiv = qobject_input_visitor_new(data->obj); g_assert(data->qiv); return data->qiv; } diff --git a/tests/test-qobject-input-visitor.c b/tests/test-qobject-input-visitor.c index 125e34c101..658fa2c24d 100644 --- a/tests/test-qobject-input-visitor.c +++ b/tests/test-qobject-input-visitor.c @@ -49,7 +49,7 @@ 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, true); + data->qiv = qobject_input_visitor_new(data->obj); g_assert(data->qiv); return data->qiv; } diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c index 66b2b1c564..c7e64f022c 100644 --- a/tests/test-visitor-serialization.c +++ b/tests/test-visitor-serialization.c @@ -1040,7 +1040,7 @@ static void qmp_deserialize(void **native_out, void *datap, obj = qobject_from_json(qstring_get_str(output_json)); QDECREF(output_json); - d->qiv = qobject_input_visitor_new(obj, true); + d->qiv = qobject_input_visitor_new(obj); qobject_decref(obj_orig); qobject_decref(obj); visit(d->qiv, native_out, errp); -- cgit 1.4.1 From 77c47de23ff58ff987fed92713546d99720bd099 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 3 Mar 2017 13:32:40 +0100 Subject: tests-qobject-input-strict: Merge into test-qobject-input-visitor Much of test-qobject-input-strict.c duplicates test-qobject-input-strict.c, but with less assertions on expected output: * test_validate_struct() duplicates test_visitor_in_struct() * test_validate_struct_nested() duplicates test_visitor_in_struct_nested() * test_validate_list() duplicates the first half of test_visitor_in_list() * test_validate_union_native_list() duplicates test_visitor_in_native_list_int() * test_validate_union_flat() duplicates test_visitor_in_union_flat() * test_validate_alternate() duplicates the first part of test_visitor_in_alternate() Merge the remaining test cases into test-qobject-input-visitor.c, and drop the now redundant test-qobject-input-strict.c. Test case "/visitor/input-strict/fail/list" isn't really about lists, it's about a bad struct nested in a list. Rename accordingly. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <1488544368-30622-21-git-send-email-armbru@redhat.com> --- tests/Makefile.include | 4 +- tests/test-qobject-input-strict.c | 381 ------------------------------------- tests/test-qobject-input-visitor.c | 187 ++++++++++++++++++ 3 files changed, 188 insertions(+), 384 deletions(-) delete mode 100644 tests/test-qobject-input-strict.c (limited to 'tests/test-qobject-input-visitor.c') diff --git a/tests/Makefile.include b/tests/Makefile.include index bb0f430b34..ace4e80464 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -28,7 +28,6 @@ check-unit-y += tests/test-clone-visitor$(EXESUF) gcov-files-test-clone-visitor-y = qapi/qapi-clone-visitor.c check-unit-y += tests/test-qobject-input-visitor$(EXESUF) gcov-files-test-qobject-input-visitor-y = qapi/qobject-input-visitor.c -check-unit-y += tests/test-qobject-input-strict$(EXESUF) check-unit-y += tests/test-qmp-commands$(EXESUF) gcov-files-test-qmp-commands-y = qapi/qmp-dispatch.c check-unit-y += tests/test-string-input-visitor$(EXESUF) @@ -489,7 +488,7 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \ tests/test-coroutine.o tests/test-string-output-visitor.o \ tests/test-string-input-visitor.o tests/test-qobject-output-visitor.o \ tests/test-clone-visitor.o \ - tests/test-qobject-input-visitor.o tests/test-qobject-input-strict.o \ + tests/test-qobject-input-visitor.o \ tests/test-qmp-commands.o tests/test-visitor-serialization.o \ tests/test-x86-cpuid.o tests/test-mul64.o tests/test-int128.o \ tests/test-opts-visitor.o tests/test-qmp-event.o \ @@ -598,7 +597,6 @@ tests/test-qmp-event$(EXESUF): tests/test-qmp-event.o $(test-qapi-obj-y) tests/test-qobject-output-visitor$(EXESUF): tests/test-qobject-output-visitor.o $(test-qapi-obj-y) tests/test-clone-visitor$(EXESUF): tests/test-clone-visitor.o $(test-qapi-obj-y) tests/test-qobject-input-visitor$(EXESUF): tests/test-qobject-input-visitor.o $(test-qapi-obj-y) -tests/test-qobject-input-strict$(EXESUF): tests/test-qobject-input-strict.o $(test-qapi-obj-y) tests/test-qmp-commands$(EXESUF): tests/test-qmp-commands.o tests/test-qmp-marshal.o $(test-qapi-obj-y) tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o $(test-qapi-obj-y) tests/test-opts-visitor$(EXESUF): tests/test-opts-visitor.o $(test-qapi-obj-y) diff --git a/tests/test-qobject-input-strict.c b/tests/test-qobject-input-strict.c deleted file mode 100644 index 7d26113013..0000000000 --- a/tests/test-qobject-input-strict.c +++ /dev/null @@ -1,381 +0,0 @@ -/* - * QObject Input Visitor unit-tests (strict mode). - * - * Copyright (C) 2011-2012, 2015 Red Hat Inc. - * - * Authors: - * Luiz Capitulino - * Paolo Bonzini - * - * This work is licensed under the terms of the GNU GPL, version 2 or later. - * See the COPYING file in the top-level directory. - */ - -#include "qemu/osdep.h" - -#include "qemu-common.h" -#include "qapi/error.h" -#include "qapi/qobject-input-visitor.h" -#include "test-qapi-types.h" -#include "test-qapi-visit.h" -#include "qapi/qmp/types.h" -#include "qapi/qmp/qjson.h" -#include "test-qmp-introspect.h" -#include "qmp-introspect.h" -#include "qapi-visit.h" - -typedef struct TestInputVisitorData { - QObject *obj; - Visitor *qiv; -} TestInputVisitorData; - -static void validate_teardown(TestInputVisitorData *data, - const void *unused) -{ - qobject_decref(data->obj); - data->obj = NULL; - - if (data->qiv) { - visit_free(data->qiv); - data->qiv = NULL; - } -} - -/* The various test_init functions are provided instead of a test setup - function so that the JSON string used by the tests are kept in the test - functions (and not in main()). */ -static Visitor *validate_test_init_internal(TestInputVisitorData *data, - const char *json_string, - va_list *ap) -{ - validate_teardown(data, NULL); - - data->obj = qobject_from_jsonv(json_string, ap); - g_assert(data->obj); - - data->qiv = qobject_input_visitor_new(data->obj); - g_assert(data->qiv); - return data->qiv; -} - -static GCC_FMT_ATTR(2, 3) -Visitor *validate_test_init(TestInputVisitorData *data, - const char *json_string, ...) -{ - Visitor *v; - va_list ap; - - va_start(ap, json_string); - v = validate_test_init_internal(data, json_string, &ap); - va_end(ap); - return v; -} - -/* similar to validate_test_init(), but does not expect a string - * literal/format json_string argument and so can be used for - * programatically generated strings (and we can't pass in programatically - * generated strings via %s format parameters since qobject_from_jsonv() - * will wrap those in double-quotes and treat the entire object as a - * string) - */ -static Visitor *validate_test_init_raw(TestInputVisitorData *data, - const char *json_string) -{ - return validate_test_init_internal(data, json_string, NULL); -} - - -static void test_validate_struct(TestInputVisitorData *data, - const void *unused) -{ - TestStruct *p = NULL; - Visitor *v; - - v = validate_test_init(data, "{ 'integer': -42, 'boolean': true, 'string': 'foo' }"); - - visit_type_TestStruct(v, NULL, &p, &error_abort); - g_free(p->string); - g_free(p); -} - -static void test_validate_struct_nested(TestInputVisitorData *data, - const void *unused) -{ - UserDefTwo *udp = NULL; - Visitor *v; - - v = validate_test_init(data, "{ 'string0': 'string0', " - "'dict1': { 'string1': 'string1', " - "'dict2': { 'userdef': { 'integer': 42, " - "'string': 'string' }, 'string': 'string2'}}}"); - - visit_type_UserDefTwo(v, NULL, &udp, &error_abort); - qapi_free_UserDefTwo(udp); -} - -static void test_validate_list(TestInputVisitorData *data, - const void *unused) -{ - UserDefOneList *head = NULL; - Visitor *v; - - v = validate_test_init(data, "[ { 'string': 'string0', 'integer': 42 }, { 'string': 'string1', 'integer': 43 }, { 'string': 'string2', 'integer': 44 } ]"); - - visit_type_UserDefOneList(v, NULL, &head, &error_abort); - qapi_free_UserDefOneList(head); -} - -static void test_validate_union_native_list(TestInputVisitorData *data, - const void *unused) -{ - UserDefNativeListUnion *tmp = NULL; - Visitor *v; - - v = validate_test_init(data, "{ 'type': 'integer', 'data' : [ 1, 2 ] }"); - - visit_type_UserDefNativeListUnion(v, NULL, &tmp, &error_abort); - qapi_free_UserDefNativeListUnion(tmp); -} - -static void test_validate_union_flat(TestInputVisitorData *data, - const void *unused) -{ - UserDefFlatUnion *tmp = NULL; - Visitor *v; - - v = validate_test_init(data, - "{ 'enum1': 'value1', " - "'integer': 41, " - "'string': 'str', " - "'boolean': true }"); - - visit_type_UserDefFlatUnion(v, NULL, &tmp, &error_abort); - qapi_free_UserDefFlatUnion(tmp); -} - -static void test_validate_alternate(TestInputVisitorData *data, - const void *unused) -{ - UserDefAlternate *tmp = NULL; - Visitor *v; - - v = validate_test_init(data, "42"); - - visit_type_UserDefAlternate(v, NULL, &tmp, &error_abort); - qapi_free_UserDefAlternate(tmp); -} - -static void test_validate_fail_struct(TestInputVisitorData *data, - const void *unused) -{ - TestStruct *p = NULL; - Error *err = NULL; - Visitor *v; - - v = validate_test_init(data, "{ 'integer': -42, 'boolean': true, 'string': 'foo', 'extra': 42 }"); - - visit_type_TestStruct(v, NULL, &p, &err); - error_free_or_abort(&err); - g_assert(!p); -} - -static void test_validate_fail_struct_nested(TestInputVisitorData *data, - const void *unused) -{ - UserDefTwo *udp = NULL; - Error *err = NULL; - Visitor *v; - - v = validate_test_init(data, "{ 'string0': 'string0', 'dict1': { 'string1': 'string1', 'dict2': { 'userdef1': { 'integer': 42, 'string': 'string', 'extra': [42, 23, {'foo':'bar'}] }, 'string2': 'string2'}}}"); - - visit_type_UserDefTwo(v, NULL, &udp, &err); - error_free_or_abort(&err); - g_assert(!udp); -} - -static void test_validate_fail_struct_missing(TestInputVisitorData *data, - const void *unused) -{ - Error *err = NULL; - Visitor *v; - QObject *any; - GenericAlternate *alt; - bool present; - int en; - int64_t i64; - uint32_t u32; - int8_t i8; - char *str; - double dbl; - - v = validate_test_init(data, "{}"); - visit_start_struct(v, NULL, NULL, 0, &error_abort); - visit_start_struct(v, "struct", NULL, 0, &err); - error_free_or_abort(&err); - visit_start_list(v, "list", NULL, 0, &err); - error_free_or_abort(&err); - visit_start_alternate(v, "alternate", &alt, sizeof(*alt), false, &err); - error_free_or_abort(&err); - visit_optional(v, "optional", &present); - g_assert(!present); - visit_type_enum(v, "enum", &en, EnumOne_lookup, &err); - error_free_or_abort(&err); - visit_type_int(v, "i64", &i64, &err); - error_free_or_abort(&err); - visit_type_uint32(v, "u32", &u32, &err); - error_free_or_abort(&err); - visit_type_int8(v, "i8", &i8, &err); - error_free_or_abort(&err); - visit_type_str(v, "i8", &str, &err); - error_free_or_abort(&err); - visit_type_number(v, "dbl", &dbl, &err); - error_free_or_abort(&err); - visit_type_any(v, "any", &any, &err); - error_free_or_abort(&err); - visit_type_null(v, "null", &err); - error_free_or_abort(&err); - visit_end_struct(v, NULL); -} - -static void test_validate_fail_list(TestInputVisitorData *data, - const void *unused) -{ - UserDefOneList *head = NULL; - Error *err = NULL; - Visitor *v; - - v = validate_test_init(data, "[ { 'string': 'string0', 'integer': 42 }, { 'string': 'string1', 'integer': 43 }, { 'string': 'string2', 'integer': 44, 'extra': 'ggg' } ]"); - - visit_type_UserDefOneList(v, NULL, &head, &err); - error_free_or_abort(&err); - g_assert(!head); -} - -static void test_validate_fail_union_native_list(TestInputVisitorData *data, - const void *unused) -{ - UserDefNativeListUnion *tmp = NULL; - Error *err = NULL; - Visitor *v; - - v = validate_test_init(data, - "{ 'type': 'integer', 'data' : [ 'string' ] }"); - - visit_type_UserDefNativeListUnion(v, NULL, &tmp, &err); - error_free_or_abort(&err); - g_assert(!tmp); -} - -static void test_validate_fail_union_flat(TestInputVisitorData *data, - const void *unused) -{ - UserDefFlatUnion *tmp = NULL; - Error *err = NULL; - Visitor *v; - - v = validate_test_init(data, "{ 'string': 'c', 'integer': 41, 'boolean': true }"); - - visit_type_UserDefFlatUnion(v, NULL, &tmp, &err); - error_free_or_abort(&err); - g_assert(!tmp); -} - -static void test_validate_fail_union_flat_no_discrim(TestInputVisitorData *data, - const void *unused) -{ - UserDefFlatUnion2 *tmp = NULL; - Error *err = NULL; - Visitor *v; - - /* test situation where discriminator field ('enum1' here) is missing */ - v = validate_test_init(data, "{ 'integer': 42, 'string': 'c', 'string1': 'd', 'string2': 'e' }"); - - visit_type_UserDefFlatUnion2(v, NULL, &tmp, &err); - error_free_or_abort(&err); - g_assert(!tmp); -} - -static void test_validate_fail_alternate(TestInputVisitorData *data, - const void *unused) -{ - UserDefAlternate *tmp; - Visitor *v; - Error *err = NULL; - - v = validate_test_init(data, "3.14"); - - visit_type_UserDefAlternate(v, NULL, &tmp, &err); - error_free_or_abort(&err); - g_assert(!tmp); -} - -static void do_test_validate_qmp_introspect(TestInputVisitorData *data, - const char *schema_json) -{ - SchemaInfoList *schema = NULL; - Visitor *v; - - v = validate_test_init_raw(data, schema_json); - - visit_type_SchemaInfoList(v, NULL, &schema, &error_abort); - g_assert(schema); - - qapi_free_SchemaInfoList(schema); -} - -static void test_validate_qmp_introspect(TestInputVisitorData *data, - const void *unused) -{ - do_test_validate_qmp_introspect(data, test_qmp_schema_json); - do_test_validate_qmp_introspect(data, qmp_schema_json); -} - -static void validate_test_add(const char *testpath, - TestInputVisitorData *data, - void (*test_func)(TestInputVisitorData *data, const void *user_data)) -{ - g_test_add(testpath, TestInputVisitorData, data, NULL, test_func, - validate_teardown); -} - -int main(int argc, char **argv) -{ - TestInputVisitorData testdata; - - g_test_init(&argc, &argv, NULL); - - validate_test_add("/visitor/input-strict/pass/struct", - &testdata, test_validate_struct); - validate_test_add("/visitor/input-strict/pass/struct-nested", - &testdata, test_validate_struct_nested); - validate_test_add("/visitor/input-strict/pass/list", - &testdata, test_validate_list); - validate_test_add("/visitor/input-strict/pass/union-flat", - &testdata, test_validate_union_flat); - validate_test_add("/visitor/input-strict/pass/alternate", - &testdata, test_validate_alternate); - validate_test_add("/visitor/input-strict/pass/union-native-list", - &testdata, test_validate_union_native_list); - validate_test_add("/visitor/input-strict/fail/struct", - &testdata, test_validate_fail_struct); - validate_test_add("/visitor/input-strict/fail/struct-nested", - &testdata, test_validate_fail_struct_nested); - validate_test_add("/visitor/input-strict/fail/struct-missing", - &testdata, test_validate_fail_struct_missing); - validate_test_add("/visitor/input-strict/fail/list", - &testdata, test_validate_fail_list); - validate_test_add("/visitor/input-strict/fail/union-flat", - &testdata, test_validate_fail_union_flat); - validate_test_add("/visitor/input-strict/fail/union-flat-no-discriminator", - &testdata, test_validate_fail_union_flat_no_discrim); - validate_test_add("/visitor/input-strict/fail/alternate", - &testdata, test_validate_fail_alternate); - validate_test_add("/visitor/input-strict/fail/union-native-list", - &testdata, test_validate_fail_union_native_list); - validate_test_add("/visitor/input-strict/pass/qmp-introspect", - &testdata, test_validate_qmp_introspect); - - g_test_run(); - - return 0; -} diff --git a/tests/test-qobject-input-visitor.c b/tests/test-qobject-input-visitor.c index 658fa2c24d..32c6b3d689 100644 --- a/tests/test-qobject-input-visitor.c +++ b/tests/test-qobject-input-visitor.c @@ -5,6 +5,7 @@ * * Authors: * Luiz Capitulino + * Paolo Bonzini * * This work is licensed under the terms of the GNU GPL, version 2 or later. * See the COPYING file in the top-level directory. @@ -19,6 +20,9 @@ #include "test-qapi-visit.h" #include "qapi/qmp/types.h" #include "qapi/qmp/qjson.h" +#include "test-qmp-introspect.h" +#include "qmp-introspect.h" +#include "qapi-visit.h" typedef struct TestInputVisitorData { QObject *obj; @@ -833,6 +837,171 @@ static void test_visitor_in_wrong_type(TestInputVisitorData *data, error_free_or_abort(&err); } +static void test_visitor_in_fail_struct(TestInputVisitorData *data, + const void *unused) +{ + TestStruct *p = NULL; + Error *err = NULL; + Visitor *v; + + v = visitor_input_test_init(data, "{ 'integer': -42, 'boolean': true, 'string': 'foo', 'extra': 42 }"); + + visit_type_TestStruct(v, NULL, &p, &err); + error_free_or_abort(&err); + g_assert(!p); +} + +static void test_visitor_in_fail_struct_nested(TestInputVisitorData *data, + const void *unused) +{ + UserDefTwo *udp = NULL; + Error *err = NULL; + Visitor *v; + + v = visitor_input_test_init(data, "{ 'string0': 'string0', 'dict1': { 'string1': 'string1', 'dict2': { 'userdef1': { 'integer': 42, 'string': 'string', 'extra': [42, 23, {'foo':'bar'}] }, 'string2': 'string2'}}}"); + + visit_type_UserDefTwo(v, NULL, &udp, &err); + error_free_or_abort(&err); + g_assert(!udp); +} + +static void test_visitor_in_fail_struct_in_list(TestInputVisitorData *data, + const void *unused) +{ + UserDefOneList *head = NULL; + Error *err = NULL; + Visitor *v; + + v = visitor_input_test_init(data, "[ { 'string': 'string0', 'integer': 42 }, { 'string': 'string1', 'integer': 43 }, { 'string': 'string2', 'integer': 44, 'extra': 'ggg' } ]"); + + visit_type_UserDefOneList(v, NULL, &head, &err); + error_free_or_abort(&err); + g_assert(!head); +} + +static void test_visitor_in_fail_struct_missing(TestInputVisitorData *data, + const void *unused) +{ + Error *err = NULL; + Visitor *v; + QObject *any; + GenericAlternate *alt; + bool present; + int en; + int64_t i64; + uint32_t u32; + int8_t i8; + char *str; + double dbl; + + v = visitor_input_test_init(data, "{}"); + visit_start_struct(v, NULL, NULL, 0, &error_abort); + visit_start_struct(v, "struct", NULL, 0, &err); + error_free_or_abort(&err); + visit_start_list(v, "list", NULL, 0, &err); + error_free_or_abort(&err); + visit_start_alternate(v, "alternate", &alt, sizeof(*alt), false, &err); + error_free_or_abort(&err); + visit_optional(v, "optional", &present); + g_assert(!present); + visit_type_enum(v, "enum", &en, EnumOne_lookup, &err); + error_free_or_abort(&err); + visit_type_int(v, "i64", &i64, &err); + error_free_or_abort(&err); + visit_type_uint32(v, "u32", &u32, &err); + error_free_or_abort(&err); + visit_type_int8(v, "i8", &i8, &err); + error_free_or_abort(&err); + visit_type_str(v, "i8", &str, &err); + error_free_or_abort(&err); + visit_type_number(v, "dbl", &dbl, &err); + error_free_or_abort(&err); + visit_type_any(v, "any", &any, &err); + error_free_or_abort(&err); + visit_type_null(v, "null", &err); + error_free_or_abort(&err); + visit_end_struct(v, NULL); +} + +static void test_visitor_in_fail_union_native_list(TestInputVisitorData *data, + const void *unused) +{ + UserDefNativeListUnion *tmp = NULL; + Error *err = NULL; + Visitor *v; + + v = visitor_input_test_init(data, + "{ 'type': 'integer', 'data' : [ 'string' ] }"); + + visit_type_UserDefNativeListUnion(v, NULL, &tmp, &err); + error_free_or_abort(&err); + g_assert(!tmp); +} + +static void test_visitor_in_fail_union_flat(TestInputVisitorData *data, + const void *unused) +{ + UserDefFlatUnion *tmp = NULL; + Error *err = NULL; + Visitor *v; + + v = visitor_input_test_init(data, "{ 'string': 'c', 'integer': 41, 'boolean': true }"); + + visit_type_UserDefFlatUnion(v, NULL, &tmp, &err); + error_free_or_abort(&err); + g_assert(!tmp); +} + +static void test_visitor_in_fail_union_flat_no_discrim(TestInputVisitorData *data, + const void *unused) +{ + UserDefFlatUnion2 *tmp = NULL; + Error *err = NULL; + Visitor *v; + + /* test situation where discriminator field ('enum1' here) is missing */ + v = visitor_input_test_init(data, "{ 'integer': 42, 'string': 'c', 'string1': 'd', 'string2': 'e' }"); + + visit_type_UserDefFlatUnion2(v, NULL, &tmp, &err); + error_free_or_abort(&err); + g_assert(!tmp); +} + +static void test_visitor_in_fail_alternate(TestInputVisitorData *data, + const void *unused) +{ + UserDefAlternate *tmp; + Visitor *v; + Error *err = NULL; + + v = visitor_input_test_init(data, "3.14"); + + visit_type_UserDefAlternate(v, NULL, &tmp, &err); + error_free_or_abort(&err); + g_assert(!tmp); +} + +static void do_test_visitor_in_qmp_introspect(TestInputVisitorData *data, + const char *schema_json) +{ + SchemaInfoList *schema = NULL; + Visitor *v; + + v = visitor_input_test_init_raw(data, schema_json); + + visit_type_SchemaInfoList(v, NULL, &schema, &error_abort); + g_assert(schema); + + qapi_free_SchemaInfoList(schema); +} + +static void test_visitor_in_qmp_introspect(TestInputVisitorData *data, + const void *unused) +{ + do_test_visitor_in_qmp_introspect(data, test_qmp_schema_json); + do_test_visitor_in_qmp_introspect(data, qmp_schema_json); +} + int main(int argc, char **argv) { g_test_init(&argc, &argv, NULL); @@ -893,6 +1062,24 @@ int main(int argc, char **argv) NULL, test_visitor_in_native_list_string); input_visitor_test_add("/visitor/input/native_list/number", NULL, test_visitor_in_native_list_number); + input_visitor_test_add("/visitor/input/fail/struct", + NULL, test_visitor_in_fail_struct); + input_visitor_test_add("/visitor/input/fail/struct-nested", + NULL, test_visitor_in_fail_struct_nested); + input_visitor_test_add("/visitor/input/fail/struct-in-list", + NULL, test_visitor_in_fail_struct_in_list); + input_visitor_test_add("/visitor/input/fail/struct-missing", + NULL, test_visitor_in_fail_struct_missing); + input_visitor_test_add("/visitor/input/fail/union-flat", + NULL, test_visitor_in_fail_union_flat); + input_visitor_test_add("/visitor/input/fail/union-flat-no-discriminator", + NULL, test_visitor_in_fail_union_flat_no_discrim); + input_visitor_test_add("/visitor/input/fail/alternate", + NULL, test_visitor_in_fail_alternate); + input_visitor_test_add("/visitor/input/fail/union-native-list", + NULL, test_visitor_in_fail_union_native_list); + input_visitor_test_add("/visitor/input/qmp-introspect", + NULL, test_visitor_in_qmp_introspect); g_test_run(); -- cgit 1.4.1 From 9cb8ef36681b9645af1f7bd8fb64656e65de8a03 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 3 Mar 2017 13:32:43 +0100 Subject: tests: Cover partial input visit of list Demonstrates a design flaw: there is no way to for input visitors to report that a list visit didn't visit the complete input list. The generated list visits always do, but manual visits needn't. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <1488544368-30622-24-git-send-email-armbru@redhat.com> --- tests/test-opts-visitor.c | 41 +++++++++++++++++++++++++++++++++++ tests/test-qobject-input-visitor.c | 44 ++++++++++++++++++++++++++++++++++++++ tests/test-string-input-visitor.c | 22 +++++++++++++++++++ 3 files changed, 107 insertions(+) (limited to 'tests/test-qobject-input-visitor.c') diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c index 0a9e75f1bb..d0f764699b 100644 --- a/tests/test-opts-visitor.c +++ b/tests/test-opts-visitor.c @@ -172,6 +172,44 @@ expect_u64_max(OptsVisitorFixture *f, gconstpointer test_data) /* test cases */ +static void +test_opts_range_unvisited(void) +{ + intList *list = NULL; + intList *tail; + QemuOpts *opts; + Visitor *v; + + opts = qemu_opts_parse(qemu_find_opts("userdef"), "ilist=0-2", false, + &error_abort); + + v = opts_visitor_new(opts); + + visit_start_struct(v, NULL, NULL, 0, &error_abort); + + /* Would be simpler if the visitor genuinely supported virtual walks */ + visit_start_list(v, "ilist", (GenericList **)&list, sizeof(*list), + &error_abort); + tail = list; + visit_type_int(v, NULL, &tail->value, &error_abort); + g_assert_cmpint(tail->value, ==, 0); + tail = (intList *)visit_next_list(v, (GenericList *)tail, sizeof(*list)); + g_assert(tail); + visit_type_int(v, NULL, &tail->value, &error_abort); + g_assert_cmpint(tail->value, ==, 1); + tail = (intList *)visit_next_list(v, (GenericList *)tail, sizeof(*list)); + g_assert(tail); + visit_end_list(v, (void **)&list); + /* BUG: unvisited tail not reported; actually not reportable by design */ + + visit_check_struct(v, &error_abort); + visit_end_struct(v, NULL); + + qapi_free_intList(list); + visit_free(v); + qemu_opts_del(opts); +} + int main(int argc, char **argv) { @@ -263,6 +301,9 @@ main(int argc, char **argv) add_test("/visitor/opts/i64/range/2big/full", &expect_fail, "i64=-0x8000000000000000-0x7fffffffffffffff"); + g_test_add_func("/visitor/opts/range/unvisited", + test_opts_range_unvisited); + g_test_run(); return 0; } diff --git a/tests/test-qobject-input-visitor.c b/tests/test-qobject-input-visitor.c index 32c6b3d689..10c15c46db 100644 --- a/tests/test-qobject-input-visitor.c +++ b/tests/test-qobject-input-visitor.c @@ -923,6 +923,46 @@ static void test_visitor_in_fail_struct_missing(TestInputVisitorData *data, visit_end_struct(v, NULL); } +static void test_visitor_in_fail_list(TestInputVisitorData *data, + const void *unused) +{ + int64_t i64 = -1; + Visitor *v; + + /* Unvisited list tail */ + + v = visitor_input_test_init(data, "[ 1, 2, 3 ]"); + + visit_start_list(v, NULL, NULL, 0, &error_abort); + visit_type_int(v, NULL, &i64, &error_abort); + g_assert_cmpint(i64, ==, 1); + visit_type_int(v, NULL, &i64, &error_abort); + g_assert_cmpint(i64, ==, 2); + visit_end_list(v, NULL); + /* BUG: unvisited tail not reported; actually not reportable by design */ +} + +static void test_visitor_in_fail_list_nested(TestInputVisitorData *data, + const void *unused) +{ + int64_t i64 = -1; + Visitor *v; + + /* Unvisited nested list tail */ + + v = visitor_input_test_init(data, "[ 0, [ 1, 2, 3 ] ]"); + + visit_start_list(v, NULL, NULL, 0, &error_abort); + visit_type_int(v, NULL, &i64, &error_abort); + g_assert_cmpint(i64, ==, 0); + visit_start_list(v, NULL, NULL, 0, &error_abort); + visit_type_int(v, NULL, &i64, &error_abort); + g_assert_cmpint(i64, ==, 1); + visit_end_list(v, NULL); + /* BUG: unvisited tail not reported; actually not reportable by design */ + visit_end_list(v, NULL); +} + static void test_visitor_in_fail_union_native_list(TestInputVisitorData *data, const void *unused) { @@ -1070,6 +1110,10 @@ int main(int argc, char **argv) NULL, test_visitor_in_fail_struct_in_list); input_visitor_test_add("/visitor/input/fail/struct-missing", NULL, test_visitor_in_fail_struct_missing); + input_visitor_test_add("/visitor/input/fail/list", + NULL, test_visitor_in_fail_list); + input_visitor_test_add("/visitor/input/fail/list-nested", + NULL, test_visitor_in_fail_list_nested); input_visitor_test_add("/visitor/input/fail/union-flat", NULL, test_visitor_in_fail_union_flat); input_visitor_test_add("/visitor/input/fail/union-flat-no-discriminator", diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c index 72f8732f97..70cee65cd0 100644 --- a/tests/test-string-input-visitor.c +++ b/tests/test-string-input-visitor.c @@ -121,6 +121,7 @@ static void test_visitor_in_intList(TestInputVisitorData *data, uint64_t expect4[] = { UINT64_MAX }; Error *err = NULL; int64List *res = NULL; + int64List *tail; Visitor *v; /* Valid lists */ @@ -151,6 +152,27 @@ static void test_visitor_in_intList(TestInputVisitorData *data, visit_type_int64List(v, NULL, &res, &err); error_free_or_abort(&err); g_assert(!res); + + /* Unvisited list tail */ + + v = visitor_input_test_init(data, "0,2-3"); + + /* Would be simpler if the visitor genuinely supported virtual walks */ + visit_start_list(v, NULL, (GenericList **)&res, sizeof(*res), + &error_abort); + tail = res; + visit_type_int64(v, NULL, &tail->value, &error_abort); + g_assert_cmpint(tail->value, ==, 0); + tail = (int64List *)visit_next_list(v, (GenericList *)tail, sizeof(*res)); + g_assert(tail); + visit_type_int64(v, NULL, &tail->value, &error_abort); + g_assert_cmpint(tail->value, ==, 2); + tail = (int64List *)visit_next_list(v, (GenericList *)tail, sizeof(*res)); + g_assert(tail); + visit_end_list(v, (void **)&res); + /* BUG: unvisited tail not reported; actually not reportable by design */ + + qapi_free_int64List(res); } static void test_visitor_in_bool(TestInputVisitorData *data, -- cgit 1.4.1 From 86ca0dbe04d8eeebf460b56111c9af125e14528f Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 3 Mar 2017 13:32:44 +0100 Subject: test-qobject-input-visitor: Cover missing nested struct member Signed-off-by: Markus Armbruster Message-Id: <1488544368-30622-25-git-send-email-armbru@redhat.com> Reviewed-by: Eric Blake --- tests/test-qobject-input-visitor.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'tests/test-qobject-input-visitor.c') diff --git a/tests/test-qobject-input-visitor.c b/tests/test-qobject-input-visitor.c index 10c15c46db..9f3a826353 100644 --- a/tests/test-qobject-input-visitor.c +++ b/tests/test-qobject-input-visitor.c @@ -894,7 +894,7 @@ static void test_visitor_in_fail_struct_missing(TestInputVisitorData *data, char *str; double dbl; - v = visitor_input_test_init(data, "{}"); + v = visitor_input_test_init(data, "{ 'sub': [ {} ] }"); visit_start_struct(v, NULL, NULL, 0, &error_abort); visit_start_struct(v, "struct", NULL, 0, &err); error_free_or_abort(&err); @@ -920,6 +920,12 @@ static void test_visitor_in_fail_struct_missing(TestInputVisitorData *data, error_free_or_abort(&err); visit_type_null(v, "null", &err); error_free_or_abort(&err); + visit_start_list(v, "sub", NULL, 0, &error_abort); + visit_start_struct(v, NULL, NULL, 0, &error_abort); + visit_type_int(v, "i64", &i64, &err); + error_free_or_abort(&err); + visit_end_struct(v, NULL); + visit_end_list(v, NULL); visit_end_struct(v, NULL); } -- cgit 1.4.1 From a4a1c70dc759e5b81627e96564f344ab43ea86eb Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 3 Mar 2017 13:32:45 +0100 Subject: qapi: Make input visitors detect unvisited list tails Fix the design flaw demonstrated in the previous commit: new method check_list() lets input visitors report that unvisited input remains for a list, exactly like check_struct() lets them report that unvisited input remains for a struct or union. Implement the method for the qobject input visitor (straightforward), and the string input visitor (less so, due to the magic list syntax there). The opts visitor's list magic is even more impenetrable, and all I can do there today is a stub with a FIXME comment. No worse than before. Signed-off-by: Markus Armbruster Message-Id: <1488544368-30622-26-git-send-email-armbru@redhat.com> Reviewed-by: Eric Blake --- hw/ppc/spapr_drc.c | 5 +++++ include/qapi/visitor-impl.h | 3 +++ include/qapi/visitor.h | 13 +++++++++++++ qapi/opts-visitor.c | 11 +++++++++++ qapi/qapi-visit-core.c | 8 ++++++++ qapi/qobject-input-visitor.c | 37 +++++++++++++++++++++++++++++++------ qapi/string-input-visitor.c | 30 ++++++++++++++++++++++++++++++ qapi/trace-events | 1 + scripts/qapi-visit.py | 3 +++ tests/test-opts-visitor.c | 2 +- tests/test-qobject-input-visitor.c | 9 +++++++-- tests/test-string-input-visitor.c | 4 +++- 12 files changed, 116 insertions(+), 10 deletions(-) (limited to 'tests/test-qobject-input-visitor.c') diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index 2de6377cca..150f6bf2c7 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -326,7 +326,12 @@ static void prop_get_fdt(Object *obj, Visitor *v, const char *name, return; } } + visit_check_list(v, &err); visit_end_list(v, NULL); + if (err) { + error_propagate(errp, err); + return; + } break; } default: diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h index 962ba1df35..e87709db5c 100644 --- a/include/qapi/visitor-impl.h +++ b/include/qapi/visitor-impl.h @@ -61,6 +61,9 @@ struct Visitor /* Must be set */ GenericList *(*next_list)(Visitor *v, GenericList *tail, size_t size); + /* Optional; intended for input visitors */ + void (*check_list)(Visitor *v, Error **errp); + /* Must be set */ void (*end_list)(Visitor *v, void **list); diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index 7c91a50a4c..1a1b62012b 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -369,6 +369,19 @@ void visit_start_list(Visitor *v, const char *name, GenericList **list, */ GenericList *visit_next_list(Visitor *v, GenericList *tail, size_t size); +/* + * Prepare for completing a list visit. + * + * @errp obeys typical error usage, and reports failures such as + * unvisited list tail remaining in the input stream. + * + * Should be called prior to visit_end_list() if all other + * intermediate visit steps were successful, to allow the visitor one + * last chance to report errors. May be skipped on a cleanup path, + * where there is no need to check for further errors. + */ +void visit_check_list(Visitor *v, Error **errp); + /* * Complete a list visit started earlier. * diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index c50dc4bbbf..026d25b767 100644 --- a/qapi/opts-visitor.c +++ b/qapi/opts-visitor.c @@ -272,6 +272,16 @@ opts_next_list(Visitor *v, GenericList *tail, size_t size) } +static void +opts_check_list(Visitor *v, Error **errp) +{ + /* + * FIXME should set error when unvisited elements remain. Mostly + * harmless, as the generated visits always visit all elements. + */ +} + + static void opts_end_list(Visitor *v, void **obj) { @@ -539,6 +549,7 @@ opts_visitor_new(const QemuOpts *opts) ov->visitor.start_list = &opts_start_list; ov->visitor.next_list = &opts_next_list; + ov->visitor.check_list = &opts_check_list; ov->visitor.end_list = &opts_end_list; ov->visitor.type_int64 = &opts_type_int64; diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c index e6e93f02e6..43a09d147d 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qapi-visit-core.c @@ -90,6 +90,14 @@ GenericList *visit_next_list(Visitor *v, GenericList *tail, size_t size) return v->next_list(v, tail, size); } +void visit_check_list(Visitor *v, Error **errp) +{ + trace_visit_check_list(v); + if (v->check_list) { + v->check_list(v, errp); + } +} + void visit_end_list(Visitor *v, void **obj) { trace_visit_end_list(v, obj); diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c index eafcdf4625..34065ba7dd 100644 --- a/qapi/qobject-input-visitor.c +++ b/qapi/qobject-input-visitor.c @@ -51,7 +51,8 @@ static QObjectInputVisitor *to_qiv(Visitor *v) return container_of(v, QObjectInputVisitor, visitor); } -static const char *full_name(QObjectInputVisitor *qiv, const char *name) +static const char *full_name_nth(QObjectInputVisitor *qiv, const char *name, + int n) { StackObject *so; char buf[32]; @@ -63,8 +64,10 @@ static const char *full_name(QObjectInputVisitor *qiv, const char *name) } QSLIST_FOREACH(so , &qiv->stack, node) { - if (qobject_type(so->obj) == QTYPE_QDICT) { - g_string_prepend(qiv->errname, name); + if (n) { + n--; + } else if (qobject_type(so->obj) == QTYPE_QDICT) { + g_string_prepend(qiv->errname, name ?: ""); g_string_prepend_c(qiv->errname, '.'); } else { snprintf(buf, sizeof(buf), "[%u]", so->index); @@ -72,18 +75,24 @@ static const char *full_name(QObjectInputVisitor *qiv, const char *name) } name = so->name; } + assert(!n); if (name) { g_string_prepend(qiv->errname, name); } else if (qiv->errname->str[0] == '.') { g_string_erase(qiv->errname, 0, 1); - } else { + } else if (!qiv->errname->str[0]) { return ""; } return qiv->errname->str; } +static const char *full_name(QObjectInputVisitor *qiv, const char *name) +{ + return full_name_nth(qiv, name, 0); +} + static QObject *qobject_input_try_get_object(QObjectInputVisitor *qiv, const char *name, bool consume) @@ -260,15 +269,30 @@ static GenericList *qobject_input_next_list(Visitor *v, GenericList *tail, size_t size) { QObjectInputVisitor *qiv = to_qiv(v); - StackObject *so = QSLIST_FIRST(&qiv->stack); + StackObject *tos = QSLIST_FIRST(&qiv->stack); + + assert(tos && tos->obj && qobject_type(tos->obj) == QTYPE_QLIST); - if (!so->entry) { + if (!tos->entry) { return NULL; } tail->next = g_malloc0(size); return tail->next; } +static void qobject_input_check_list(Visitor *v, Error **errp) +{ + QObjectInputVisitor *qiv = to_qiv(v); + StackObject *tos = QSLIST_FIRST(&qiv->stack); + + assert(tos && tos->obj && qobject_type(tos->obj) == QTYPE_QLIST); + + if (tos->entry) { + error_setg(errp, "Only %u list elements expected in %s", + tos->index + 1, full_name_nth(qiv, NULL, 1)); + } +} + static void qobject_input_start_alternate(Visitor *v, const char *name, GenericAlternate **obj, size_t size, @@ -471,6 +495,7 @@ Visitor *qobject_input_visitor_new(QObject *obj) 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.check_list = qobject_input_check_list; v->visitor.end_list = qobject_input_pop; v->visitor.start_alternate = qobject_input_start_alternate; v->visitor.type_int64 = qobject_input_type_int64; diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c index f126cd95a9..806b01ae3a 100644 --- a/qapi/string-input-visitor.c +++ b/qapi/string-input-visitor.c @@ -170,6 +170,35 @@ static GenericList *next_list(Visitor *v, GenericList *tail, size_t size) return tail->next; } +static void check_list(Visitor *v, Error **errp) +{ + const StringInputVisitor *siv = to_siv(v); + Range *r; + GList *cur_range; + + if (!siv->ranges || !siv->cur_range) { + return; + } + + r = siv->cur_range->data; + if (!r) { + return; + } + + if (!range_contains(r, siv->cur)) { + cur_range = g_list_next(siv->cur_range); + if (!cur_range) { + return; + } + r = cur_range->data; + if (!r) { + return; + } + } + + error_setg(errp, "Range contains too many values"); +} + static void end_list(Visitor *v, void **obj) { StringInputVisitor *siv = to_siv(v); @@ -318,6 +347,7 @@ Visitor *string_input_visitor_new(const char *str) v->visitor.type_number = parse_type_number; v->visitor.start_list = start_list; v->visitor.next_list = next_list; + v->visitor.check_list = check_list; v->visitor.end_list = end_list; v->visitor.free = string_input_free; diff --git a/qapi/trace-events b/qapi/trace-events index 9cbb61b2bd..339cacf0ad 100644 --- a/qapi/trace-events +++ b/qapi/trace-events @@ -8,6 +8,7 @@ visit_end_struct(void *v, void *obj) "v=%p obj=%p" visit_start_list(void *v, const char *name, void *obj, size_t size) "v=%p name=%s obj=%p size=%zu" visit_next_list(void *v, void *tail, size_t size) "v=%p tail=%p size=%zu" +visit_check_list(void *v) "v=%p" visit_end_list(void *v, void *obj) "v=%p obj=%p" visit_start_alternate(void *v, const char *name, void *obj, size_t size, bool promote_int) "v=%p name=%s obj=%p size=%zu promote_int=%d" diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 96f2491c16..330b9f321b 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -133,6 +133,9 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error } } + if (!err) { + visit_check_list(v, &err); + } visit_end_list(v, (void **)obj); if (err && visit_is_input(v)) { qapi_free_%(c_name)s(*obj); diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c index d0f764699b..b93fd330a8 100644 --- a/tests/test-opts-visitor.c +++ b/tests/test-opts-visitor.c @@ -199,8 +199,8 @@ test_opts_range_unvisited(void) g_assert_cmpint(tail->value, ==, 1); tail = (intList *)visit_next_list(v, (GenericList *)tail, sizeof(*list)); g_assert(tail); + visit_check_list(v, &error_abort); /* BUG: unvisited tail not reported */ visit_end_list(v, (void **)&list); - /* BUG: unvisited tail not reported; actually not reportable by design */ visit_check_struct(v, &error_abort); visit_end_struct(v, NULL); diff --git a/tests/test-qobject-input-visitor.c b/tests/test-qobject-input-visitor.c index 9f3a826353..87d4a77e4a 100644 --- a/tests/test-qobject-input-visitor.c +++ b/tests/test-qobject-input-visitor.c @@ -933,6 +933,7 @@ static void test_visitor_in_fail_list(TestInputVisitorData *data, const void *unused) { int64_t i64 = -1; + Error *err = NULL; Visitor *v; /* Unvisited list tail */ @@ -944,14 +945,16 @@ static void test_visitor_in_fail_list(TestInputVisitorData *data, g_assert_cmpint(i64, ==, 1); visit_type_int(v, NULL, &i64, &error_abort); g_assert_cmpint(i64, ==, 2); + visit_check_list(v, &err); + error_free_or_abort(&err); visit_end_list(v, NULL); - /* BUG: unvisited tail not reported; actually not reportable by design */ } static void test_visitor_in_fail_list_nested(TestInputVisitorData *data, const void *unused) { int64_t i64 = -1; + Error *err = NULL; Visitor *v; /* Unvisited nested list tail */ @@ -964,8 +967,10 @@ static void test_visitor_in_fail_list_nested(TestInputVisitorData *data, visit_start_list(v, NULL, NULL, 0, &error_abort); visit_type_int(v, NULL, &i64, &error_abort); g_assert_cmpint(i64, ==, 1); + visit_check_list(v, &err); + error_free_or_abort(&err); visit_end_list(v, NULL); - /* BUG: unvisited tail not reported; actually not reportable by design */ + visit_check_list(v, &error_abort); visit_end_list(v, NULL); } diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c index 70cee65cd0..fbe380acbe 100644 --- a/tests/test-string-input-visitor.c +++ b/tests/test-string-input-visitor.c @@ -169,8 +169,10 @@ static void test_visitor_in_intList(TestInputVisitorData *data, g_assert_cmpint(tail->value, ==, 2); tail = (int64List *)visit_next_list(v, (GenericList *)tail, sizeof(*res)); g_assert(tail); + + visit_check_list(v, &err); + error_free_or_abort(&err); visit_end_list(v, (void **)&res); - /* BUG: unvisited tail not reported; actually not reportable by design */ qapi_free_int64List(res); } -- cgit 1.4.1 From a9416dc62c36079b93b4951c894a0b15e53bb38c Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 3 Mar 2017 13:32:46 +0100 Subject: tests: Cover input visit beyond end of list When you try to visit beyond the end of a list, the qobject input visitor crashes, and the string visitor screws returns garbage. The generated list visits never go beyond the list end, but manual visits could. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <1488544368-30622-27-git-send-email-armbru@redhat.com> --- tests/test-opts-visitor.c | 39 ++++++++++++++++++++++++++++++++++++++ tests/test-qobject-input-visitor.c | 10 ++++++++++ tests/test-string-input-visitor.c | 16 ++++++++++++++++ 3 files changed, 65 insertions(+) (limited to 'tests/test-qobject-input-visitor.c') diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c index b93fd330a8..2238f8efe5 100644 --- a/tests/test-opts-visitor.c +++ b/tests/test-opts-visitor.c @@ -210,6 +210,43 @@ test_opts_range_unvisited(void) qemu_opts_del(opts); } +static void +test_opts_range_beyond(void) +{ + Error *err = NULL; + intList *list = NULL; + intList *tail; + QemuOpts *opts; + Visitor *v; + int64_t val; + + opts = qemu_opts_parse(qemu_find_opts("userdef"), "ilist=0", false, + &error_abort); + + v = opts_visitor_new(opts); + + visit_start_struct(v, NULL, NULL, 0, &error_abort); + + /* Would be simpler if the visitor genuinely supported virtual walks */ + visit_start_list(v, "ilist", (GenericList **)&list, sizeof(*list), + &error_abort); + tail = list; + visit_type_int(v, NULL, &tail->value, &error_abort); + g_assert_cmpint(tail->value, ==, 0); + tail = (intList *)visit_next_list(v, (GenericList *)tail, sizeof(*tail)); + g_assert(!tail); + visit_type_int(v, NULL, &val, &err); + error_free_or_abort(&err); + visit_end_list(v, (void **)&list); + + visit_check_struct(v, &error_abort); + visit_end_struct(v, NULL); + + qapi_free_intList(list); + visit_free(v); + qemu_opts_del(opts); +} + int main(int argc, char **argv) { @@ -303,6 +340,8 @@ main(int argc, char **argv) g_test_add_func("/visitor/opts/range/unvisited", test_opts_range_unvisited); + g_test_add_func("/visitor/opts/range/beyond", + test_opts_range_beyond); g_test_run(); return 0; diff --git a/tests/test-qobject-input-visitor.c b/tests/test-qobject-input-visitor.c index 87d4a77e4a..8011baaa38 100644 --- a/tests/test-qobject-input-visitor.c +++ b/tests/test-qobject-input-visitor.c @@ -948,6 +948,16 @@ static void test_visitor_in_fail_list(TestInputVisitorData *data, visit_check_list(v, &err); error_free_or_abort(&err); visit_end_list(v, NULL); + + /* Visit beyond end of list */ + v = visitor_input_test_init(data, "[]"); + + visit_start_list(v, NULL, NULL, 0, &error_abort); +#if 0 /* FIXME crash */ + visit_type_int(v, NULL, &i64, &err); + error_free_or_abort(&err); +#endif + visit_end_list(v, NULL); } static void test_visitor_in_fail_list_nested(TestInputVisitorData *data, diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c index fbe380acbe..6db850bc89 100644 --- a/tests/test-string-input-visitor.c +++ b/tests/test-string-input-visitor.c @@ -123,6 +123,7 @@ static void test_visitor_in_intList(TestInputVisitorData *data, int64List *res = NULL; int64List *tail; Visitor *v; + int64_t val; /* Valid lists */ @@ -175,6 +176,21 @@ static void test_visitor_in_intList(TestInputVisitorData *data, visit_end_list(v, (void **)&res); qapi_free_int64List(res); + + /* Visit beyond end of list */ + v = visitor_input_test_init(data, "0"); + + visit_start_list(v, NULL, (GenericList **)&res, sizeof(*res), + &error_abort); + tail = res; + visit_type_int64(v, NULL, &tail->value, &err); + g_assert_cmpint(tail->value, ==, 0); + visit_type_int64(v, NULL, &val, &err); + g_assert_cmpint(val, ==, 1); /* BUG */ + visit_check_list(v, &error_abort); + visit_end_list(v, (void **)&res); + + qapi_free_int64List(res); } static void test_visitor_in_bool(TestInputVisitorData *data, -- cgit 1.4.1 From 1f41a645b65530859bf5984aa08e103bb452b473 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 3 Mar 2017 13:32:47 +0100 Subject: qapi: Fix object input visit beyond end of list Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <1488544368-30622-28-git-send-email-armbru@redhat.com> --- qapi/qobject-input-visitor.c | 11 ++++++++--- tests/test-qobject-input-visitor.c | 2 -- 2 files changed, 8 insertions(+), 5 deletions(-) (limited to 'tests/test-qobject-input-visitor.c') diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c index 34065ba7dd..d192727e0b 100644 --- a/qapi/qobject-input-visitor.c +++ b/qapi/qobject-input-visitor.c @@ -122,10 +122,15 @@ static QObject *qobject_input_try_get_object(QObjectInputVisitor *qiv, } else { assert(qobject_type(qobj) == QTYPE_QLIST); assert(!name); - ret = qlist_entry_obj(tos->entry); - assert(ret); + if (tos->entry) { + ret = qlist_entry_obj(tos->entry); + if (consume) { + tos->entry = qlist_next(tos->entry); + } + } else { + ret = NULL; + } if (consume) { - tos->entry = qlist_next(tos->entry); tos->index++; } } diff --git a/tests/test-qobject-input-visitor.c b/tests/test-qobject-input-visitor.c index 8011baaa38..94305f58ca 100644 --- a/tests/test-qobject-input-visitor.c +++ b/tests/test-qobject-input-visitor.c @@ -953,10 +953,8 @@ static void test_visitor_in_fail_list(TestInputVisitorData *data, v = visitor_input_test_init(data, "[]"); visit_start_list(v, NULL, NULL, 0, &error_abort); -#if 0 /* FIXME crash */ visit_type_int(v, NULL, &i64, &err); error_free_or_abort(&err); -#endif visit_end_list(v, NULL); } -- cgit 1.4.1