From b2cd5b925c529d91fae5fe8373490714e1a8fe6c Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 20 Mar 2017 13:55:44 +0100 Subject: [PATCH 01/17] test-keyval: Tweaks to improve list coverage We have a negative test case for a list index with leading zero. Add positive ones. Tweak the test case for list index greater or equal the number of elements: test "equal" instead of "greater" to guard against off-by-one mistakes. Signed-off-by: Markus Armbruster Message-Id: <1490014548-15083-2-git-send-email-armbru@redhat.com> Reviewed-by: Eric Blake --- tests/test-keyval.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test-keyval.c b/tests/test-keyval.c index 71288b082c..e97f6d554d 100644 --- a/tests/test-keyval.c +++ b/tests/test-keyval.c @@ -218,14 +218,14 @@ static void test_keyval_parse_list(void) QDECREF(qdict); /* Multiple indexes, last one wins */ - qdict = keyval_parse("list.1=goner,list.0=null,list.1=eins,list.2=zwei", + qdict = keyval_parse("list.1=goner,list.0=null,list.01=eins,list.2=zwei", NULL, &error_abort); g_assert_cmpint(qdict_size(qdict), ==, 1); check_list012(qdict_get_qlist(qdict, "list")); QDECREF(qdict); /* List at deeper nesting */ - qdict = keyval_parse("a.list.1=eins,a.list.0=null,a.list.2=zwei", + qdict = keyval_parse("a.list.1=eins,a.list.00=null,a.list.2=zwei", NULL, &error_abort); g_assert_cmpint(qdict_size(qdict), ==, 1); sub_qdict = qdict_get_qdict(qdict, "a"); @@ -242,7 +242,7 @@ static void test_keyval_parse_list(void) g_assert(!qdict); /* Missing list indexes */ - qdict = keyval_parse("list.2=lonely", NULL, &err); + qdict = keyval_parse("list.1=lonely", NULL, &err); error_free_or_abort(&err); g_assert(!qdict); qdict = keyval_parse("list.0=null,list.2=eins,list.02=zwei", NULL, &err); From fae425d74f20d45a3c0bea4b7403b16f7b659e25 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 20 Mar 2017 13:55:45 +0100 Subject: [PATCH 02/17] keyval: Improve some comments Signed-off-by: Markus Armbruster Message-Id: <1490014548-15083-3-git-send-email-armbru@redhat.com> Reviewed-by: Eric Blake --- util/keyval.c | 47 +++++++++++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/util/keyval.c b/util/keyval.c index f646b36821..46cd540c5a 100644 --- a/util/keyval.c +++ b/util/keyval.c @@ -21,22 +21,36 @@ * * Semantics defined by reduction to JSON: * - * key-vals is a tree of objects and arrays rooted at object R - * where for each key-val = key-fragment . ... = val in key-vals - * R op key-fragment op ... = val' - * where (left-associative) op is - * array subscript L[key-fragment] for numeric key-fragment - * member reference L.key-fragment otherwise - * val' is val with ',,' replaced by ',' - * and only R may be empty. + * key-vals specifies a JSON object, i.e. a tree whose root is an + * object, inner nodes other than the root are objects or arrays, + * and leaves are strings. * - * Duplicate keys are permitted; all but the last one are ignored. + * Each key-val = key-fragment '.' ... '=' val specifies a path from + * root to a leaf (left of '='), and the leaf's value (right of + * '='). * - * The equations must have a solution. Counter-example: a.b=1,a=2 - * doesn't have one, because R.a must be an object to satisfy a.b=1 - * and a string to satisfy a=2. + * A path from the root is defined recursively: + * L '.' key-fragment is a child of the node denoted by path L + * key-fragment is a child of the tree root + * If key-fragment is numeric, the parent is an array and the child + * is its key-fragment-th member, counting from zero. + * Else, the parent is an object, and the child is its member named + * key-fragment. * - * Key-fragments must be valid QAPI names or consist only of digits. + * This constrains inner nodes to be either array or object. The + * constraints must be satisfiable. Counter-example: a.b=1,a=2 is + * not, because root.a must be an object to satisfy a.b=1 and a + * string to satisfy a=2. + * + * Array subscripts can occur in any order, but the set of + * subscripts must not have gaps. For instance, a.1=v is not okay, + * because root.a[0] is missing. + * + * If multiple key-val denote the same leaf, the last one determines + * the value. + * + * Key-fragments must be valid QAPI names or consist only of decimal + * digits. * * The length of any key-fragment must be between 1 and 127. * @@ -64,8 +78,8 @@ /* * Convert @key to a list index. - * Convert all leading digits to a (non-negative) number, capped at - * INT_MAX. + * Convert all leading decimal digits to a (non-negative) number, + * capped at INT_MAX. * If @end is non-null, assign a pointer to the first character after * the number to *@end. * Else, fail if any characters follow. @@ -337,7 +351,8 @@ static QObject *keyval_listify(QDict *cur, GSList *key_of_cur, Error **errp) } /* - * Make a list from @elt[], reporting any missing elements. + * Make a list from @elt[], reporting the first missing element, + * if any. * If we dropped an index >= nelt in the previous loop, this loop * will run into the sentinel and report index @nelt missing. */ From 599c156bac7fd7f80be9d0e958e1aa19ad3dab57 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 20 Mar 2017 13:55:46 +0100 Subject: [PATCH 03/17] test-keyval: Cover alternate and 'any' type Signed-off-by: Markus Armbruster Message-Id: <1490014548-15083-4-git-send-email-armbru@redhat.com> Reviewed-by: Eric Blake --- tests/Makefile.include | 2 +- tests/test-keyval.c | 53 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/tests/Makefile.include b/tests/Makefile.include index 402e71cf06..86f9490c7c 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -736,7 +736,7 @@ tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o $(test-util-obj-y) \ $(chardev-obj-y) tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o $(test-util-obj-y) -tests/test-keyval$(EXESUF): tests/test-keyval.o $(test-util-obj-y) +tests/test-keyval$(EXESUF): tests/test-keyval.o $(test-util-obj-y) $(test-qapi-obj-y) tests/test-write-threshold$(EXESUF): tests/test-write-threshold.o $(test-block-obj-y) tests/test-netfilter$(EXESUF): tests/test-netfilter.o $(qtest-obj-y) tests/test-filter-mirror$(EXESUF): tests/test-filter-mirror.o $(qtest-obj-y) diff --git a/tests/test-keyval.c b/tests/test-keyval.c index e97f6d554d..ba19560a22 100644 --- a/tests/test-keyval.c +++ b/tests/test-keyval.c @@ -14,6 +14,7 @@ #include "qapi/error.h" #include "qapi/qmp/qstring.h" #include "qapi/qobject-input-visitor.h" +#include "test-qapi-visit.h" #include "qemu/cutils.h" #include "qemu/option.h" @@ -608,6 +609,56 @@ static void test_keyval_visit_optional(void) visit_free(v); } +static void test_keyval_visit_alternate(void) +{ + Error *err = NULL; + Visitor *v; + QDict *qdict; + AltNumStr *ans; + AltNumInt *ani; + + /* + * Can't do scalar alternate variants other than string. You get + * the string variant if there is one, else an error. + */ + qdict = keyval_parse("a=1,b=2", NULL, &error_abort); + v = qobject_input_visitor_new_keyval(QOBJECT(qdict)); + QDECREF(qdict); + visit_start_struct(v, NULL, NULL, 0, &error_abort); + visit_type_AltNumStr(v, "a", &ans, &error_abort); + g_assert_cmpint(ans->type, ==, QTYPE_QSTRING); + g_assert_cmpstr(ans->u.s, ==, "1"); + visit_type_AltNumInt(v, "a", &ani, &err); + error_free_or_abort(&err); + visit_end_struct(v, NULL); + visit_free(v); +} + +static void test_keyval_visit_any(void) +{ + Visitor *v; + QDict *qdict; + QObject *any; + QList *qlist; + QString *qstr; + + qdict = keyval_parse("a.0=null,a.1=1", NULL, &error_abort); + v = qobject_input_visitor_new_keyval(QOBJECT(qdict)); + QDECREF(qdict); + visit_start_struct(v, NULL, NULL, 0, &error_abort); + visit_type_any(v, "a", &any, &error_abort); + qlist = qobject_to_qlist(any); + g_assert(qlist); + qstr = qobject_to_qstring(qlist_pop(qlist)); + g_assert_cmpstr(qstring_get_str(qstr), ==, "null"); + qstr = qobject_to_qstring(qlist_pop(qlist)); + g_assert_cmpstr(qstring_get_str(qstr), ==, "1"); + g_assert(qlist_empty(qlist)); + visit_check_struct(v, &error_abort); + visit_end_struct(v, NULL); + visit_free(v); +} + int main(int argc, char *argv[]) { g_test_init(&argc, &argv, NULL); @@ -619,6 +670,8 @@ int main(int argc, char *argv[]) g_test_add_func("/keyval/visit/dict", test_keyval_visit_dict); g_test_add_func("/keyval/visit/list", test_keyval_visit_list); g_test_add_func("/keyval/visit/optional", test_keyval_visit_optional); + g_test_add_func("/keyval/visit/alternate", test_keyval_visit_alternate); + g_test_add_func("/keyval/visit/any", test_keyval_visit_any); g_test_run(); return 0; } From 0ee9ae7c8cda8b371b6b414c0d895feaec04f79e Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 20 Mar 2017 13:55:47 +0100 Subject: [PATCH 04/17] keyval: Document issues with 'any' and alternate types Signed-off-by: Markus Armbruster Message-Id: <1490014548-15083-5-git-send-email-armbru@redhat.com> Reviewed-by: Eric Blake --- util/keyval.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/util/keyval.c b/util/keyval.c index 46cd540c5a..93d5db6b59 100644 --- a/util/keyval.c +++ b/util/keyval.c @@ -61,6 +61,16 @@ * "key absent" already means "optional object/array absent", which * isn't the same as "empty object/array present". * + * Design flaw: scalar values can only be strings; there is no way to + * denote numbers, true, false or null. The special QObject input + * visitor returned by qobject_input_visitor_new_keyval() mostly hides + * this by automatically converting strings to the type the visitor + * expects. Breaks down for alternate types and type 'any', where the + * visitor's expectation isn't clear. Code visiting such types needs + * to do the conversion itself, but only when using this keyval + * visitor. Awkward. Alternate types without a string member don't + * work at all. + * * Additional syntax for use with an implied key: * * key-vals-ik = val-no-key [ ',' key-vals ] From e94630d3adcb2ff8b70b95f5dd3873ecadfbd19b Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 20 Mar 2017 13:55:48 +0100 Subject: [PATCH 05/17] MAINTAINERS: Add myself for files I touched recently Signed-off-by: Markus Armbruster Message-Id: <1490014548-15083-6-git-send-email-armbru@redhat.com> Reviewed-by: Eric Blake --- MAINTAINERS | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 779c429059..c60235eaf6 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1231,6 +1231,15 @@ M: Samuel Thibault S: Maintained F: backends/baum.c +Command line option argument parsing +M: Markus Armbruster +S: Supported +F: include/qemu/option.h +F: tests/test-keyval.c +F: tests/test-qemu-opts.c +F: util/keyval.c +F: util/qemu-option.c + Coverity model M: Markus Armbruster S: Supported @@ -1365,7 +1374,9 @@ X: include/qapi/qmp/ F: include/qapi/qmp/dispatch.h F: tests/qapi-schema/ F: tests/test-*-visitor.c +F: tests/test-qapi-*.c F: tests/test-qmp-*.c +F: tests/test-visitor-serialization.c F: scripts/qapi* F: docs/qapi* T: git git://repo.or.cz/qemu/armbru.git qapi-next From 4afeeb57a16ab28727ffe1d644b691f9c83b8207 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 20 Mar 2017 14:11:50 +0100 Subject: [PATCH 06/17] qapi: Drop excessive Make dependencies on qapi2texi.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When qapi2texi.py changes, we regenerate everything QAPI. Screwed up in commit 56e8bdd. Signed-off-by: Markus Armbruster Message-Id: <1490015515-25851-2-git-send-email-armbru@redhat.com> Reviewed-by: Marc-André Lureau --- Makefile | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index f4f90dfad6..6c359b2f86 100644 --- a/Makefile +++ b/Makefile @@ -392,7 +392,6 @@ qemu-ga$(EXESUF): QEMU_CFLAGS += -I qga/qapi-generated gen-out-type = $(subst .,-,$(suffix $@)) qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py -qapi-py += $(SRC_PATH)/scripts/qapi2texi.py qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\ $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) @@ -701,10 +700,12 @@ qemu-monitor-info.texi: $(SRC_PATH)/hmp-commands-info.hx $(SRC_PATH)/scripts/hxt qemu-img-cmds.texi: $(SRC_PATH)/qemu-img-cmds.hx $(SRC_PATH)/scripts/hxtool $(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -t < $< > $@,"GEN","$@") -docs/qemu-qmp-qapi.texi: $(qapi-modules) $(qapi-py) +docs/qemu-qmp-qapi.texi docs/qemu-ga-qapi.texi: $(SRC_PATH)/scripts/qapi2texi.py $(qapi-py) + +docs/qemu-qmp-qapi.texi: $(qapi-modules) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi2texi.py $< > $@,"GEN","$@") -docs/qemu-ga-qapi.texi: $(SRC_PATH)/qga/qapi-schema.json $(qapi-py) +docs/qemu-ga-qapi.texi: $(SRC_PATH)/qga/qapi-schema.json $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi2texi.py $< > $@,"GEN","$@") qemu.1: qemu-doc.texi qemu-options.texi qemu-monitor.texi qemu-monitor-info.texi From e8ba07ea9af6e71d6b1d946df7bcc5f5d6b0d22f Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 20 Mar 2017 14:11:51 +0100 Subject: [PATCH 07/17] qapi2texi: Fix to actually fail when 'doc-required' is false MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Messed up in commit bc52d03. Signed-off-by: Markus Armbruster Message-Id: <1490015515-25851-3-git-send-email-armbru@redhat.com> Reviewed-by: Marc-André Lureau --- scripts/qapi2texi.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py index 8eed11a60c..5c4db78574 100755 --- a/scripts/qapi2texi.py +++ b/scripts/qapi2texi.py @@ -292,6 +292,7 @@ def main(argv): if not qapi.doc_required: print >>sys.stderr, ("%s: need pragma 'doc-required' " "to generate documentation" % argv[0]) + sys.exit(1) print texi_schema(schema) From 32b8a2ad618b20ff9eb3ac9ce0418a9239916571 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 20 Mar 2017 14:11:52 +0100 Subject: [PATCH 08/17] qapi: Drop unused QAPIDoc member optional MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unused since commit aa964b7 "qapi2texi: Convert to QAPISchemaVisitor" Signed-off-by: Markus Armbruster Message-Id: <1490015515-25851-4-git-send-email-armbru@redhat.com> Reviewed-by: Marc-André Lureau --- scripts/qapi.py | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/qapi.py b/scripts/qapi.py index e88c047c2e..6c4d554165 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -106,7 +106,6 @@ class QAPIDoc(object): self.name = name # the list of lines for this section self.content = [] - self.optional = False def append(self, line): self.content.append(line) From 818c3318332a0b0ba5931d8d74d2260fb0328a71 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 20 Mar 2017 14:11:53 +0100 Subject: [PATCH 09/17] tests/qapi-schema: Make test-qapi.py print docs again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test-qapi.py used to print the internal representation of doc comments (commit 3313b61). This went away when we dropped the doc comments in positive tests (commit 87c16dc). Bring it back, because I'm going to add real positive doc comment tests. Signed-off-by: Markus Armbruster Message-Id: <1490015515-25851-5-git-send-email-armbru@redhat.com> Reviewed-by: Marc-André Lureau --- tests/qapi-schema/test-qapi.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py index ef74e2c4c8..c7724d3437 100644 --- a/tests/qapi-schema/test-qapi.py +++ b/tests/qapi-schema/test-qapi.py @@ -55,3 +55,14 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor): schema = QAPISchema(sys.argv[1]) schema.visit(QAPISchemaTestVisitor()) + +for doc in schema.docs: + if doc.symbol: + print 'doc symbol=%s' % doc.symbol + else: + print 'doc freeform' + print ' body=\n%s' % doc.body + for arg, section in doc.args.iteritems(): + print ' arg=%s\n%s' % (arg, section) + for section in doc.sections: + print ' section=%s\n%s' % (section.name, section) From 80d1f2e4a5df739842806610fb462753dde27b9a Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 20 Mar 2017 14:11:54 +0100 Subject: [PATCH 10/17] tests/qapi-schema: Systematic positive doc comment tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We have a number of negative tests, but we don't have systematic positive coverage. Fix that. Signed-off-by: Markus Armbruster Message-Id: <1490015515-25851-6-git-send-email-armbru@redhat.com> Reviewed-by: Marc-André Lureau --- tests/Makefile.include | 13 +- tests/qapi-schema/doc-good.err | 0 tests/qapi-schema/doc-good.exit | 1 + tests/qapi-schema/doc-good.json | 136 ++++++++++++++++++ tests/qapi-schema/doc-good.out | 148 +++++++++++++++++++ tests/qapi-schema/doc-good.texi | 243 ++++++++++++++++++++++++++++++++ 6 files changed, 537 insertions(+), 4 deletions(-) create mode 100644 tests/qapi-schema/doc-good.err create mode 100644 tests/qapi-schema/doc-good.exit create mode 100644 tests/qapi-schema/doc-good.json create mode 100644 tests/qapi-schema/doc-good.out create mode 100644 tests/qapi-schema/doc-good.texi diff --git a/tests/Makefile.include b/tests/Makefile.include index 86f9490c7c..f3de81fcfb 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -379,6 +379,7 @@ qapi-schema += doc-duplicated-since.json qapi-schema += doc-empty-arg.json qapi-schema += doc-empty-section.json qapi-schema += doc-empty-symbol.json +qapi-schema += doc-good.json qapi-schema += doc-interleaved-section.json qapi-schema += doc-invalid-end.json qapi-schema += doc-invalid-end2.json @@ -607,6 +608,9 @@ $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-int $(gen-out-type) -o tests -p "test-" $<, \ "GEN","$@") +tests/qapi-schema/doc-good.test.texi: $(SRC_PATH)/tests/qapi-schema/doc-good.json $(SRC_PATH)/scripts/qapi2texi.py $(qapi-py) + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi2texi.py $< > $@,"GEN","$@") + tests/test-string-output-visitor$(EXESUF): tests/test-string-output-visitor.o $(test-qapi-obj-y) tests/test-string-input-visitor$(EXESUF): tests/test-string-input-visitor.o $(test-qapi-obj-y) tests/test-qmp-event$(EXESUF): tests/test-qmp-event.o $(test-qapi-obj-y) @@ -856,9 +860,6 @@ QEMU_IOTESTS_HELPERS-$(CONFIG_LINUX) = tests/qemu-iotests/socket_scm_helper$(EXE check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF) qemu-io$(EXESUF) $(QEMU_IOTESTS_HELPERS-y) $< -.PHONY: check-tests/test-qapi.py -check-tests/test-qapi.py: tests/test-qapi.py - .PHONY: $(patsubst %, check-%, $(check-qapi-schema-y)) $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json $(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts \ @@ -871,10 +872,14 @@ $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json @perl -p -e 's|\Q$(SRC_PATH)\E/||g' $*.test.err | diff -q $(SRC_PATH)/$*.err - @diff -q $(SRC_PATH)/$*.exit $*.test.exit +.PHONY: check-tests/qapi-schema/doc-good.texi +check-tests/qapi-schema/doc-good.texi: tests/qapi-schema/doc-good.test.texi + @diff -q $(SRC_PATH)/tests/qapi-schema/doc-good.texi $< + # Consolidated targets .PHONY: check-qapi-schema check-qtest check-unit check check-clean -check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y)) +check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y)) check-tests/qapi-schema/doc-good.texi check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS)) check-unit: $(patsubst %,check-%, $(check-unit-y)) check-block: $(patsubst %,check-%, $(check-block-y)) diff --git a/tests/qapi-schema/doc-good.err b/tests/qapi-schema/doc-good.err new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/qapi-schema/doc-good.exit b/tests/qapi-schema/doc-good.exit new file mode 100644 index 0000000000..573541ac97 --- /dev/null +++ b/tests/qapi-schema/doc-good.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json new file mode 100644 index 0000000000..cfdc0a8a81 --- /dev/null +++ b/tests/qapi-schema/doc-good.json @@ -0,0 +1,136 @@ +# -*- Mode: Python -*- +# Positive QAPI doc comment tests + +{ 'pragma': { 'doc-required': true } } + +## +# = Section +# +# == Subsection +# +# *strong* _with emphasis_ +# @var {in braces} +# * List item one +# - Two, multiple +# lines +# +# 3. Three +# Still in list +# +# Not in list +# - Second list +# Note: still in list +# +# Note: not in list +# 1. Third list +# is numbered +# +# - another item +# +# | example +# | multiple lines +# +# Returns: the King +# Since: the first age +# Notes: +# +# 1. Lorem ipsum dolor sit amet +# +# 2. Ut enim ad minim veniam +# +# Duis aute irure dolor +# +# Example: +# +# -> in +# <- out +# Examples: +# - *verbatim* +# - {braces} +## + +## +# @Enum: +# == Produces *invalid* texinfo +# @one: The _one_ {and only} +# +# @two is undocumented +## +{ 'enum': 'Enum', 'data': [ 'one', 'two' ] } + +## +# @Base: +# @base1: +# the first member +## +{ 'struct': 'Base', 'data': { 'base1': 'Enum' } } + +## +# @Variant1: +# A paragraph +# +# Another paragraph (but no @var: line) +## +{ 'struct': 'Variant1', 'data': { 'var1': 'str' } } + +## +# @Variant2: +## +{ 'struct': 'Variant2', 'data': {} } + +## +# @Object: +## +{ 'union': 'Object', + 'base': 'Base', + 'discriminator': 'base1', + 'data': { 'one': 'Variant1', 'two': 'Variant2' } } + +## +# @SugaredUnion: +## +{ 'union': 'SugaredUnion', + 'data': { 'one': 'Variant1', 'two': 'Variant2' } } + +## +# == Another subsection +## + +## +# @cmd: +# @arg1: the first argument +# +# @arg2: the second +# argument +# Note: @arg3 is undocumented +# Returns: @Object +# TODO: frobnicate +# Notes: +# - Lorem ipsum dolor sit amet +# - Ut enim ad minim veniam +# +# Duis aute irure dolor +# Example: +# +# -> in +# <- out +# Examples: +# - *verbatim* +# - {braces} +# Since: 2.10 +## +{ 'command': 'cmd', + 'data': { 'arg1': 'int', '*arg2': 'str', 'arg3': 'bool' }, + 'returns': 'Object' } + +## +# @cmd-boxed: +# If you're bored enough to read this, go see a video of boxed cats +# Example: +# +# -> in +# +# <- out +## +{ 'command': 'cmd-boxed', 'boxed': true, + 'data': 'Object' } diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out new file mode 100644 index 0000000000..70c1252408 --- /dev/null +++ b/tests/qapi-schema/doc-good.out @@ -0,0 +1,148 @@ +object Base + member base1: Enum optional=False +enum Enum ['one', 'two'] +object Object + base Base + tag base1 + case one: Variant1 + case two: Variant2 +enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool'] + prefix QTYPE +object SugaredUnion + member type: SugaredUnionKind optional=False + tag type + case one: q_obj_Variant1-wrapper + case two: q_obj_Variant2-wrapper +enum SugaredUnionKind ['one', 'two'] +object Variant1 + member var1: str optional=False +object Variant2 +command cmd q_obj_cmd-arg -> Object + gen=True success_response=True boxed=False +command cmd-boxed Object -> None + gen=True success_response=True boxed=True +object q_empty +object q_obj_Variant1-wrapper + member data: Variant1 optional=False +object q_obj_Variant2-wrapper + member data: Variant2 optional=False +object q_obj_cmd-arg + member arg1: int optional=False + member arg2: str optional=True + member arg3: bool optional=False +doc freeform + body= += Section + +== Subsection + +*strong* _with emphasis_ +@var {in braces} +* List item one +- Two, multiple +lines + +3. Three +Still in list + +Not in list +- Second list +Note: still in list + +Note: not in list +1. Third list +is numbered + +- another item + +| example +| multiple lines + +Returns: the King +Since: the first age +Notes: + +1. Lorem ipsum dolor sit amet + +2. Ut enim ad minim veniam + +Duis aute irure dolor + +Example: + +-> in +<- out +Examples: +- *verbatim* +- {braces} +doc symbol=Enum + body= +== Produces *invalid* texinfo + arg=one +The _one_ {and only} + arg=two + + section= +@two is undocumented +doc symbol=Base + body= + + arg=base1 +the first member +doc symbol=Variant1 + body= +A paragraph + +Another paragraph (but no @var: line) + arg=var1 + +doc symbol=Variant2 + body= + +doc symbol=Object + body= + +doc symbol=SugaredUnion + body= + + arg=type + +doc freeform + body= +== Another subsection +doc symbol=cmd + body= + + arg=arg1 +the first argument + arg=arg2 +the second +argument + arg=arg3 + + section=Note +@arg3 is undocumented + section=Returns +@Object + section=TODO +frobnicate + section=Notes +- Lorem ipsum dolor sit amet +- Ut enim ad minim veniam + +Duis aute irure dolor + section=Example +-> in +<- out + section=Examples +- *verbatim* +- {braces} + section=Since +2.10 +doc symbol=cmd-boxed + body= +If you're bored enough to read this, go see a video of boxed cats + section=Example +-> in + +<- out diff --git a/tests/qapi-schema/doc-good.texi b/tests/qapi-schema/doc-good.texi new file mode 100644 index 0000000000..1160aaf851 --- /dev/null +++ b/tests/qapi-schema/doc-good.texi @@ -0,0 +1,243 @@ +@section Section + +@subsection Subsection + +@emph{strong} @emph{with emphasis} +@code{var} @{in braces@} +@itemize @bullet +@item +List item one +@item +Two, multiple +lines + +@item +Three +Still in list + +@end itemize + +Not in list +@itemize @minus +@item +Second list +Note: still in list + +@end itemize + +Note: not in list +@enumerate +@item +Third list +is numbered + +@item +another item + +@example +example +@end example + +@example +multiple lines +@end example + + +@end enumerate + +Returns: the King +Since: the first age +Notes: + +@enumerate +@item +Lorem ipsum dolor sit amet + +@item +Ut enim ad minim veniam + +@end enumerate + +Duis aute irure dolor + +Example: + +-> in +<- out +Examples: +@itemize @minus +@item +@emph{verbatim} +@item +@{braces@} +@end itemize + + + +@deftp {Enum} Enum + +@subsection Produces @emph{invalid} texinfo + +@b{Values:} +@table @asis +@item @code{one} +The @emph{one} @{and only@} +@item @code{two} +Not documented +@end table +@code{two} is undocumented + +@end deftp + + + +@deftp {Object} Base + + + +@b{Members:} +@table @asis +@item @code{base1: Enum} +the first member +@end table + + +@end deftp + + + +@deftp {Object} Variant1 + +A paragraph + +Another paragraph (but no @code{var}: line) + +@b{Members:} +@table @asis +@item @code{var1: string} +Not documented +@end table + + +@end deftp + + + +@deftp {Object} Variant2 + + + + +@end deftp + + + +@deftp {Object} Object + + + +@b{Members:} +@table @asis +@item The members of @code{Base} +@item The members of @code{Variant1} when @code{base1} is @t{"one"} +@item The members of @code{Variant2} when @code{base1} is @t{"two"} +@end table + + +@end deftp + + + +@deftp {Object} SugaredUnion + + + +@b{Members:} +@table @asis +@item @code{type} +One of @t{"one"}, @t{"two"} +@item @code{data: Variant1} when @code{type} is @t{"one"} +@item @code{data: Variant2} when @code{type} is @t{"two"} +@end table + + +@end deftp + + +@subsection Another subsection + + +@deftypefn Command {} cmd + + + +@b{Arguments:} +@table @asis +@item @code{arg1: int} +the first argument +@item @code{arg2: string} (optional) +the second +argument +@item @code{arg3: boolean} +Not documented +@end table + + +@b{Note:} +@code{arg3} is undocumented + +@b{Returns:} +@code{Object} + +@b{TODO:} +frobnicate + +@b{Notes:} +@itemize @minus +@item +Lorem ipsum dolor sit amet +@item +Ut enim ad minim veniam + +@end itemize + +Duis aute irure dolor + +@b{Example:} +@example +-> in +<- out +@end example + + +@b{Examples:} +@example +- *verbatim* +- @{braces@} +@end example + + +@b{Since:} +2.10 + +@end deftypefn + + + +@deftypefn Command {} cmd-boxed + +If you're bored enough to read this, go see a video of boxed cats + +@b{Arguments:} the members of @code{Object} + +@b{Example:} +@example +-> in + +<- out +@end example + + +@end deftypefn + + From c32617a1941aadfe580af6c4418ef02c9644cd44 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 20 Mar 2017 14:11:55 +0100 Subject: [PATCH 11/17] qapi2texi: Fix translation of *strong* and _emphasized_ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Markus Armbruster Message-Id: <1490015515-25851-7-git-send-email-armbru@redhat.com> Reviewed-by: Marc-André Lureau --- scripts/qapi2texi.py | 4 ++-- tests/qapi-schema/doc-good.texi | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py index 5c4db78574..9e015002ef 100755 --- a/scripts/qapi2texi.py +++ b/scripts/qapi2texi.py @@ -35,12 +35,12 @@ EXAMPLE_FMT = """@example def subst_strong(doc): """Replaces *foo* by @strong{foo}""" - return re.sub(r'\*([^*\n]+)\*', r'@emph{\1}', doc) + return re.sub(r'\*([^*\n]+)\*', r'@strong{\1}', doc) def subst_emph(doc): """Replaces _foo_ by @emph{foo}""" - return re.sub(r'\b_([^_\n]+)_\b', r' @emph{\1} ', doc) + return re.sub(r'\b_([^_\n]+)_\b', r'@emph{\1}', doc) def subst_vars(doc): diff --git a/tests/qapi-schema/doc-good.texi b/tests/qapi-schema/doc-good.texi index 1160aaf851..c410626e4a 100644 --- a/tests/qapi-schema/doc-good.texi +++ b/tests/qapi-schema/doc-good.texi @@ -2,7 +2,7 @@ @subsection Subsection -@emph{strong} @emph{with emphasis} +@strong{strong} @emph{with emphasis} @code{var} @{in braces@} @itemize @bullet @item @@ -67,7 +67,7 @@ Example: Examples: @itemize @minus @item -@emph{verbatim} +@strong{verbatim} @item @{braces@} @end itemize @@ -76,12 +76,12 @@ Examples: @deftp {Enum} Enum -@subsection Produces @emph{invalid} texinfo +@subsection Produces @strong{invalid} texinfo @b{Values:} @table @asis @item @code{one} -The @emph{one} @{and only@} +The @emph{one} @{and only@} @item @code{two} Not documented @end table From d2788227c6185c72d88ef3127e9fed41686f8e39 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 20 Mar 2017 17:13:43 +0100 Subject: [PATCH 12/17] qapi: Fix string input visitor regression for empty lists Visiting a list when input is the empty string should result in an empty list, not an error. Noticed when commit 3d089ce belatedly added tests, but simply accepted as weird then. It's actually a regression: broken in commit 74f24cb, v2.7.0. Fix it, and throw in another test case for empty string. Signed-off-by: Markus Armbruster Message-Id: <1490026424-11330-2-git-send-email-armbru@redhat.com> Reviewed-by: Michael Roth Reviewed-by: Eric Blake --- qapi/string-input-visitor.c | 4 ++++ tests/test-string-input-visitor.c | 11 ++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c index 806b01ae3a..c089491c24 100644 --- a/qapi/string-input-visitor.c +++ b/qapi/string-input-visitor.c @@ -54,6 +54,10 @@ static int parse_str(StringInputVisitor *siv, const char *name, Error **errp) return 0; } + if (!*str) { + return 0; + } + do { errno = 0; start = strtoll(str, &endptr, 0); diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c index 6db850bc89..79313a7f7a 100644 --- a/tests/test-string-input-visitor.c +++ b/tests/test-string-input-visitor.c @@ -63,6 +63,11 @@ static void test_visitor_in_int(TestInputVisitorData *data, visit_type_int(v, NULL, &res, &err); error_free_or_abort(&err); + + v = visitor_input_test_init(data, ""); + + visit_type_int(v, NULL, &res, &err); + error_free_or_abort(&err); } static void check_ilist(Visitor *v, int64_t *expected, size_t n) @@ -140,11 +145,11 @@ static void test_visitor_in_intList(TestInputVisitorData *data, v = visitor_input_test_init(data, "18446744073709551615"); check_ulist(v, expect4, ARRAY_SIZE(expect4)); - /* Empty list is invalid (weird) */ + /* Empty list */ v = visitor_input_test_init(data, ""); - visit_type_int64List(v, NULL, &res, &err); - error_free_or_abort(&err); + visit_type_int64List(v, NULL, &res, &error_abort); + g_assert(!res); /* Not a list */ From 658ae5a7b90139a6a296cd4cd83643d843964796 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 20 Mar 2017 17:13:44 +0100 Subject: [PATCH 13/17] Revert "hostmem: fix QEMU crash by 'info memdev'" This reverts commit 1454d33f0507cb54d62ed80f494884157c9e7130. The string input visitor regression fixed in the previous commit made visit_type_uint16List() fail on empty input. query_memdev() calls it via object_property_get_uint16List(). Because it doesn't expect it to fail, it passes &error_abort, and duly crashes. Commit 1454d33 "fixes" this crash by making host_memory_backend_get_host_nodes() return a list containing just MAX_NODES instead of the empty list. Papers over the regression, and leads to bogus "info memdev" output, as shown below; revert. I suspect that if we had bisected the crash back then, we would have found and fixed the actual bug instead of papering over it. To reproduce, run HMP command "info memdev" with $ qemu-system-x86_64 --nodefaults -S -display none -monitor stdio -object memory-backend-ram,id=mem1,size=4k With this commit, "info memdev" prints memory backend: mem1 size: 4096 merge: true dump: true prealloc: false policy: default host nodes: exactly like before commit 74f24cb. Between commit 1454d33 and this commit, it prints memory backend: mem1 size: 4096 merge: true dump: true prealloc: false policy: default host nodes: 128 The last line is bogus. Between commit 74f24cb and 1454d33, it crashes like this: Unexpected error in parse_str() at /work/armbru/tmp/qemu/qapi/string-input-visitor.c:126: Parameter 'null' expects an int64 value or range Aborted (core dumped) Cc: Xiao Guangrong Cc: Paolo Bonzini Signed-off-by: Markus Armbruster Message-Id: <1490026424-11330-3-git-send-email-armbru@redhat.com> Reviewed-by: Michael Roth Reviewed-by: Eric Blake --- backends/hostmem.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/backends/hostmem.c b/backends/hostmem.c index 162c2187d8..89feb9ed75 100644 --- a/backends/hostmem.c +++ b/backends/hostmem.c @@ -64,14 +64,6 @@ out: error_propagate(errp, local_err); } -static uint16List **host_memory_append_node(uint16List **node, - unsigned long value) -{ - *node = g_malloc0(sizeof(**node)); - (*node)->value = value; - return &(*node)->next; -} - static void host_memory_backend_get_host_nodes(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) @@ -82,23 +74,25 @@ host_memory_backend_get_host_nodes(Object *obj, Visitor *v, const char *name, unsigned long value; value = find_first_bit(backend->host_nodes, MAX_NODES); - - node = host_memory_append_node(node, value); - if (value == MAX_NODES) { - goto out; + return; } + *node = g_malloc0(sizeof(**node)); + (*node)->value = value; + node = &(*node)->next; + do { value = find_next_bit(backend->host_nodes, MAX_NODES, value + 1); if (value == MAX_NODES) { break; } - node = host_memory_append_node(node, value); + *node = g_malloc0(sizeof(**node)); + (*node)->value = value; + node = &(*node)->next; } while (true); -out: visit_type_uint16List(v, name, &host_nodes, errp); } From 4bc0c94da40055dc68645847fa8bcddef47fedd5 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 21 Mar 2017 18:44:50 +0100 Subject: [PATCH 14/17] test-qobject-input-visitor: Cover visit_type_uint64() The new test demonstrates known bugs: integers between INT64_MAX+1 and UINT64_MAX rejected, and integers between INT64_MIN and -1 are accepted modulo 2^64. Signed-off-by: Markus Armbruster Message-Id: <1490118290-6133-1-git-send-email-armbru@redhat.com> Reviewed-by: Eric Blake --- tests/test-qobject-input-visitor.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/test-qobject-input-visitor.c b/tests/test-qobject-input-visitor.c index 6eb48fee7b..f965743b6e 100644 --- a/tests/test-qobject-input-visitor.c +++ b/tests/test-qobject-input-visitor.c @@ -116,6 +116,34 @@ static void test_visitor_in_int(TestInputVisitorData *data, g_assert_cmpint(res, ==, value); } +static void test_visitor_in_uint(TestInputVisitorData *data, + const void *unused) +{ + Error *err = NULL; + uint64_t res = 0; + int value = 42; + Visitor *v; + + v = visitor_input_test_init(data, "%d", value); + + visit_type_uint64(v, NULL, &res, &error_abort); + g_assert_cmpuint(res, ==, (uint64_t)value); + + /* BUG: value between INT64_MIN and -1 accepted modulo 2^64 */ + + v = visitor_input_test_init(data, "%d", -value); + + visit_type_uint64(v, NULL, &res, &error_abort); + g_assert_cmpuint(res, ==, (uint64_t)-value); + + /* BUG: value between INT64_MAX+1 and UINT64_MAX rejected */ + + v = visitor_input_test_init(data, "18446744073709551574"); + + visit_type_uint64(v, NULL, &res, &err); + error_free_or_abort(&err); +} + static void test_visitor_in_int_overflow(TestInputVisitorData *data, const void *unused) { @@ -1225,6 +1253,8 @@ int main(int argc, char **argv) input_visitor_test_add("/visitor/input/int", NULL, test_visitor_in_int); + input_visitor_test_add("/visitor/input/uint", + NULL, test_visitor_in_uint); input_visitor_test_add("/visitor/input/int_overflow", NULL, test_visitor_in_int_overflow); input_visitor_test_add("/visitor/input/int_keyval", From 76861f6befc50c30cb71e84b372dc91dd26656a4 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Wed, 22 Mar 2017 09:45:23 -0500 Subject: [PATCH 15/17] tests: Expose regression in QemuOpts visitor Commit 15c2f669e broke the ability of the QemuOpts visitor to flag extra input parameters, but the regression went unnoticed because of missing testsuite coverage. Add a test to cover this; take the approach already used in 9cb8ef3 of adding a test that passes (to avoid breaking bisection) but marks with BUG the behavior that we don't like, so that the actual impact of the fix in a later patch is easier to see. CC: qemu-stable@nongnu.org Signed-off-by: Eric Blake Reviewed-by: Michael Roth Message-Id: <20170322144525.18964-2-eblake@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- tests/test-opts-visitor.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c index 2238f8efe5..8e0dda5f68 100644 --- a/tests/test-opts-visitor.c +++ b/tests/test-opts-visitor.c @@ -247,6 +247,24 @@ test_opts_range_beyond(void) qemu_opts_del(opts); } +static void +test_opts_dict_unvisited(void) +{ + QemuOpts *opts; + Visitor *v; + UserDefOptions *userdef; + + opts = qemu_opts_parse(qemu_find_opts("userdef"), "i64x=0,bogus=1", false, + &error_abort); + + v = opts_visitor_new(opts); + /* BUG: bogus should be diagnosed */ + visit_type_UserDefOptions(v, NULL, &userdef, &error_abort); + visit_free(v); + qemu_opts_del(opts); + qapi_free_UserDefOptions(userdef); +} + int main(int argc, char **argv) { @@ -343,6 +361,8 @@ main(int argc, char **argv) g_test_add_func("/visitor/opts/range/beyond", test_opts_range_beyond); + g_test_add_func("/visitor/opts/dict/unvisited", test_opts_dict_unvisited); + g_test_run(); return 0; } From 9a6d1acb3e752e1e0b693cc0491fa12875b76be0 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Wed, 22 Mar 2017 09:45:24 -0500 Subject: [PATCH 16/17] qom: Avoid unvisited 'id'/'qom-type' in user_creatable_add_opts A regression in commit 15c2f669e caused us to silently ignore excess input to the QemuOpts visitor. Later, commit ea4641 accidentally abused that situation, by removing "qom-type" and "id" from the corresponding QDict but leaving them defined in the QemuOpts, when using the pair of containers to create a user-defined object. Note that since we are already traversing two separate items (a QDict and a QemuOpts), we are already able to flag bogus arguments, as in: $ ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio -object memory-backend-ram,id=mem1,size=4k,bogus=huh qemu-system-x86_64: -object memory-backend-ram,id=mem1,size=4k,bogus=huh: Property '.bogus' not found So the only real concern is that when we re-enable strict checking in the QemuOpts visitor, we do not want to start flagging the two leftover keys as unvisited. Rearrange the code to clean out the QemuOpts listing in advance, rather than removing items from the QDict. Since "qom-type" is usually an automatic implicit default, we don't have to restore it (this does mean that once instantiated, QemuOpts is not necessarily an accurate representation of the original command line - but this is not the first place to do that); however "id" has to be put back (requiring us to cast away a const). [As a side note, hmp_object_add() turns a QDict into a QemuOpts, then calls user_creatable_add_opts() which converts QemuOpts into a new QDict. There are probably a lot of wasteful conversions like this, but cleaning them up is a much bigger task than the immediate regression fix.] CC: qemu-stable@nongnu.org Signed-off-by: Eric Blake Message-Id: <20170322144525.18964-3-eblake@redhat.com> Tested-by: Laurent Vivier Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- qom/object_interfaces.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c index 03a95c3276..9c271ad32a 100644 --- a/qom/object_interfaces.c +++ b/qom/object_interfaces.c @@ -114,7 +114,7 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error **errp) QDict *pdict; Object *obj; const char *id = qemu_opts_id(opts); - const char *type = qemu_opt_get(opts, "qom-type"); + char *type = qemu_opt_get_del(opts, "qom-type"); if (!type) { error_setg(errp, QERR_MISSING_PARAMETER, "qom-type"); @@ -122,17 +122,19 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error **errp) } if (!id) { error_setg(errp, QERR_MISSING_PARAMETER, "id"); + g_free(type); return NULL; } + qemu_opts_set_id(opts, NULL); pdict = qemu_opts_to_qdict(opts, NULL); - qdict_del(pdict, "qom-type"); - qdict_del(pdict, "id"); v = opts_visitor_new(opts); obj = user_creatable_add_type(type, id, pdict, v, errp); visit_free(v); + qemu_opts_set_id(opts, (char *) id); + g_free(type); QDECREF(pdict); return obj; } From 21f88d021d0d2b4ecee8f6cd6ca63a943a3ce71d Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Wed, 22 Mar 2017 09:45:25 -0500 Subject: [PATCH 17/17] qapi: Fix QemuOpts visitor regression on unvisited input An off-by-one in commit 15c2f669e meant that we were failing to check for unparsed input in all QemuOpts visitors. Recent testsuite additions show that fixing the obvious bug with bogus fields will also fix the case of an incomplete list visit; update the tests to match the new behavior. Simple testcase: ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio -numa node,size=1g failed to diagnose that 'size' is not a valid argument to -numa, and now once again reports: qemu-system-x86_64: -numa node,size=1g: Invalid parameter 'size' See also https://bugzilla.redhat.com/show_bug.cgi?id=1434666 CC: qemu-stable@nongnu.org Signed-off-by: Eric Blake Reviewed-by: Michael Roth Tested-by: Laurent Vivier Message-Id: <20170322144525.18964-4-eblake@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- qapi/opts-visitor.c | 6 +++--- tests/test-opts-visitor.c | 13 ++++++++----- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index 026d25b767..324b197495 100644 --- a/qapi/opts-visitor.c +++ b/qapi/opts-visitor.c @@ -164,7 +164,7 @@ opts_check_struct(Visitor *v, Error **errp) GHashTableIter iter; GQueue *any; - if (ov->depth > 0) { + if (ov->depth > 1) { return; } @@ -276,8 +276,8 @@ 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. + * Unvisited list elements will be reported later when checking + * whether unvisited struct members remain. */ } diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c index 8e0dda5f68..23e897061c 100644 --- a/tests/test-opts-visitor.c +++ b/tests/test-opts-visitor.c @@ -175,6 +175,7 @@ expect_u64_max(OptsVisitorFixture *f, gconstpointer test_data) static void test_opts_range_unvisited(void) { + Error *err = NULL; intList *list = NULL; intList *tail; QemuOpts *opts; @@ -199,10 +200,11 @@ 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_check_list(v, &error_abort); /* unvisited tail ignored until... */ visit_end_list(v, (void **)&list); - visit_check_struct(v, &error_abort); + visit_check_struct(v, &err); /* ...here */ + error_free_or_abort(&err); visit_end_struct(v, NULL); qapi_free_intList(list); @@ -250,6 +252,7 @@ test_opts_range_beyond(void) static void test_opts_dict_unvisited(void) { + Error *err = NULL; QemuOpts *opts; Visitor *v; UserDefOptions *userdef; @@ -258,11 +261,11 @@ test_opts_dict_unvisited(void) &error_abort); v = opts_visitor_new(opts); - /* BUG: bogus should be diagnosed */ - visit_type_UserDefOptions(v, NULL, &userdef, &error_abort); + visit_type_UserDefOptions(v, NULL, &userdef, &err); + error_free_or_abort(&err); visit_free(v); qemu_opts_del(opts); - qapi_free_UserDefOptions(userdef); + g_assert(!userdef); } int