* [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs
@ 2014-08-06 1:14 Eric Blake
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 01/14] qapi: consistent whitespace in tests/Makefile Eric Blake
` (16 more replies)
0 siblings, 17 replies; 29+ messages in thread
From: Eric Blake @ 2014-08-06 1:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng, Markus Armbruster, wenchaoqemu, Luiz Capitulino
According to this email:
https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg00708.html
we want to repurpose 'data': { 'name': {dict...} } in qapi files
for future use of designating default values of optional parameters.
But to do that, we must first nuke existing use of that syntax for
declaring nested structs. Enhancing the testsuite while at it
never hurts.
v3:
No code changes, but fix mis-send of 11-14 [Peter]
v2:
New patches: 1-2, 5-9
consistent TAB usage in Makefile [Fam]
catch more bad coding constructs, and test them
avoid code duplication in type validity checks (patch 14 [former 7] is simpler because of patch 9)
Eric Blake (14):
qapi: consistent whitespace in tests/Makefile
qapi: ignore files created during make check
qapi: add some enum tests
qapi: better error message for bad enum
qapi: add some expr tests
qapi: require valid expressions
qapi: add some type check tests
qapi: add expr_name() helper
qapi: add check_type helper function
qapi: merge UserDefTwo and UserDefNested in tests
qapi: drop tests for inline subtypes
qapi: drop inline subtype in query-version
qapi: drop inline subtype in query-pci
qapi: drop support for inline subtypes
hmp.c | 28 ++++----
hw/pci/pci.c | 42 ++++++------
qapi-schema.json | 90 ++++++++++++++++++--------
qapi/common.json | 26 ++++++--
qmp.c | 9 +--
scripts/qapi-commands.py | 8 +--
scripts/qapi-event.py | 4 +-
scripts/qapi-types.py | 9 +--
scripts/qapi-visit.py | 37 ++---------
scripts/qapi.py | 96 ++++++++++++++++++++++------
tests/.gitignore | 3 +
tests/Makefile | 37 ++++++-----
tests/qapi-schema/data-array-empty.err | 0
tests/qapi-schema/data-array-empty.exit | 1 +
tests/qapi-schema/data-array-empty.json | 1 +
tests/qapi-schema/data-array-empty.out | 3 +
tests/qapi-schema/data-array-unknown.err | 1 +
tests/qapi-schema/data-array-unknown.exit | 1 +
tests/qapi-schema/data-array-unknown.json | 1 +
tests/qapi-schema/data-array-unknown.out | 0
tests/qapi-schema/data-int.err | 1 +
tests/qapi-schema/data-int.exit | 1 +
tests/qapi-schema/data-int.json | 1 +
tests/qapi-schema/data-int.out | 0
tests/qapi-schema/data-unknown.err | 1 +
tests/qapi-schema/data-unknown.exit | 1 +
tests/qapi-schema/data-unknown.json | 1 +
tests/qapi-schema/data-unknown.out | 0
tests/qapi-schema/double-type.err | 1 +
tests/qapi-schema/double-type.exit | 1 +
tests/qapi-schema/double-type.json | 1 +
tests/qapi-schema/double-type.out | 0
tests/qapi-schema/enum-empty.err | 0
tests/qapi-schema/enum-empty.exit | 1 +
tests/qapi-schema/enum-empty.json | 1 +
tests/qapi-schema/enum-empty.out | 3 +
tests/qapi-schema/enum-missing-data.err | 1 +
tests/qapi-schema/enum-missing-data.exit | 1 +
tests/qapi-schema/enum-missing-data.json | 1 +
tests/qapi-schema/enum-missing-data.out | 0
tests/qapi-schema/enum-wrong-data.err | 1 +
tests/qapi-schema/enum-wrong-data.exit | 1 +
tests/qapi-schema/enum-wrong-data.json | 1 +
tests/qapi-schema/enum-wrong-data.out | 0
tests/qapi-schema/event-nest-struct.err | 2 +-
tests/qapi-schema/indented-expr.json | 4 +-
tests/qapi-schema/indented-expr.out | 2 +-
tests/qapi-schema/missing-type.err | 1 +
tests/qapi-schema/missing-type.exit | 1 +
tests/qapi-schema/missing-type.json | 1 +
tests/qapi-schema/missing-type.out | 0
tests/qapi-schema/nested-struct-data.err | 1 +
tests/qapi-schema/nested-struct-data.exit | 1 +
tests/qapi-schema/nested-struct-data.json | 3 +
tests/qapi-schema/nested-struct-data.out | 0
tests/qapi-schema/nested-struct-returns.err | 1 +
tests/qapi-schema/nested-struct-returns.exit | 1 +
tests/qapi-schema/nested-struct-returns.json | 2 +
tests/qapi-schema/nested-struct-returns.out | 0
tests/qapi-schema/qapi-schema-test.json | 18 +++---
tests/qapi-schema/qapi-schema-test.out | 10 +--
tests/qapi-schema/returns-array-bad.err | 1 +
tests/qapi-schema/returns-array-bad.exit | 1 +
tests/qapi-schema/returns-array-bad.json | 1 +
tests/qapi-schema/returns-array-bad.out | 0
tests/qapi-schema/returns-int.err | 0
tests/qapi-schema/returns-int.exit | 1 +
tests/qapi-schema/returns-int.json | 1 +
tests/qapi-schema/returns-int.out | 3 +
tests/qapi-schema/returns-unknown.err | 1 +
tests/qapi-schema/returns-unknown.exit | 1 +
tests/qapi-schema/returns-unknown.json | 1 +
tests/qapi-schema/returns-unknown.out | 0
tests/test-qmp-commands.c | 35 +++++-----
tests/test-qmp-input-strict.c | 19 +++---
tests/test-qmp-input-visitor.c | 25 +++++---
tests/test-qmp-output-visitor.c | 64 ++++++++++---------
tests/test-visitor-serialization.c | 84 +++++++++++++-----------
78 files changed, 433 insertions(+), 270 deletions(-)
create mode 100644 tests/qapi-schema/data-array-empty.err
create mode 100644 tests/qapi-schema/data-array-empty.exit
create mode 100644 tests/qapi-schema/data-array-empty.json
create mode 100644 tests/qapi-schema/data-array-empty.out
create mode 100644 tests/qapi-schema/data-array-unknown.err
create mode 100644 tests/qapi-schema/data-array-unknown.exit
create mode 100644 tests/qapi-schema/data-array-unknown.json
create mode 100644 tests/qapi-schema/data-array-unknown.out
create mode 100644 tests/qapi-schema/data-int.err
create mode 100644 tests/qapi-schema/data-int.exit
create mode 100644 tests/qapi-schema/data-int.json
create mode 100644 tests/qapi-schema/data-int.out
create mode 100644 tests/qapi-schema/data-unknown.err
create mode 100644 tests/qapi-schema/data-unknown.exit
create mode 100644 tests/qapi-schema/data-unknown.json
create mode 100644 tests/qapi-schema/data-unknown.out
create mode 100644 tests/qapi-schema/double-type.err
create mode 100644 tests/qapi-schema/double-type.exit
create mode 100644 tests/qapi-schema/double-type.json
create mode 100644 tests/qapi-schema/double-type.out
create mode 100644 tests/qapi-schema/enum-empty.err
create mode 100644 tests/qapi-schema/enum-empty.exit
create mode 100644 tests/qapi-schema/enum-empty.json
create mode 100644 tests/qapi-schema/enum-empty.out
create mode 100644 tests/qapi-schema/enum-missing-data.err
create mode 100644 tests/qapi-schema/enum-missing-data.exit
create mode 100644 tests/qapi-schema/enum-missing-data.json
create mode 100644 tests/qapi-schema/enum-missing-data.out
create mode 100644 tests/qapi-schema/enum-wrong-data.err
create mode 100644 tests/qapi-schema/enum-wrong-data.exit
create mode 100644 tests/qapi-schema/enum-wrong-data.json
create mode 100644 tests/qapi-schema/enum-wrong-data.out
create mode 100644 tests/qapi-schema/missing-type.err
create mode 100644 tests/qapi-schema/missing-type.exit
create mode 100644 tests/qapi-schema/missing-type.json
create mode 100644 tests/qapi-schema/missing-type.out
create mode 100644 tests/qapi-schema/nested-struct-data.err
create mode 100644 tests/qapi-schema/nested-struct-data.exit
create mode 100644 tests/qapi-schema/nested-struct-data.json
create mode 100644 tests/qapi-schema/nested-struct-data.out
create mode 100644 tests/qapi-schema/nested-struct-returns.err
create mode 100644 tests/qapi-schema/nested-struct-returns.exit
create mode 100644 tests/qapi-schema/nested-struct-returns.json
create mode 100644 tests/qapi-schema/nested-struct-returns.out
create mode 100644 tests/qapi-schema/returns-array-bad.err
create mode 100644 tests/qapi-schema/returns-array-bad.exit
create mode 100644 tests/qapi-schema/returns-array-bad.json
create mode 100644 tests/qapi-schema/returns-array-bad.out
create mode 100644 tests/qapi-schema/returns-int.err
create mode 100644 tests/qapi-schema/returns-int.exit
create mode 100644 tests/qapi-schema/returns-int.json
create mode 100644 tests/qapi-schema/returns-int.out
create mode 100644 tests/qapi-schema/returns-unknown.err
create mode 100644 tests/qapi-schema/returns-unknown.exit
create mode 100644 tests/qapi-schema/returns-unknown.json
create mode 100644 tests/qapi-schema/returns-unknown.out
--
1.9.3
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH v3 01/14] qapi: consistent whitespace in tests/Makefile
2014-08-06 1:14 [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs Eric Blake
@ 2014-08-06 1:14 ` Eric Blake
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 02/14] qapi: ignore files created during make check Eric Blake
` (15 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Eric Blake @ 2014-08-06 1:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng, Markus Armbruster, wenchaoqemu, Luiz Capitulino
This file had a mix of TAB vs. 8-space indentation; given that
it is a Makefile, TAB is more idiomatic even though in these
particular cases the choice of whitespace didn't matter.
* tests/Makefile (check-qapi-schema-y, GENERATED_HEADERS)
(test-qapi-obj-y): Consistent tab indentation.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tests/Makefile | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/tests/Makefile b/tests/Makefile
index 4b2e1bb..aa17e44 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -190,23 +190,23 @@ $(foreach target,$(SYSEMU_TARGET_LIST), \
$(eval check-qtest-$(target)-y += tests/qom-test$(EXESUF)))
check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
- comments.json empty.json funny-char.json indented-expr.json \
- missing-colon.json missing-comma-list.json \
- missing-comma-object.json non-objects.json \
- qapi-schema-test.json quoted-structural-chars.json \
- trailing-comma-list.json trailing-comma-object.json \
- unclosed-list.json unclosed-object.json unclosed-string.json \
- duplicate-key.json union-invalid-base.json flat-union-no-base.json \
- flat-union-invalid-discriminator.json \
- flat-union-invalid-branch-key.json flat-union-reverse-define.json \
- flat-union-string-discriminator.json \
- include-simple.json include-relpath.json include-format-err.json \
- include-non-file.json include-no-file.json include-before-err.json \
- include-nested-err.json include-self-cycle.json include-cycle.json \
- include-repetition.json event-nest-struct.json)
+ comments.json empty.json funny-char.json indented-expr.json \
+ missing-colon.json missing-comma-list.json \
+ missing-comma-object.json non-objects.json \
+ qapi-schema-test.json quoted-structural-chars.json \
+ trailing-comma-list.json trailing-comma-object.json \
+ unclosed-list.json unclosed-object.json unclosed-string.json \
+ duplicate-key.json union-invalid-base.json flat-union-no-base.json \
+ flat-union-invalid-discriminator.json \
+ flat-union-invalid-branch-key.json flat-union-reverse-define.json \
+ flat-union-string-discriminator.json \
+ include-simple.json include-relpath.json include-format-err.json \
+ include-non-file.json include-no-file.json include-before-err.json \
+ include-nested-err.json include-self-cycle.json include-cycle.json \
+ include-repetition.json event-nest-struct.json)
GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h \
- tests/test-qmp-commands.h tests/test-qapi-event.h
+ tests/test-qmp-commands.h tests/test-qapi-event.h
test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
tests/check-qlist.o tests/check-qfloat.o tests/check-qjson.o \
@@ -218,7 +218,7 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
tests/test-opts-visitor.o tests/test-qmp-event.o
test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o \
- tests/test-qapi-event.o
+ tests/test-qapi-event.o
$(test-obj-y): QEMU_INCLUDES += -Itests
QEMU_CFLAGS += -I$(SRC_PATH)/tests
--
1.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH v3 02/14] qapi: ignore files created during make check
2014-08-06 1:14 [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs Eric Blake
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 01/14] qapi: consistent whitespace in tests/Makefile Eric Blake
@ 2014-08-06 1:14 ` Eric Blake
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 03/14] qapi: add some enum tests Eric Blake
` (14 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Eric Blake @ 2014-08-06 1:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng, Markus Armbruster, wenchaoqemu, Luiz Capitulino
After an in-tree build and run of 'make check-{qapi-schema,unit}',
I noticed some leftover files.
* tests/.gitignore: Ignore more testsuite droppings.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tests/.gitignore | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tests/.gitignore b/tests/.gitignore
index c71c110..e2e4957 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -14,11 +14,14 @@ test-int128
test-iov
test-mul64
test-opts-visitor
+test-qapi-event.[ch]
test-qapi-types.[ch]
test-qapi-visit.[ch]
test-qdev-global-props
+test-qemu-opts
test-qmp-commands
test-qmp-commands.h
+test-qmp-event
test-qmp-input-strict
test-qmp-input-visitor
test-qmp-marshal.c
--
1.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH v3 03/14] qapi: add some enum tests
2014-08-06 1:14 [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs Eric Blake
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 01/14] qapi: consistent whitespace in tests/Makefile Eric Blake
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 02/14] qapi: ignore files created during make check Eric Blake
@ 2014-08-06 1:14 ` Eric Blake
2014-08-14 9:23 ` Markus Armbruster
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 04/14] qapi: better error message for bad enum Eric Blake
` (13 subsequent siblings)
16 siblings, 1 reply; 29+ messages in thread
From: Eric Blake @ 2014-08-06 1:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng, Markus Armbruster, wenchaoqemu, Luiz Capitulino
Demonstrate that the qapi generator doesn't deal well with enums
that aren't up to par. Later patches will update the expected
results as the generator is made stricter.
* tests/qapi-schema/enum-empty.*: New files.
* tests/qapi-schema/enum-missing-data.*: Likewise.
* tests/qapi-schema/enum-wrong-data.*: Likewise.
* tests/Makefile (check-qapi-schema-y): Run them.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tests/Makefile | 3 ++-
tests/qapi-schema/enum-empty.err | 0
tests/qapi-schema/enum-empty.exit | 1 +
tests/qapi-schema/enum-empty.json | 1 +
tests/qapi-schema/enum-empty.out | 3 +++
tests/qapi-schema/enum-missing-data.err | 6 ++++++
tests/qapi-schema/enum-missing-data.exit | 1 +
tests/qapi-schema/enum-missing-data.json | 1 +
tests/qapi-schema/enum-missing-data.out | 0
tests/qapi-schema/enum-wrong-data.err | 0
tests/qapi-schema/enum-wrong-data.exit | 1 +
tests/qapi-schema/enum-wrong-data.json | 1 +
tests/qapi-schema/enum-wrong-data.out | 3 +++
13 files changed, 20 insertions(+), 1 deletion(-)
create mode 100644 tests/qapi-schema/enum-empty.err
create mode 100644 tests/qapi-schema/enum-empty.exit
create mode 100644 tests/qapi-schema/enum-empty.json
create mode 100644 tests/qapi-schema/enum-empty.out
create mode 100644 tests/qapi-schema/enum-missing-data.err
create mode 100644 tests/qapi-schema/enum-missing-data.exit
create mode 100644 tests/qapi-schema/enum-missing-data.json
create mode 100644 tests/qapi-schema/enum-missing-data.out
create mode 100644 tests/qapi-schema/enum-wrong-data.err
create mode 100644 tests/qapi-schema/enum-wrong-data.exit
create mode 100644 tests/qapi-schema/enum-wrong-data.json
create mode 100644 tests/qapi-schema/enum-wrong-data.out
diff --git a/tests/Makefile b/tests/Makefile
index aa17e44..fbf6909 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -190,7 +190,8 @@ $(foreach target,$(SYSEMU_TARGET_LIST), \
$(eval check-qtest-$(target)-y += tests/qom-test$(EXESUF)))
check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
- comments.json empty.json funny-char.json indented-expr.json \
+ comments.json empty.json enum-empty.json enum-missing-data.json \
+ enum-wrong-data.json funny-char.json indented-expr.json \
missing-colon.json missing-comma-list.json \
missing-comma-object.json non-objects.json \
qapi-schema-test.json quoted-structural-chars.json \
diff --git a/tests/qapi-schema/enum-empty.err b/tests/qapi-schema/enum-empty.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/enum-empty.exit b/tests/qapi-schema/enum-empty.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/enum-empty.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/enum-empty.json b/tests/qapi-schema/enum-empty.json
new file mode 100644
index 0000000..f550aa0
--- /dev/null
+++ b/tests/qapi-schema/enum-empty.json
@@ -0,0 +1 @@
+{ 'enum': 'MyEnum', 'data': [ ] }
diff --git a/tests/qapi-schema/enum-empty.out b/tests/qapi-schema/enum-empty.out
new file mode 100644
index 0000000..3b75c16
--- /dev/null
+++ b/tests/qapi-schema/enum-empty.out
@@ -0,0 +1,3 @@
+[OrderedDict([('enum', 'MyEnum'), ('data', [])])]
+[{'enum_name': 'MyEnum', 'enum_values': []}]
+[]
diff --git a/tests/qapi-schema/enum-missing-data.err b/tests/qapi-schema/enum-missing-data.err
new file mode 100644
index 0000000..1fec213
--- /dev/null
+++ b/tests/qapi-schema/enum-missing-data.err
@@ -0,0 +1,6 @@
+Traceback (most recent call last):
+ File "tests/qapi-schema/test-qapi.py", line 19, in <module>
+ exprs = parse_schema(sys.argv[1])
+ File "scripts/qapi.py", line 339, in parse_schema
+ add_enum(expr['enum'], expr['data'])
+KeyError: 'data'
diff --git a/tests/qapi-schema/enum-missing-data.exit b/tests/qapi-schema/enum-missing-data.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/enum-missing-data.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/enum-missing-data.json b/tests/qapi-schema/enum-missing-data.json
new file mode 100644
index 0000000..6e2acd8
--- /dev/null
+++ b/tests/qapi-schema/enum-missing-data.json
@@ -0,0 +1 @@
+{ 'enum': 'MyEnum' }
diff --git a/tests/qapi-schema/enum-missing-data.out b/tests/qapi-schema/enum-missing-data.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/enum-wrong-data.err b/tests/qapi-schema/enum-wrong-data.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/enum-wrong-data.exit b/tests/qapi-schema/enum-wrong-data.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/enum-wrong-data.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/enum-wrong-data.json b/tests/qapi-schema/enum-wrong-data.json
new file mode 100644
index 0000000..4b7e90c
--- /dev/null
+++ b/tests/qapi-schema/enum-wrong-data.json
@@ -0,0 +1 @@
+{ 'enum': 'MyEnum', 'data': { 'value': 'str' } }
diff --git a/tests/qapi-schema/enum-wrong-data.out b/tests/qapi-schema/enum-wrong-data.out
new file mode 100644
index 0000000..28d2211
--- /dev/null
+++ b/tests/qapi-schema/enum-wrong-data.out
@@ -0,0 +1,3 @@
+[OrderedDict([('enum', 'MyEnum'), ('data', OrderedDict([('value', 'str')]))])]
+[{'enum_name': 'MyEnum', 'enum_values': OrderedDict([('value', 'str')])}]
+[]
--
1.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH v3 04/14] qapi: better error message for bad enum
2014-08-06 1:14 [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs Eric Blake
` (2 preceding siblings ...)
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 03/14] qapi: add some enum tests Eric Blake
@ 2014-08-06 1:14 ` Eric Blake
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 05/14] qapi: add some expr tests Eric Blake
` (12 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Eric Blake @ 2014-08-06 1:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng, Markus Armbruster, wenchaoqemu, Luiz Capitulino
The previous commit demonstrated that the generator choked if an
enum forgot 'data', and silently ignored an enum where 'data'
was the wrong type. Fix both cases to give a sane error message.
* scripts/qapi.py (parse_schema): Avoid bad deref.
(check_exprs): Check for array on enums.
* tests/qapi-schema/enum-missing-data.err: Update expected results.
* tests/qapi-schema/enum-wrong-data.*: Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi.py | 16 ++++++++++++----
tests/qapi-schema/enum-missing-data.err | 7 +------
tests/qapi-schema/enum-wrong-data.err | 1 +
tests/qapi-schema/enum-wrong-data.exit | 2 +-
tests/qapi-schema/enum-wrong-data.out | 3 ---
5 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index f2c6d1f..1082416 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -2,7 +2,7 @@
# QAPI helper library
#
# Copyright IBM, Corp. 2011
-# Copyright (c) 2013 Red Hat Inc.
+# Copyright (c) 2013-2014 Red Hat Inc.
#
# Authors:
# Anthony Liguori <aliguori@us.ibm.com>
@@ -319,10 +319,18 @@ def check_union(expr, expr_info):
def check_exprs(schema):
for expr_elem in schema.exprs:
expr = expr_elem['expr']
+ info = expr_elem['info']
+ members = expr.get('data')
+
if expr.has_key('union'):
- check_union(expr, expr_elem['info'])
+ check_union(expr, info)
if expr.has_key('event'):
- check_event(expr, expr_elem['info'])
+ check_event(expr, info)
+ if expr.has_key('enum'):
+ if not isinstance(members, list):
+ raise QAPIExprError(info,
+ "enum '%s' requires an array for 'data'"
+ % expr.get('enum'))
def parse_schema(input_file):
try:
@@ -336,7 +344,7 @@ def parse_schema(input_file):
for expr_elem in schema.exprs:
expr = expr_elem['expr']
if expr.has_key('enum'):
- add_enum(expr['enum'], expr['data'])
+ add_enum(expr['enum'], expr.get('data'))
elif expr.has_key('union'):
add_union(expr)
elif expr.has_key('type'):
diff --git a/tests/qapi-schema/enum-missing-data.err b/tests/qapi-schema/enum-missing-data.err
index 1fec213..4912aab 100644
--- a/tests/qapi-schema/enum-missing-data.err
+++ b/tests/qapi-schema/enum-missing-data.err
@@ -1,6 +1 @@
-Traceback (most recent call last):
- File "tests/qapi-schema/test-qapi.py", line 19, in <module>
- exprs = parse_schema(sys.argv[1])
- File "scripts/qapi.py", line 339, in parse_schema
- add_enum(expr['enum'], expr['data'])
-KeyError: 'data'
+tests/qapi-schema/enum-missing-data.json:1: enum 'MyEnum' requires an array for 'data'
diff --git a/tests/qapi-schema/enum-wrong-data.err b/tests/qapi-schema/enum-wrong-data.err
index e69de29..857f0bd 100644
--- a/tests/qapi-schema/enum-wrong-data.err
+++ b/tests/qapi-schema/enum-wrong-data.err
@@ -0,0 +1 @@
+tests/qapi-schema/enum-wrong-data.json:1: enum 'MyEnum' requires an array for 'data'
diff --git a/tests/qapi-schema/enum-wrong-data.exit b/tests/qapi-schema/enum-wrong-data.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/enum-wrong-data.exit
+++ b/tests/qapi-schema/enum-wrong-data.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/enum-wrong-data.out b/tests/qapi-schema/enum-wrong-data.out
index 28d2211..e69de29 100644
--- a/tests/qapi-schema/enum-wrong-data.out
+++ b/tests/qapi-schema/enum-wrong-data.out
@@ -1,3 +0,0 @@
-[OrderedDict([('enum', 'MyEnum'), ('data', OrderedDict([('value', 'str')]))])]
-[{'enum_name': 'MyEnum', 'enum_values': OrderedDict([('value', 'str')])}]
-[]
--
1.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH v3 05/14] qapi: add some expr tests
2014-08-06 1:14 [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs Eric Blake
` (3 preceding siblings ...)
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 04/14] qapi: better error message for bad enum Eric Blake
@ 2014-08-06 1:14 ` Eric Blake
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 06/14] qapi: require valid expressions Eric Blake
` (11 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Eric Blake @ 2014-08-06 1:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng, Markus Armbruster, wenchaoqemu, Luiz Capitulino
Demonstrate that the qapi generator doesn't deal well with
expressions that aren't up to par. Later patches will improve
the expected results as the generator is made stricter.
* tests/qapi-schema/missing-type.*: New files.
* tests/qapi-schema/double-type.*: Likewise.
* tests/Makefile (check-qapi-schema-y): Run them.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tests/Makefile | 1 +
tests/qapi-schema/double-type.err | 0
tests/qapi-schema/double-type.exit | 1 +
tests/qapi-schema/double-type.json | 1 +
tests/qapi-schema/double-type.out | 3 +++
tests/qapi-schema/missing-type.err | 0
tests/qapi-schema/missing-type.exit | 1 +
tests/qapi-schema/missing-type.json | 1 +
tests/qapi-schema/missing-type.out | 3 +++
9 files changed, 11 insertions(+)
create mode 100644 tests/qapi-schema/double-type.err
create mode 100644 tests/qapi-schema/double-type.exit
create mode 100644 tests/qapi-schema/double-type.json
create mode 100644 tests/qapi-schema/double-type.out
create mode 100644 tests/qapi-schema/missing-type.err
create mode 100644 tests/qapi-schema/missing-type.exit
create mode 100644 tests/qapi-schema/missing-type.json
create mode 100644 tests/qapi-schema/missing-type.out
diff --git a/tests/Makefile b/tests/Makefile
index fbf6909..1b0c96f 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -192,6 +192,7 @@ $(foreach target,$(SYSEMU_TARGET_LIST), \
check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
comments.json empty.json enum-empty.json enum-missing-data.json \
enum-wrong-data.json funny-char.json indented-expr.json \
+ missing-type.json double-type.json \
missing-colon.json missing-comma-list.json \
missing-comma-object.json non-objects.json \
qapi-schema-test.json quoted-structural-chars.json \
diff --git a/tests/qapi-schema/double-type.err b/tests/qapi-schema/double-type.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/double-type.exit b/tests/qapi-schema/double-type.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/double-type.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/double-type.json b/tests/qapi-schema/double-type.json
new file mode 100644
index 0000000..2d4d97d
--- /dev/null
+++ b/tests/qapi-schema/double-type.json
@@ -0,0 +1 @@
+{ 'command': 'foo', 'type': 'bar', 'data': { } }
diff --git a/tests/qapi-schema/double-type.out b/tests/qapi-schema/double-type.out
new file mode 100644
index 0000000..3e244f5
--- /dev/null
+++ b/tests/qapi-schema/double-type.out
@@ -0,0 +1,3 @@
+[OrderedDict([('command', 'foo'), ('type', 'bar'), ('data', OrderedDict())])]
+[]
+[OrderedDict([('command', 'foo'), ('type', 'bar'), ('data', OrderedDict())])]
diff --git a/tests/qapi-schema/missing-type.err b/tests/qapi-schema/missing-type.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/missing-type.exit b/tests/qapi-schema/missing-type.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/missing-type.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/missing-type.json b/tests/qapi-schema/missing-type.json
new file mode 100644
index 0000000..9043aa4
--- /dev/null
+++ b/tests/qapi-schema/missing-type.json
@@ -0,0 +1 @@
+{ 'data': { } }
diff --git a/tests/qapi-schema/missing-type.out b/tests/qapi-schema/missing-type.out
new file mode 100644
index 0000000..67fd4fa
--- /dev/null
+++ b/tests/qapi-schema/missing-type.out
@@ -0,0 +1,3 @@
+[OrderedDict([('data', OrderedDict())])]
+[]
+[]
--
1.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH v3 06/14] qapi: require valid expressions
2014-08-06 1:14 [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs Eric Blake
` (4 preceding siblings ...)
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 05/14] qapi: add some expr tests Eric Blake
@ 2014-08-06 1:14 ` Eric Blake
2014-08-14 9:38 ` Markus Armbruster
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 07/14] qapi: add some type check tests Eric Blake
` (10 subsequent siblings)
16 siblings, 1 reply; 29+ messages in thread
From: Eric Blake @ 2014-08-06 1:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng, Markus Armbruster, wenchaoqemu, Luiz Capitulino
The previous patch demonstrated that the generator could get
confused if an expression had conflicting meta-types, and
silently ignored expressions that lacked a known meta-type.
Fix both cases to give a sane error message.
* scripts/qapi.py (check_exprs): Require a valid meta-type for
every expression.
* tests/qapi-schema/indented-expr.*: Use valid types.
* tests/qapi-schema/missing-type.*: Update expected output.
* tests/qapi-schema/double-type.*: Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi.py | 12 ++++++++++++
tests/qapi-schema/double-type.err | 1 +
tests/qapi-schema/double-type.exit | 2 +-
tests/qapi-schema/double-type.out | 3 ---
tests/qapi-schema/indented-expr.json | 4 ++--
tests/qapi-schema/indented-expr.out | 2 +-
tests/qapi-schema/missing-type.err | 1 +
tests/qapi-schema/missing-type.exit | 2 +-
tests/qapi-schema/missing-type.out | 3 ---
9 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 1082416..910e422 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -322,6 +322,18 @@ def check_exprs(schema):
info = expr_elem['info']
members = expr.get('data')
+ # 'include' has already been flattened; at this point, all exprs
+ # should have one of the remaining keys
+ keys = (expr.has_key('enum') + expr.has_key('union') +
+ expr.has_key('type') + expr.has_key('command') +
+ expr.has_key('event'))
+ if keys < 1:
+ raise QAPIExprError(info,
+ "Missing expression meta-type")
+ if keys > 1:
+ raise QAPIExprError(info,
+ "Conflicting expression meta-types")
+
if expr.has_key('union'):
check_union(expr, info)
if expr.has_key('event'):
diff --git a/tests/qapi-schema/double-type.err b/tests/qapi-schema/double-type.err
index e69de29..2df4a12 100644
--- a/tests/qapi-schema/double-type.err
+++ b/tests/qapi-schema/double-type.err
@@ -0,0 +1 @@
+tests/qapi-schema/double-type.json:1: Conflicting expression meta-types
diff --git a/tests/qapi-schema/double-type.exit b/tests/qapi-schema/double-type.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/double-type.exit
+++ b/tests/qapi-schema/double-type.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/double-type.out b/tests/qapi-schema/double-type.out
index 3e244f5..e69de29 100644
--- a/tests/qapi-schema/double-type.out
+++ b/tests/qapi-schema/double-type.out
@@ -1,3 +0,0 @@
-[OrderedDict([('command', 'foo'), ('type', 'bar'), ('data', OrderedDict())])]
-[]
-[OrderedDict([('command', 'foo'), ('type', 'bar'), ('data', OrderedDict())])]
diff --git a/tests/qapi-schema/indented-expr.json b/tests/qapi-schema/indented-expr.json
index d80af60..7115d31 100644
--- a/tests/qapi-schema/indented-expr.json
+++ b/tests/qapi-schema/indented-expr.json
@@ -1,2 +1,2 @@
-{ 'id' : 'eins' }
- { 'id' : 'zwei' }
+{ 'command' : 'eins' }
+ { 'command' : 'zwei' }
diff --git a/tests/qapi-schema/indented-expr.out b/tests/qapi-schema/indented-expr.out
index 98af89a..b5ce915 100644
--- a/tests/qapi-schema/indented-expr.out
+++ b/tests/qapi-schema/indented-expr.out
@@ -1,3 +1,3 @@
-[OrderedDict([('id', 'eins')]), OrderedDict([('id', 'zwei')])]
+[OrderedDict([('command', 'eins')]), OrderedDict([('command', 'zwei')])]
[]
[]
diff --git a/tests/qapi-schema/missing-type.err b/tests/qapi-schema/missing-type.err
index e69de29..af650e0 100644
--- a/tests/qapi-schema/missing-type.err
+++ b/tests/qapi-schema/missing-type.err
@@ -0,0 +1 @@
+tests/qapi-schema/missing-type.json:1: Missing expression meta-type
diff --git a/tests/qapi-schema/missing-type.exit b/tests/qapi-schema/missing-type.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/missing-type.exit
+++ b/tests/qapi-schema/missing-type.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/missing-type.out b/tests/qapi-schema/missing-type.out
index 67fd4fa..e69de29 100644
--- a/tests/qapi-schema/missing-type.out
+++ b/tests/qapi-schema/missing-type.out
@@ -1,3 +0,0 @@
-[OrderedDict([('data', OrderedDict())])]
-[]
-[]
--
1.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH v3 07/14] qapi: add some type check tests
2014-08-06 1:14 [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs Eric Blake
` (5 preceding siblings ...)
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 06/14] qapi: require valid expressions Eric Blake
@ 2014-08-06 1:14 ` Eric Blake
2014-08-14 9:47 ` Markus Armbruster
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 08/14] qapi: add expr_name() helper Eric Blake
` (9 subsequent siblings)
16 siblings, 1 reply; 29+ messages in thread
From: Eric Blake @ 2014-08-06 1:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng, Markus Armbruster, wenchaoqemu, Luiz Capitulino
Demonstrate that the qapi generator silently parses confusing
types, which may cause other errors later on. Later patches
will update the expected results as the generator is made stricter.
* tests/qapi-schema/data-array-empty.*: New files.
* tests/qapi-schema/data-array-unknown.*: Likewise.
* tests/qapi-schema/data-unknown.*: Likewise.
* tests/qapi-schema/data-int.*: Likewise.
* tests/qapi-schema/returns-unknown.*: Likewise.
* tests/qapi-schema/returns-int.*: Likewise.
* tests/qapi-schema/returns-array-bad.*: Likewise.
* tests/Makefile (check-qapi-schema-y): Run them.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tests/Makefile | 4 +++-
tests/qapi-schema/data-array-empty.err | 0
tests/qapi-schema/data-array-empty.exit | 1 +
tests/qapi-schema/data-array-empty.json | 1 +
tests/qapi-schema/data-array-empty.out | 3 +++
tests/qapi-schema/data-array-unknown.err | 0
tests/qapi-schema/data-array-unknown.exit | 1 +
tests/qapi-schema/data-array-unknown.json | 1 +
tests/qapi-schema/data-array-unknown.out | 3 +++
tests/qapi-schema/data-int.err | 0
tests/qapi-schema/data-int.exit | 1 +
tests/qapi-schema/data-int.json | 1 +
tests/qapi-schema/data-int.out | 3 +++
tests/qapi-schema/data-unknown.err | 0
tests/qapi-schema/data-unknown.exit | 1 +
tests/qapi-schema/data-unknown.json | 1 +
tests/qapi-schema/data-unknown.out | 3 +++
tests/qapi-schema/returns-array-bad.err | 0
tests/qapi-schema/returns-array-bad.exit | 1 +
tests/qapi-schema/returns-array-bad.json | 1 +
tests/qapi-schema/returns-array-bad.out | 3 +++
tests/qapi-schema/returns-int.err | 0
tests/qapi-schema/returns-int.exit | 1 +
tests/qapi-schema/returns-int.json | 1 +
tests/qapi-schema/returns-int.out | 3 +++
tests/qapi-schema/returns-unknown.err | 0
tests/qapi-schema/returns-unknown.exit | 1 +
tests/qapi-schema/returns-unknown.json | 1 +
tests/qapi-schema/returns-unknown.out | 3 +++
29 files changed, 38 insertions(+), 1 deletion(-)
create mode 100644 tests/qapi-schema/data-array-empty.err
create mode 100644 tests/qapi-schema/data-array-empty.exit
create mode 100644 tests/qapi-schema/data-array-empty.json
create mode 100644 tests/qapi-schema/data-array-empty.out
create mode 100644 tests/qapi-schema/data-array-unknown.err
create mode 100644 tests/qapi-schema/data-array-unknown.exit
create mode 100644 tests/qapi-schema/data-array-unknown.json
create mode 100644 tests/qapi-schema/data-array-unknown.out
create mode 100644 tests/qapi-schema/data-int.err
create mode 100644 tests/qapi-schema/data-int.exit
create mode 100644 tests/qapi-schema/data-int.json
create mode 100644 tests/qapi-schema/data-int.out
create mode 100644 tests/qapi-schema/data-unknown.err
create mode 100644 tests/qapi-schema/data-unknown.exit
create mode 100644 tests/qapi-schema/data-unknown.json
create mode 100644 tests/qapi-schema/data-unknown.out
create mode 100644 tests/qapi-schema/returns-array-bad.err
create mode 100644 tests/qapi-schema/returns-array-bad.exit
create mode 100644 tests/qapi-schema/returns-array-bad.json
create mode 100644 tests/qapi-schema/returns-array-bad.out
create mode 100644 tests/qapi-schema/returns-int.err
create mode 100644 tests/qapi-schema/returns-int.exit
create mode 100644 tests/qapi-schema/returns-int.json
create mode 100644 tests/qapi-schema/returns-int.out
create mode 100644 tests/qapi-schema/returns-unknown.err
create mode 100644 tests/qapi-schema/returns-unknown.exit
create mode 100644 tests/qapi-schema/returns-unknown.json
create mode 100644 tests/qapi-schema/returns-unknown.out
diff --git a/tests/Makefile b/tests/Makefile
index 1b0c96f..c36faca 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -192,7 +192,9 @@ $(foreach target,$(SYSEMU_TARGET_LIST), \
check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
comments.json empty.json enum-empty.json enum-missing-data.json \
enum-wrong-data.json funny-char.json indented-expr.json \
- missing-type.json double-type.json \
+ missing-type.json double-type.json data-array-empty.json \
+ data-array-unknown.json data-unknown.json data-int.json \
+ returns-unknown.json returns-int.json returns-array-bad.json \
missing-colon.json missing-comma-list.json \
missing-comma-object.json non-objects.json \
qapi-schema-test.json quoted-structural-chars.json \
diff --git a/tests/qapi-schema/data-array-empty.err b/tests/qapi-schema/data-array-empty.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/data-array-empty.exit b/tests/qapi-schema/data-array-empty.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/data-array-empty.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/data-array-empty.json b/tests/qapi-schema/data-array-empty.json
new file mode 100644
index 0000000..00352ac
--- /dev/null
+++ b/tests/qapi-schema/data-array-empty.json
@@ -0,0 +1 @@
+{ 'command': 'oops', 'data': [ ] }
diff --git a/tests/qapi-schema/data-array-empty.out b/tests/qapi-schema/data-array-empty.out
new file mode 100644
index 0000000..67802be
--- /dev/null
+++ b/tests/qapi-schema/data-array-empty.out
@@ -0,0 +1,3 @@
+[OrderedDict([('command', 'oops'), ('data', [])])]
+[]
+[]
diff --git a/tests/qapi-schema/data-array-unknown.err b/tests/qapi-schema/data-array-unknown.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/data-array-unknown.exit b/tests/qapi-schema/data-array-unknown.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/data-array-unknown.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/data-array-unknown.json b/tests/qapi-schema/data-array-unknown.json
new file mode 100644
index 0000000..03ef321
--- /dev/null
+++ b/tests/qapi-schema/data-array-unknown.json
@@ -0,0 +1 @@
+{ 'command': 'oops', 'data': [ 'NoSuchType' ] }
diff --git a/tests/qapi-schema/data-array-unknown.out b/tests/qapi-schema/data-array-unknown.out
new file mode 100644
index 0000000..c3bc05e
--- /dev/null
+++ b/tests/qapi-schema/data-array-unknown.out
@@ -0,0 +1,3 @@
+[OrderedDict([('command', 'oops'), ('data', ['NoSuchType'])])]
+[]
+[]
diff --git a/tests/qapi-schema/data-int.err b/tests/qapi-schema/data-int.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/data-int.exit b/tests/qapi-schema/data-int.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/data-int.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/data-int.json b/tests/qapi-schema/data-int.json
new file mode 100644
index 0000000..926c75e
--- /dev/null
+++ b/tests/qapi-schema/data-int.json
@@ -0,0 +1 @@
+{ 'command': 'oops', 'data': 'int' }
diff --git a/tests/qapi-schema/data-int.out b/tests/qapi-schema/data-int.out
new file mode 100644
index 0000000..e589a4f
--- /dev/null
+++ b/tests/qapi-schema/data-int.out
@@ -0,0 +1,3 @@
+[OrderedDict([('command', 'oops'), ('data', 'int')])]
+[]
+[]
diff --git a/tests/qapi-schema/data-unknown.err b/tests/qapi-schema/data-unknown.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/data-unknown.exit b/tests/qapi-schema/data-unknown.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/data-unknown.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/data-unknown.json b/tests/qapi-schema/data-unknown.json
new file mode 100644
index 0000000..4415584
--- /dev/null
+++ b/tests/qapi-schema/data-unknown.json
@@ -0,0 +1 @@
+{ 'command': 'oops', 'data': 'NoSuchType' }
diff --git a/tests/qapi-schema/data-unknown.out b/tests/qapi-schema/data-unknown.out
new file mode 100644
index 0000000..2c60726
--- /dev/null
+++ b/tests/qapi-schema/data-unknown.out
@@ -0,0 +1,3 @@
+[OrderedDict([('command', 'oops'), ('data', 'NoSuchType')])]
+[]
+[]
diff --git a/tests/qapi-schema/returns-array-bad.err b/tests/qapi-schema/returns-array-bad.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/returns-array-bad.exit b/tests/qapi-schema/returns-array-bad.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/returns-array-bad.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/returns-array-bad.json b/tests/qapi-schema/returns-array-bad.json
new file mode 100644
index 0000000..76bf6df
--- /dev/null
+++ b/tests/qapi-schema/returns-array-bad.json
@@ -0,0 +1 @@
+{ 'command': 'oops', 'data': [ 'str', 'str' ] }
diff --git a/tests/qapi-schema/returns-array-bad.out b/tests/qapi-schema/returns-array-bad.out
new file mode 100644
index 0000000..cfc474e
--- /dev/null
+++ b/tests/qapi-schema/returns-array-bad.out
@@ -0,0 +1,3 @@
+[OrderedDict([('command', 'oops'), ('data', ['str', 'str'])])]
+[]
+[]
diff --git a/tests/qapi-schema/returns-int.err b/tests/qapi-schema/returns-int.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/returns-int.exit b/tests/qapi-schema/returns-int.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/returns-int.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/returns-int.json b/tests/qapi-schema/returns-int.json
new file mode 100644
index 0000000..0884968
--- /dev/null
+++ b/tests/qapi-schema/returns-int.json
@@ -0,0 +1 @@
+{ 'command': 'okay', 'returns': 'int' }
diff --git a/tests/qapi-schema/returns-int.out b/tests/qapi-schema/returns-int.out
new file mode 100644
index 0000000..36b00a9
--- /dev/null
+++ b/tests/qapi-schema/returns-int.out
@@ -0,0 +1,3 @@
+[OrderedDict([('command', 'okay'), ('returns', 'int')])]
+[]
+[]
diff --git a/tests/qapi-schema/returns-unknown.err b/tests/qapi-schema/returns-unknown.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/returns-unknown.exit b/tests/qapi-schema/returns-unknown.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/returns-unknown.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/returns-unknown.json b/tests/qapi-schema/returns-unknown.json
new file mode 100644
index 0000000..508df3e
--- /dev/null
+++ b/tests/qapi-schema/returns-unknown.json
@@ -0,0 +1 @@
+{ 'command': 'oops', 'returns': 'NoSuchType' }
diff --git a/tests/qapi-schema/returns-unknown.out b/tests/qapi-schema/returns-unknown.out
new file mode 100644
index 0000000..a482c83
--- /dev/null
+++ b/tests/qapi-schema/returns-unknown.out
@@ -0,0 +1,3 @@
+[OrderedDict([('command', 'oops'), ('returns', 'NoSuchType')])]
+[]
+[]
--
1.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH v3 08/14] qapi: add expr_name() helper
2014-08-06 1:14 [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs Eric Blake
` (6 preceding siblings ...)
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 07/14] qapi: add some type check tests Eric Blake
@ 2014-08-06 1:14 ` Eric Blake
2014-08-14 9:49 ` Markus Armbruster
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 09/14] qapi: add check_type helper function Eric Blake
` (8 subsequent siblings)
16 siblings, 1 reply; 29+ messages in thread
From: Eric Blake @ 2014-08-06 1:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng, Markus Armbruster, wenchaoqemu, Luiz Capitulino
Now that we know every expression has a known meta-type, we
can add a helper function that retrieves the name of an
arbitrary expression, for use in future error messages.
* scripts/qapi.py (expr_name): New function.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi.py | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 910e422..e02fa0b 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -248,6 +248,19 @@ def discriminator_find_enum_define(expr):
return find_enum(discriminator_type)
+def expr_name(expr):
+ if expr.has_key('union'):
+ return expr['union']
+ if expr.has_key('type'):
+ return expr['type']
+ if expr.has_key('enum'):
+ return expr['enum']
+ if expr.has_key('command'):
+ return expr['command']
+ if expr.has_key('event'):
+ return expr['event']
+ return None
+
def check_event(expr, expr_info):
params = expr.get('data')
if params:
--
1.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH v3 09/14] qapi: add check_type helper function
2014-08-06 1:14 [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs Eric Blake
` (7 preceding siblings ...)
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 08/14] qapi: add expr_name() helper Eric Blake
@ 2014-08-06 1:14 ` Eric Blake
2014-08-14 10:10 ` Markus Armbruster
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 10/14] qapi: merge UserDefTwo and UserDefNested in tests Eric Blake
` (7 subsequent siblings)
16 siblings, 1 reply; 29+ messages in thread
From: Eric Blake @ 2014-08-06 1:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng, Markus Armbruster, wenchaoqemu, Luiz Capitulino
Add a helper function for use in a later patch that will
validate that a 'data':... or 'returns':... member of an
expression evaluates to a valid type.
* scripts/qapi.py (check_type): New function.
(check_exprs): Use it.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi.py | 30 ++++++++++++++++++++++++++++++
tests/qapi-schema/data-array-unknown.err | 1 +
tests/qapi-schema/data-array-unknown.exit | 2 +-
tests/qapi-schema/data-array-unknown.out | 3 ---
tests/qapi-schema/data-int.err | 1 +
tests/qapi-schema/data-int.exit | 2 +-
tests/qapi-schema/data-int.out | 3 ---
tests/qapi-schema/data-unknown.err | 1 +
tests/qapi-schema/data-unknown.exit | 2 +-
tests/qapi-schema/data-unknown.out | 3 ---
tests/qapi-schema/returns-array-bad.err | 1 +
tests/qapi-schema/returns-array-bad.exit | 2 +-
tests/qapi-schema/returns-array-bad.out | 3 ---
tests/qapi-schema/returns-unknown.err | 1 +
tests/qapi-schema/returns-unknown.exit | 2 +-
tests/qapi-schema/returns-unknown.out | 3 ---
16 files changed, 40 insertions(+), 20 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index e02fa0b..e4d27d6 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -329,6 +329,32 @@ def check_union(expr, expr_info):
# Todo: add checking for values. Key is checked as above, value can be
# also checked here, but we need more functions to handle array case.
+def check_type(expr_info, name, data, allow_native = False):
+ if not data:
+ return
+ if isinstance(data, list):
+ if len(data) != 1 or not isinstance(data[0], basestring):
+ raise QAPIExprError(expr_info,
+ "Use of array type in '%s' must contain "
+ "single element type name" % name)
+ data = data[0]
+
+ if isinstance(data, basestring):
+ if find_struct(data) or find_enum(data) or find_union(data):
+ return
+ if data == 'str' or data == 'int' or data == 'visitor':
+ if allow_native:
+ return
+ raise QAPIExprError(expr_info,
+ "Primitive type '%s' not allowed as data "
+ "of '%s'" % (data, name))
+ raise QAPIExprError(expr_info,
+ "Unknown type '%s' as data of '%s'"
+ % (data, name))
+ elif not isinstance(data, OrderedDict):
+ raise QAPIExprError(expr_info,
+ "Expecting dictionary for data of '%s'" % name)
+
def check_exprs(schema):
for expr_elem in schema.exprs:
expr = expr_elem['expr']
@@ -356,6 +382,10 @@ def check_exprs(schema):
raise QAPIExprError(info,
"enum '%s' requires an array for 'data'"
% expr.get('enum'))
+ else:
+ check_type(info, expr_name(expr), members)
+ if expr.has_key('command') and expr.has_key('returns'):
+ check_type(info, expr['command'], expr['returns'], True)
def parse_schema(input_file):
try:
diff --git a/tests/qapi-schema/data-array-unknown.err b/tests/qapi-schema/data-array-unknown.err
index e69de29..e0433b8 100644
--- a/tests/qapi-schema/data-array-unknown.err
+++ b/tests/qapi-schema/data-array-unknown.err
@@ -0,0 +1 @@
+tests/qapi-schema/data-array-unknown.json:1: Unknown type 'NoSuchType' as data of 'oops'
diff --git a/tests/qapi-schema/data-array-unknown.exit b/tests/qapi-schema/data-array-unknown.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/data-array-unknown.exit
+++ b/tests/qapi-schema/data-array-unknown.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/data-array-unknown.out b/tests/qapi-schema/data-array-unknown.out
index c3bc05e..e69de29 100644
--- a/tests/qapi-schema/data-array-unknown.out
+++ b/tests/qapi-schema/data-array-unknown.out
@@ -1,3 +0,0 @@
-[OrderedDict([('command', 'oops'), ('data', ['NoSuchType'])])]
-[]
-[]
diff --git a/tests/qapi-schema/data-int.err b/tests/qapi-schema/data-int.err
index e69de29..30bbffc 100644
--- a/tests/qapi-schema/data-int.err
+++ b/tests/qapi-schema/data-int.err
@@ -0,0 +1 @@
+tests/qapi-schema/data-int.json:1: Primitive type 'int' not allowed as data of 'oops'
diff --git a/tests/qapi-schema/data-int.exit b/tests/qapi-schema/data-int.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/data-int.exit
+++ b/tests/qapi-schema/data-int.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/data-int.out b/tests/qapi-schema/data-int.out
index e589a4f..e69de29 100644
--- a/tests/qapi-schema/data-int.out
+++ b/tests/qapi-schema/data-int.out
@@ -1,3 +0,0 @@
-[OrderedDict([('command', 'oops'), ('data', 'int')])]
-[]
-[]
diff --git a/tests/qapi-schema/data-unknown.err b/tests/qapi-schema/data-unknown.err
index e69de29..f7fd635 100644
--- a/tests/qapi-schema/data-unknown.err
+++ b/tests/qapi-schema/data-unknown.err
@@ -0,0 +1 @@
+tests/qapi-schema/data-unknown.json:1: Unknown type 'NoSuchType' as data of 'oops'
diff --git a/tests/qapi-schema/data-unknown.exit b/tests/qapi-schema/data-unknown.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/data-unknown.exit
+++ b/tests/qapi-schema/data-unknown.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/data-unknown.out b/tests/qapi-schema/data-unknown.out
index 2c60726..e69de29 100644
--- a/tests/qapi-schema/data-unknown.out
+++ b/tests/qapi-schema/data-unknown.out
@@ -1,3 +0,0 @@
-[OrderedDict([('command', 'oops'), ('data', 'NoSuchType')])]
-[]
-[]
diff --git a/tests/qapi-schema/returns-array-bad.err b/tests/qapi-schema/returns-array-bad.err
index e69de29..d2a216f 100644
--- a/tests/qapi-schema/returns-array-bad.err
+++ b/tests/qapi-schema/returns-array-bad.err
@@ -0,0 +1 @@
+tests/qapi-schema/returns-array-bad.json:1: Use of array type in 'oops' must contain single element type name
diff --git a/tests/qapi-schema/returns-array-bad.exit b/tests/qapi-schema/returns-array-bad.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/returns-array-bad.exit
+++ b/tests/qapi-schema/returns-array-bad.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/returns-array-bad.out b/tests/qapi-schema/returns-array-bad.out
index cfc474e..e69de29 100644
--- a/tests/qapi-schema/returns-array-bad.out
+++ b/tests/qapi-schema/returns-array-bad.out
@@ -1,3 +0,0 @@
-[OrderedDict([('command', 'oops'), ('data', ['str', 'str'])])]
-[]
-[]
diff --git a/tests/qapi-schema/returns-unknown.err b/tests/qapi-schema/returns-unknown.err
index e69de29..d09fb92 100644
--- a/tests/qapi-schema/returns-unknown.err
+++ b/tests/qapi-schema/returns-unknown.err
@@ -0,0 +1 @@
+tests/qapi-schema/returns-unknown.json:1: Unknown type 'NoSuchType' as data of 'oops'
diff --git a/tests/qapi-schema/returns-unknown.exit b/tests/qapi-schema/returns-unknown.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/returns-unknown.exit
+++ b/tests/qapi-schema/returns-unknown.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/returns-unknown.out b/tests/qapi-schema/returns-unknown.out
index a482c83..e69de29 100644
--- a/tests/qapi-schema/returns-unknown.out
+++ b/tests/qapi-schema/returns-unknown.out
@@ -1,3 +0,0 @@
-[OrderedDict([('command', 'oops'), ('returns', 'NoSuchType')])]
-[]
-[]
--
1.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH v3 10/14] qapi: merge UserDefTwo and UserDefNested in tests
2014-08-06 1:14 [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs Eric Blake
` (8 preceding siblings ...)
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 09/14] qapi: add check_type helper function Eric Blake
@ 2014-08-06 1:14 ` Eric Blake
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 11/14] qapi: drop tests for inline subtypes Eric Blake
` (6 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Eric Blake @ 2014-08-06 1:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng, Markus Armbruster, wenchaoqemu, Luiz Capitulino
In the testsuite, UserDefTwo and UserDefNested were identical
types other than the member names. Reduce code duplication by
having just one type, and choose names that also favor reuse.
This will also make it easier for a later patch to get rid of
nested inline subtypes in QAPI.
Ensure that 'make check-qapi-schema check-unit' still passes.
* tests/qapi-schema/qapi-schema-test.json (UserDefNested): Drop.
(UserDefTwo): Rename element members for clarity.
* tests/qapi-schema/qapi-schema-test.out: Update expected output.
* tests/test-qmp-output-visitor.c
(test_visitor_out_struct_nested, test_visitor_out_list_qapi_free):
Update clients.
* tests/test-qmp-input-visitor.c (test_visitor_in_struct_nested):
Likewise.
* tests/test-qmp-input-strict.c (test_validate_struct_nested)
(test_validate_fail_struct_nested): Likewise.
* tests/test-qmp-commands.c (qmp_user_def_cmd2)
(test_dealloc_partial, test_dispatch_cmd_io): Likewise.
* tests/test-visitor-serialization.c (nested_struct_create)
(nested_struct_compare, nested_struct_cleanup)
(visit_nested_struct, visit_nested_struct_list)
(test_nested_struct, test_nested_struct_list): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tests/qapi-schema/qapi-schema-test.json | 10 +----
tests/qapi-schema/qapi-schema-test.out | 6 +--
tests/test-qmp-commands.c | 32 +++++++--------
tests/test-qmp-input-strict.c | 19 +++++----
tests/test-qmp-input-visitor.c | 19 +++++----
tests/test-qmp-output-visitor.c | 50 +++++++++++------------
tests/test-visitor-serialization.c | 71 +++++++++++++++++----------------
7 files changed, 103 insertions(+), 104 deletions(-)
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index ab4d3d9..2e13f09 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -15,16 +15,10 @@
'data': { 'string': 'str', '*enum1': 'EnumOne' } }
{ 'type': 'UserDefTwo',
- 'data': { 'string': 'str',
- 'dict': { 'string': 'str',
- 'dict': { 'userdef': 'UserDefOne', 'string': 'str' },
- '*dict2': { 'userdef': 'UserDefOne', 'string': 'str' } } } }
-
-{ 'type': 'UserDefNested',
'data': { 'string0': 'str',
'dict1': { 'string1': 'str',
- 'dict2': { 'userdef1': 'UserDefOne', 'string2': 'str' },
- '*dict3': { 'userdef2': 'UserDefOne', 'string3': 'str' } } } }
+ 'dict2': { 'userdef': 'UserDefOne', 'string': 'str' },
+ '*dict3': { 'userdef': 'UserDefOne', 'string': 'str' } } } }
# for testing unions
{ 'type': 'UserDefA',
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 95e9899..f94bbcc 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -2,8 +2,7 @@
OrderedDict([('type', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 'EnumOne')]))]),
OrderedDict([('type', 'UserDefZero'), ('data', OrderedDict([('integer', 'int')]))]),
OrderedDict([('type', 'UserDefOne'), ('base', 'UserDefZero'), ('data', OrderedDict([('string', 'str'), ('*enum1', 'EnumOne')]))]),
- OrderedDict([('type', 'UserDefTwo'), ('data', OrderedDict([('string', 'str'), ('dict', OrderedDict([('string', 'str'), ('dict', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')])), ('*dict2', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')]))]))]))]),
- OrderedDict([('type', 'UserDefNested'), ('data', OrderedDict([('string0', 'str'), ('dict1', OrderedDict([('string1', 'str'), ('dict2', OrderedDict([('userdef1', 'UserDefOne'), ('string2', 'str')])), ('*dict3', OrderedDict([('userdef2', 'UserDefOne'), ('string3', 'str')]))]))]))]),
+ OrderedDict([('type', 'UserDefTwo'), ('data', OrderedDict([('string0', 'str'), ('dict1', OrderedDict([('string1', 'str'), ('dict2', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')])), ('*dict3', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')]))]))]))]),
OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]),
OrderedDict([('union', 'UserDefUnion'), ('base', 'UserDefZero'), ('data', OrderedDict([('a', 'UserDefA'), ('b', 'UserDefB')]))]),
@@ -28,8 +27,7 @@
[OrderedDict([('type', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 'EnumOne')]))]),
OrderedDict([('type', 'UserDefZero'), ('data', OrderedDict([('integer', 'int')]))]),
OrderedDict([('type', 'UserDefOne'), ('base', 'UserDefZero'), ('data', OrderedDict([('string', 'str'), ('*enum1', 'EnumOne')]))]),
- OrderedDict([('type', 'UserDefTwo'), ('data', OrderedDict([('string', 'str'), ('dict', OrderedDict([('string', 'str'), ('dict', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')])), ('*dict2', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')]))]))]))]),
- OrderedDict([('type', 'UserDefNested'), ('data', OrderedDict([('string0', 'str'), ('dict1', OrderedDict([('string1', 'str'), ('dict2', OrderedDict([('userdef1', 'UserDefOne'), ('string2', 'str')])), ('*dict3', OrderedDict([('userdef2', 'UserDefOne'), ('string3', 'str')]))]))]))]),
+ OrderedDict([('type', 'UserDefTwo'), ('data', OrderedDict([('string0', 'str'), ('dict1', OrderedDict([('string1', 'str'), ('dict2', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')])), ('*dict3', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')]))]))]))]),
OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]),
OrderedDict([('type', 'UserDefUnionBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]),
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index 554e222..9189cd2 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -32,13 +32,13 @@ UserDefTwo *qmp_user_def_cmd2(UserDefOne *ud1a,
ud1d->base->integer = has_udb1 ? ud1b->base->integer : 0;
ret = g_malloc0(sizeof(UserDefTwo));
- ret->string = strdup("blah1");
- ret->dict.string = strdup("blah2");
- ret->dict.dict.userdef = ud1c;
- ret->dict.dict.string = strdup("blah3");
- ret->dict.has_dict2 = true;
- ret->dict.dict2.userdef = ud1d;
- ret->dict.dict2.string = strdup("blah4");
+ ret->string0 = strdup("blah1");
+ ret->dict1.string1 = strdup("blah2");
+ ret->dict1.dict2.userdef = ud1c;
+ ret->dict1.dict2.string = strdup("blah3");
+ ret->dict1.has_dict3 = true;
+ ret->dict1.dict3.userdef = ud1d;
+ ret->dict1.dict3.string = strdup("blah4");
return ret;
}
@@ -120,15 +120,15 @@ static void test_dispatch_cmd_io(void)
ret = qobject_to_qdict(test_qmp_dispatch(req));
- assert(!strcmp(qdict_get_str(ret, "string"), "blah1"));
- ret_dict = qdict_get_qdict(ret, "dict");
- assert(!strcmp(qdict_get_str(ret_dict, "string"), "blah2"));
- ret_dict_dict = qdict_get_qdict(ret_dict, "dict");
+ assert(!strcmp(qdict_get_str(ret, "string0"), "blah1"));
+ ret_dict = qdict_get_qdict(ret, "dict1");
+ assert(!strcmp(qdict_get_str(ret_dict, "string1"), "blah2"));
+ ret_dict_dict = qdict_get_qdict(ret_dict, "dict2");
ret_dict_dict_userdef = qdict_get_qdict(ret_dict_dict, "userdef");
assert(qdict_get_int(ret_dict_dict_userdef, "integer") == 42);
assert(!strcmp(qdict_get_str(ret_dict_dict_userdef, "string"), "hello"));
assert(!strcmp(qdict_get_str(ret_dict_dict, "string"), "blah3"));
- ret_dict_dict2 = qdict_get_qdict(ret_dict, "dict2");
+ ret_dict_dict2 = qdict_get_qdict(ret_dict, "dict3");
ret_dict_dict2_userdef = qdict_get_qdict(ret_dict_dict2, "userdef");
assert(qdict_get_int(ret_dict_dict2_userdef, "integer") == 422);
assert(!strcmp(qdict_get_str(ret_dict_dict2_userdef, "string"), "hello2"));
@@ -192,7 +192,7 @@ static void test_dealloc_partial(void)
QmpInputVisitor *qiv;
ud2_dict = qdict_new();
- qdict_put_obj(ud2_dict, "string", QOBJECT(qstring_from_str(text)));
+ qdict_put_obj(ud2_dict, "string0", QOBJECT(qstring_from_str(text)));
qiv = qmp_input_visitor_new(QOBJECT(ud2_dict));
visit_type_UserDefTwo(qmp_input_get_visitor(qiv), &ud2, NULL, &err);
@@ -202,9 +202,9 @@ static void test_dealloc_partial(void)
/* verify partial success */
assert(ud2 != NULL);
- assert(ud2->string != NULL);
- assert(strcmp(ud2->string, text) == 0);
- assert(ud2->dict.dict.userdef == NULL);
+ assert(ud2->string0 != NULL);
+ assert(strcmp(ud2->string0, text) == 0);
+ assert(ud2->dict1.dict2.userdef == NULL);
/* confirm & release construction error */
assert(err != NULL);
diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
index 0f77003..29d4f82 100644
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qmp-input-strict.c
@@ -1,7 +1,7 @@
/*
* QMP Input Visitor unit-tests (strict mode).
*
- * Copyright (C) 2011-2012 Red Hat Inc.
+ * Copyright (C) 2011-2012, 2014 Red Hat Inc.
*
* Authors:
* Luiz Capitulino <lcapitulino@redhat.com>
@@ -116,15 +116,18 @@ static void test_validate_struct(TestInputVisitorData *data,
static void test_validate_struct_nested(TestInputVisitorData *data,
const void *unused)
{
- UserDefNested *udp = NULL;
+ UserDefTwo *udp = NULL;
Error *err = NULL;
Visitor *v;
- v = validate_test_init(data, "{ 'string0': 'string0', 'dict1': { 'string1': 'string1', 'dict2': { 'userdef1': { 'integer': 42, 'string': 'string' }, 'string2': 'string2'}}}");
+ v = validate_test_init(data, "{ 'string0': 'string0', "
+ "'dict1': { 'string1': 'string1', "
+ "'dict2': { 'userdef': { 'integer': 42, "
+ "'string': 'string' }, 'string': 'string2'}}}");
- visit_type_UserDefNested(v, &udp, NULL, &err);
+ visit_type_UserDefTwo(v, &udp, NULL, &err);
g_assert(!err);
- qapi_free_UserDefNested(udp);
+ qapi_free_UserDefTwo(udp);
}
static void test_validate_list(TestInputVisitorData *data,
@@ -207,15 +210,15 @@ static void test_validate_fail_struct(TestInputVisitorData *data,
static void test_validate_fail_struct_nested(TestInputVisitorData *data,
const void *unused)
{
- UserDefNested *udp = NULL;
+ 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_UserDefNested(v, &udp, NULL, &err);
+ visit_type_UserDefTwo(v, &udp, NULL, &err);
g_assert(err);
- qapi_free_UserDefNested(udp);
+ qapi_free_UserDefTwo(udp);
}
static void test_validate_fail_list(TestInputVisitorData *data,
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 1c8e872..3e1da9d 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -1,7 +1,7 @@
/*
* QMP Input Visitor unit-tests.
*
- * Copyright (C) 2011 Red Hat Inc.
+ * Copyright (C) 2011, 2014 Red Hat Inc.
*
* Authors:
* Luiz Capitulino <lcapitulino@redhat.com>
@@ -248,23 +248,26 @@ static void check_and_free_str(char *str, const char *cmp)
static void test_visitor_in_struct_nested(TestInputVisitorData *data,
const void *unused)
{
- UserDefNested *udp = NULL;
+ 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' }, 'string2': 'string2'}}}");
+ v = visitor_input_test_init(data, "{ 'string0': 'string0', "
+ "'dict1': { 'string1': 'string1', "
+ "'dict2': { 'userdef': { 'integer': 42, "
+ "'string': 'string' }, 'string': 'string2'}}}");
- visit_type_UserDefNested(v, &udp, NULL, &err);
+ visit_type_UserDefTwo(v, &udp, NULL, &err);
g_assert(!err);
check_and_free_str(udp->string0, "string0");
check_and_free_str(udp->dict1.string1, "string1");
- g_assert_cmpint(udp->dict1.dict2.userdef1->base->integer, ==, 42);
- check_and_free_str(udp->dict1.dict2.userdef1->string, "string");
- check_and_free_str(udp->dict1.dict2.string2, "string2");
+ g_assert_cmpint(udp->dict1.dict2.userdef->base->integer, ==, 42);
+ check_and_free_str(udp->dict1.dict2.userdef->string, "string");
+ check_and_free_str(udp->dict1.dict2.string, "string2");
g_assert(udp->dict1.has_dict3 == false);
- g_free(udp->dict1.dict2.userdef1);
+ g_free(udp->dict1.dict2.userdef);
g_free(udp);
}
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index 74020de..6781a4f 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -1,7 +1,7 @@
/*
* QMP Output Visitor unit-tests.
*
- * Copyright (C) 2011 Red Hat Inc.
+ * Copyright (C) 2011, 2014 Red Hat Inc.
*
* Authors:
* Luiz Capitulino <lcapitulino@redhat.com>
@@ -234,7 +234,7 @@ static void test_visitor_out_struct_nested(TestOutputVisitorData *data,
{
int64_t value = 42;
Error *err = NULL;
- UserDefNested *ud2;
+ UserDefTwo *ud2;
QObject *obj;
QDict *qdict, *dict1, *dict2, *dict3, *userdef;
const char *string = "user def string";
@@ -245,20 +245,20 @@ static void test_visitor_out_struct_nested(TestOutputVisitorData *data,
ud2->string0 = g_strdup(strings[0]);
ud2->dict1.string1 = g_strdup(strings[1]);
- ud2->dict1.dict2.userdef1 = g_malloc0(sizeof(UserDefOne));
- ud2->dict1.dict2.userdef1->string = g_strdup(string);
- ud2->dict1.dict2.userdef1->base = g_new0(UserDefZero, 1);
- ud2->dict1.dict2.userdef1->base->integer = value;
- ud2->dict1.dict2.string2 = g_strdup(strings[2]);
+ ud2->dict1.dict2.userdef = g_malloc0(sizeof(UserDefOne));
+ ud2->dict1.dict2.userdef->string = g_strdup(string);
+ ud2->dict1.dict2.userdef->base = g_new0(UserDefZero, 1);
+ ud2->dict1.dict2.userdef->base->integer = value;
+ ud2->dict1.dict2.string = g_strdup(strings[2]);
ud2->dict1.has_dict3 = true;
- ud2->dict1.dict3.userdef2 = g_malloc0(sizeof(UserDefOne));
- ud2->dict1.dict3.userdef2->string = g_strdup(string);
- ud2->dict1.dict3.userdef2->base = g_new0(UserDefZero, 1);
- ud2->dict1.dict3.userdef2->base->integer = value;
- ud2->dict1.dict3.string3 = g_strdup(strings[3]);
+ ud2->dict1.dict3.userdef = g_malloc0(sizeof(UserDefOne));
+ ud2->dict1.dict3.userdef->string = g_strdup(string);
+ ud2->dict1.dict3.userdef->base = g_new0(UserDefZero, 1);
+ ud2->dict1.dict3.userdef->base->integer = value;
+ ud2->dict1.dict3.string = g_strdup(strings[3]);
- visit_type_UserDefNested(data->ov, &ud2, "unused", &err);
+ visit_type_UserDefTwo(data->ov, &ud2, "unused", &err);
g_assert(!err);
obj = qmp_output_get_qobject(data->qov);
@@ -275,22 +275,22 @@ static void test_visitor_out_struct_nested(TestOutputVisitorData *data,
dict2 = qdict_get_qdict(dict1, "dict2");
g_assert_cmpint(qdict_size(dict2), ==, 2);
- g_assert_cmpstr(qdict_get_str(dict2, "string2"), ==, strings[2]);
- userdef = qdict_get_qdict(dict2, "userdef1");
+ g_assert_cmpstr(qdict_get_str(dict2, "string"), ==, strings[2]);
+ userdef = qdict_get_qdict(dict2, "userdef");
g_assert_cmpint(qdict_size(userdef), ==, 2);
g_assert_cmpint(qdict_get_int(userdef, "integer"), ==, value);
g_assert_cmpstr(qdict_get_str(userdef, "string"), ==, string);
dict3 = qdict_get_qdict(dict1, "dict3");
g_assert_cmpint(qdict_size(dict3), ==, 2);
- g_assert_cmpstr(qdict_get_str(dict3, "string3"), ==, strings[3]);
- userdef = qdict_get_qdict(dict3, "userdef2");
+ g_assert_cmpstr(qdict_get_str(dict3, "string"), ==, strings[3]);
+ userdef = qdict_get_qdict(dict3, "userdef");
g_assert_cmpint(qdict_size(userdef), ==, 2);
g_assert_cmpint(qdict_get_int(userdef, "integer"), ==, value);
g_assert_cmpstr(qdict_get_str(userdef, "string"), ==, string);
QDECREF(qdict);
- qapi_free_UserDefNested(ud2);
+ qapi_free_UserDefTwo(ud2);
}
static void test_visitor_out_struct_errors(TestOutputVisitorData *data,
@@ -398,7 +398,7 @@ static void test_visitor_out_list(TestOutputVisitorData *data,
static void test_visitor_out_list_qapi_free(TestOutputVisitorData *data,
const void *unused)
{
- UserDefNestedList *p, *head = NULL;
+ UserDefTwoList *p, *head = NULL;
const char string[] = "foo bar";
int i, max_count = 1024;
@@ -408,18 +408,18 @@ static void test_visitor_out_list_qapi_free(TestOutputVisitorData *data,
p->value->string0 = g_strdup(string);
p->value->dict1.string1 = g_strdup(string);
- p->value->dict1.dict2.userdef1 = g_malloc0(sizeof(UserDefOne));
- p->value->dict1.dict2.userdef1->string = g_strdup(string);
- p->value->dict1.dict2.userdef1->base = g_new0(UserDefZero, 1);
- p->value->dict1.dict2.userdef1->base->integer = 42;
- p->value->dict1.dict2.string2 = g_strdup(string);
+ p->value->dict1.dict2.userdef = g_malloc0(sizeof(UserDefOne));
+ p->value->dict1.dict2.userdef->string = g_strdup(string);
+ p->value->dict1.dict2.userdef->base = g_new0(UserDefZero, 1);
+ p->value->dict1.dict2.userdef->base->integer = 42;
+ p->value->dict1.dict2.string = g_strdup(string);
p->value->dict1.has_dict3 = false;
p->next = head;
head = p;
}
- qapi_free_UserDefNestedList(head);
+ qapi_free_UserDefTwoList(head);
}
static void test_visitor_out_union(TestOutputVisitorData *data,
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index 7ad1886..133f882 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -249,57 +249,57 @@ static void visit_struct(Visitor *v, void **native, Error **errp)
visit_type_TestStruct(v, (TestStruct **)native, NULL, errp);
}
-static UserDefNested *nested_struct_create(void)
+static UserDefTwo *nested_struct_create(void)
{
- UserDefNested *udnp = g_malloc0(sizeof(*udnp));
+ UserDefTwo *udnp = g_malloc0(sizeof(*udnp));
udnp->string0 = strdup("test_string0");
udnp->dict1.string1 = strdup("test_string1");
- udnp->dict1.dict2.userdef1 = g_malloc0(sizeof(UserDefOne));
- udnp->dict1.dict2.userdef1->base = g_new0(UserDefZero, 1);
- udnp->dict1.dict2.userdef1->base->integer = 42;
- udnp->dict1.dict2.userdef1->string = strdup("test_string");
- udnp->dict1.dict2.string2 = strdup("test_string2");
+ udnp->dict1.dict2.userdef = g_malloc0(sizeof(UserDefOne));
+ udnp->dict1.dict2.userdef->base = g_new0(UserDefZero, 1);
+ udnp->dict1.dict2.userdef->base->integer = 42;
+ udnp->dict1.dict2.userdef->string = strdup("test_string");
+ udnp->dict1.dict2.string = strdup("test_string2");
udnp->dict1.has_dict3 = true;
- udnp->dict1.dict3.userdef2 = g_malloc0(sizeof(UserDefOne));
- udnp->dict1.dict3.userdef2->base = g_new0(UserDefZero, 1);
- udnp->dict1.dict3.userdef2->base->integer = 43;
- udnp->dict1.dict3.userdef2->string = strdup("test_string");
- udnp->dict1.dict3.string3 = strdup("test_string3");
+ udnp->dict1.dict3.userdef = g_malloc0(sizeof(UserDefOne));
+ udnp->dict1.dict3.userdef->base = g_new0(UserDefZero, 1);
+ udnp->dict1.dict3.userdef->base->integer = 43;
+ udnp->dict1.dict3.userdef->string = strdup("test_string");
+ udnp->dict1.dict3.string = strdup("test_string3");
return udnp;
}
-static void nested_struct_compare(UserDefNested *udnp1, UserDefNested *udnp2)
+static void nested_struct_compare(UserDefTwo *udnp1, UserDefTwo *udnp2)
{
g_assert(udnp1);
g_assert(udnp2);
g_assert_cmpstr(udnp1->string0, ==, udnp2->string0);
g_assert_cmpstr(udnp1->dict1.string1, ==, udnp2->dict1.string1);
- g_assert_cmpint(udnp1->dict1.dict2.userdef1->base->integer, ==,
- udnp2->dict1.dict2.userdef1->base->integer);
- g_assert_cmpstr(udnp1->dict1.dict2.userdef1->string, ==,
- udnp2->dict1.dict2.userdef1->string);
- g_assert_cmpstr(udnp1->dict1.dict2.string2, ==, udnp2->dict1.dict2.string2);
+ g_assert_cmpint(udnp1->dict1.dict2.userdef->base->integer, ==,
+ udnp2->dict1.dict2.userdef->base->integer);
+ g_assert_cmpstr(udnp1->dict1.dict2.userdef->string, ==,
+ udnp2->dict1.dict2.userdef->string);
+ g_assert_cmpstr(udnp1->dict1.dict2.string, ==, udnp2->dict1.dict2.string);
g_assert(udnp1->dict1.has_dict3 == udnp2->dict1.has_dict3);
- g_assert_cmpint(udnp1->dict1.dict3.userdef2->base->integer, ==,
- udnp2->dict1.dict3.userdef2->base->integer);
- g_assert_cmpstr(udnp1->dict1.dict3.userdef2->string, ==,
- udnp2->dict1.dict3.userdef2->string);
- g_assert_cmpstr(udnp1->dict1.dict3.string3, ==, udnp2->dict1.dict3.string3);
+ g_assert_cmpint(udnp1->dict1.dict3.userdef->base->integer, ==,
+ udnp2->dict1.dict3.userdef->base->integer);
+ g_assert_cmpstr(udnp1->dict1.dict3.userdef->string, ==,
+ udnp2->dict1.dict3.userdef->string);
+ g_assert_cmpstr(udnp1->dict1.dict3.string, ==, udnp2->dict1.dict3.string);
}
-static void nested_struct_cleanup(UserDefNested *udnp)
+static void nested_struct_cleanup(UserDefTwo *udnp)
{
- qapi_free_UserDefNested(udnp);
+ qapi_free_UserDefTwo(udnp);
}
static void visit_nested_struct(Visitor *v, void **native, Error **errp)
{
- visit_type_UserDefNested(v, (UserDefNested **)native, NULL, errp);
+ visit_type_UserDefTwo(v, (UserDefTwo **)native, NULL, errp);
}
static void visit_nested_struct_list(Visitor *v, void **native, Error **errp)
{
- visit_type_UserDefNestedList(v, (UserDefNestedList **)native, NULL, errp);
+ visit_type_UserDefTwoList(v, (UserDefTwoList **)native, NULL, errp);
}
/* test cases */
@@ -715,13 +715,14 @@ static void test_nested_struct(gconstpointer opaque)
{
TestArgs *args = (TestArgs *) opaque;
const SerializeOps *ops = args->ops;
- UserDefNested *udnp = nested_struct_create();
- UserDefNested *udnp_copy = NULL;
+ UserDefTwo *udnp = nested_struct_create();
+ UserDefTwo *udnp_copy = NULL;
Error *err = NULL;
void *serialize_data;
-
+
ops->serialize(udnp, &serialize_data, visit_nested_struct, &err);
- ops->deserialize((void **)&udnp_copy, serialize_data, visit_nested_struct, &err);
+ ops->deserialize((void **)&udnp_copy, serialize_data, visit_nested_struct,
+ &err);
g_assert(err == NULL);
nested_struct_compare(udnp, udnp_copy);
@@ -737,13 +738,13 @@ static void test_nested_struct_list(gconstpointer opaque)
{
TestArgs *args = (TestArgs *) opaque;
const SerializeOps *ops = args->ops;
- UserDefNestedList *listp = NULL, *tmp, *tmp_copy, *listp_copy = NULL;
+ UserDefTwoList *listp = NULL, *tmp, *tmp_copy, *listp_copy = NULL;
Error *err = NULL;
void *serialize_data;
int i = 0;
for (i = 0; i < 8; i++) {
- tmp = g_malloc0(sizeof(UserDefNestedList));
+ tmp = g_malloc0(sizeof(UserDefTwoList));
tmp->value = nested_struct_create();
tmp->next = listp;
listp = tmp;
@@ -764,8 +765,8 @@ static void test_nested_struct_list(gconstpointer opaque)
listp_copy = listp_copy->next;
}
- qapi_free_UserDefNestedList(tmp);
- qapi_free_UserDefNestedList(tmp_copy);
+ qapi_free_UserDefTwoList(tmp);
+ qapi_free_UserDefTwoList(tmp_copy);
ops->cleanup(serialize_data);
g_free(args);
--
1.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH v3 11/14] qapi: drop tests for inline subtypes
2014-08-06 1:14 [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs Eric Blake
` (9 preceding siblings ...)
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 10/14] qapi: merge UserDefTwo and UserDefNested in tests Eric Blake
@ 2014-08-06 1:14 ` Eric Blake
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 12/14] qapi: drop inline subtype in query-version Eric Blake
` (5 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Eric Blake @ 2014-08-06 1:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng, Markus Armbruster, wenchaoqemu, Luiz Capitulino
A future patch will be using a 'name':{dictionary} entry in the
QAPI schema to specify a default value for an optional argument;
but existing use of inline substructs conflicts with that goal.
This patch fixes the testsuite to avoid nested subtypes, by
breaking the nesting into explicit types.
* tests/qapi-schema/qapi-schema-test.json: Flatten UserDefTwo.
* tests/qapi-schema/qapi-schema-test.out: Update expected output.
* tests/test-qmp-output-visitor.c (test_visitor_out_struct_nested)
(test_visitor_out_list_qapi_free): Update clients.
* tests/test-qmp-input-visitor.c (test_visitor_in_struct_nested):
Likewise.
* tests/test-qmp-commands.c (qmp_user_def_cmd2)
(test_dealloc_partial): Likewise.
* tests/test-visitor-serialization.c (nested_struct_create)
(nested_struct_compare): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tests/qapi-schema/qapi-schema-test.json | 12 ++++++--
tests/qapi-schema/qapi-schema-test.out | 8 +++--
tests/test-qmp-commands.c | 17 ++++++-----
tests/test-qmp-input-visitor.c | 14 +++++----
tests/test-qmp-output-visitor.c | 44 +++++++++++++++------------
tests/test-visitor-serialization.c | 53 ++++++++++++++++++---------------
6 files changed, 87 insertions(+), 61 deletions(-)
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 2e13f09..1f922c1 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -14,11 +14,17 @@
'base': 'UserDefZero',
'data': { 'string': 'str', '*enum1': 'EnumOne' } }
+{ 'type': 'UserDefTwoDictDict',
+ 'data': { 'userdef': 'UserDefOne', 'string': 'str' } }
+
+{ 'type': 'UserDefTwoDict',
+ 'data': { 'string1': 'str',
+ 'dict2': 'UserDefTwoDictDict',
+ '*dict3': 'UserDefTwoDictDict' } }
+
{ 'type': 'UserDefTwo',
'data': { 'string0': 'str',
- 'dict1': { 'string1': 'str',
- 'dict2': { 'userdef': 'UserDefOne', 'string': 'str' },
- '*dict3': { 'userdef': 'UserDefOne', 'string': 'str' } } } }
+ 'dict1': 'UserDefTwoDict' } }
# for testing unions
{ 'type': 'UserDefA',
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index f94bbcc..1d7b53c 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -2,7 +2,9 @@
OrderedDict([('type', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 'EnumOne')]))]),
OrderedDict([('type', 'UserDefZero'), ('data', OrderedDict([('integer', 'int')]))]),
OrderedDict([('type', 'UserDefOne'), ('base', 'UserDefZero'), ('data', OrderedDict([('string', 'str'), ('*enum1', 'EnumOne')]))]),
- OrderedDict([('type', 'UserDefTwo'), ('data', OrderedDict([('string0', 'str'), ('dict1', OrderedDict([('string1', 'str'), ('dict2', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')])), ('*dict3', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')]))]))]))]),
+ OrderedDict([('type', 'UserDefTwoDictDict'), ('data', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')]))]),
+ OrderedDict([('type', 'UserDefTwoDict'), ('data', OrderedDict([('string1', 'str'), ('dict2', 'UserDefTwoDictDict'), ('*dict3', 'UserDefTwoDictDict')]))]),
+ OrderedDict([('type', 'UserDefTwo'), ('data', OrderedDict([('string0', 'str'), ('dict1', 'UserDefTwoDict')]))]),
OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]),
OrderedDict([('union', 'UserDefUnion'), ('base', 'UserDefZero'), ('data', OrderedDict([('a', 'UserDefA'), ('b', 'UserDefB')]))]),
@@ -27,7 +29,9 @@
[OrderedDict([('type', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 'EnumOne')]))]),
OrderedDict([('type', 'UserDefZero'), ('data', OrderedDict([('integer', 'int')]))]),
OrderedDict([('type', 'UserDefOne'), ('base', 'UserDefZero'), ('data', OrderedDict([('string', 'str'), ('*enum1', 'EnumOne')]))]),
- OrderedDict([('type', 'UserDefTwo'), ('data', OrderedDict([('string0', 'str'), ('dict1', OrderedDict([('string1', 'str'), ('dict2', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')])), ('*dict3', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')]))]))]))]),
+ OrderedDict([('type', 'UserDefTwoDictDict'), ('data', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')]))]),
+ OrderedDict([('type', 'UserDefTwoDict'), ('data', OrderedDict([('string1', 'str'), ('dict2', 'UserDefTwoDictDict'), ('*dict3', 'UserDefTwoDictDict')]))]),
+ OrderedDict([('type', 'UserDefTwo'), ('data', OrderedDict([('string0', 'str'), ('dict1', 'UserDefTwoDict')]))]),
OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]),
OrderedDict([('type', 'UserDefUnionBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]),
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index 9189cd2..dc199d3 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -33,12 +33,15 @@ UserDefTwo *qmp_user_def_cmd2(UserDefOne *ud1a,
ret = g_malloc0(sizeof(UserDefTwo));
ret->string0 = strdup("blah1");
- ret->dict1.string1 = strdup("blah2");
- ret->dict1.dict2.userdef = ud1c;
- ret->dict1.dict2.string = strdup("blah3");
- ret->dict1.has_dict3 = true;
- ret->dict1.dict3.userdef = ud1d;
- ret->dict1.dict3.string = strdup("blah4");
+ ret->dict1 = g_malloc0(sizeof(UserDefTwoDict));
+ ret->dict1->string1 = strdup("blah2");
+ ret->dict1->dict2 = g_malloc0(sizeof(UserDefTwoDictDict));
+ ret->dict1->dict2->userdef = ud1c;
+ ret->dict1->dict2->string = strdup("blah3");
+ ret->dict1->dict3 = g_malloc0(sizeof(UserDefTwoDictDict));
+ ret->dict1->has_dict3 = true;
+ ret->dict1->dict3->userdef = ud1d;
+ ret->dict1->dict3->string = strdup("blah4");
return ret;
}
@@ -204,7 +207,7 @@ static void test_dealloc_partial(void)
assert(ud2 != NULL);
assert(ud2->string0 != NULL);
assert(strcmp(ud2->string0, text) == 0);
- assert(ud2->dict1.dict2.userdef == NULL);
+ assert(ud2->dict1 == NULL);
/* confirm & release construction error */
assert(err != NULL);
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 3e1da9d..30a5441 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -261,13 +261,15 @@ static void test_visitor_in_struct_nested(TestInputVisitorData *data,
g_assert(!err);
check_and_free_str(udp->string0, "string0");
- check_and_free_str(udp->dict1.string1, "string1");
- g_assert_cmpint(udp->dict1.dict2.userdef->base->integer, ==, 42);
- check_and_free_str(udp->dict1.dict2.userdef->string, "string");
- check_and_free_str(udp->dict1.dict2.string, "string2");
- g_assert(udp->dict1.has_dict3 == false);
+ check_and_free_str(udp->dict1->string1, "string1");
+ g_assert_cmpint(udp->dict1->dict2->userdef->base->integer, ==, 42);
+ check_and_free_str(udp->dict1->dict2->userdef->string, "string");
+ check_and_free_str(udp->dict1->dict2->string, "string2");
+ g_assert(udp->dict1->has_dict3 == false);
- g_free(udp->dict1.dict2.userdef);
+ g_free(udp->dict1->dict2->userdef);
+ g_free(udp->dict1->dict2);
+ g_free(udp->dict1);
g_free(udp);
}
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index 6781a4f..6c81fc7 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -244,19 +244,23 @@ static void test_visitor_out_struct_nested(TestOutputVisitorData *data,
ud2 = g_malloc0(sizeof(*ud2));
ud2->string0 = g_strdup(strings[0]);
- ud2->dict1.string1 = g_strdup(strings[1]);
- ud2->dict1.dict2.userdef = g_malloc0(sizeof(UserDefOne));
- ud2->dict1.dict2.userdef->string = g_strdup(string);
- ud2->dict1.dict2.userdef->base = g_new0(UserDefZero, 1);
- ud2->dict1.dict2.userdef->base->integer = value;
- ud2->dict1.dict2.string = g_strdup(strings[2]);
+ ud2->dict1 = g_malloc0(sizeof(*ud2->dict1));
+ ud2->dict1->string1 = g_strdup(strings[1]);
- ud2->dict1.has_dict3 = true;
- ud2->dict1.dict3.userdef = g_malloc0(sizeof(UserDefOne));
- ud2->dict1.dict3.userdef->string = g_strdup(string);
- ud2->dict1.dict3.userdef->base = g_new0(UserDefZero, 1);
- ud2->dict1.dict3.userdef->base->integer = value;
- ud2->dict1.dict3.string = g_strdup(strings[3]);
+ ud2->dict1->dict2 = g_malloc0(sizeof(*ud2->dict1->dict2));
+ ud2->dict1->dict2->userdef = g_malloc0(sizeof(UserDefOne));
+ ud2->dict1->dict2->userdef->string = g_strdup(string);
+ ud2->dict1->dict2->userdef->base = g_new0(UserDefZero, 1);
+ ud2->dict1->dict2->userdef->base->integer = value;
+ ud2->dict1->dict2->string = g_strdup(strings[2]);
+
+ ud2->dict1->dict3 = g_malloc0(sizeof(*ud2->dict1->dict3));
+ ud2->dict1->has_dict3 = true;
+ ud2->dict1->dict3->userdef = g_malloc0(sizeof(UserDefOne));
+ ud2->dict1->dict3->userdef->string = g_strdup(string);
+ ud2->dict1->dict3->userdef->base = g_new0(UserDefZero, 1);
+ ud2->dict1->dict3->userdef->base->integer = value;
+ ud2->dict1->dict3->string = g_strdup(strings[3]);
visit_type_UserDefTwo(data->ov, &ud2, "unused", &err);
g_assert(!err);
@@ -407,13 +411,15 @@ static void test_visitor_out_list_qapi_free(TestOutputVisitorData *data,
p->value = g_malloc0(sizeof(*p->value));
p->value->string0 = g_strdup(string);
- p->value->dict1.string1 = g_strdup(string);
- p->value->dict1.dict2.userdef = g_malloc0(sizeof(UserDefOne));
- p->value->dict1.dict2.userdef->string = g_strdup(string);
- p->value->dict1.dict2.userdef->base = g_new0(UserDefZero, 1);
- p->value->dict1.dict2.userdef->base->integer = 42;
- p->value->dict1.dict2.string = g_strdup(string);
- p->value->dict1.has_dict3 = false;
+ p->value->dict1 = g_malloc0(sizeof(UserDefTwoDict));
+ p->value->dict1->string1 = g_strdup(string);
+ p->value->dict1->dict2 = g_malloc0(sizeof(UserDefTwoDictDict));
+ p->value->dict1->dict2->userdef = g_malloc0(sizeof(UserDefOne));
+ p->value->dict1->dict2->userdef->string = g_strdup(string);
+ p->value->dict1->dict2->userdef->base = g_new0(UserDefZero, 1);
+ p->value->dict1->dict2->userdef->base->integer = 42;
+ p->value->dict1->dict2->string = g_strdup(string);
+ p->value->dict1->has_dict3 = false;
p->next = head;
head = p;
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index 133f882..40169ff 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -253,18 +253,21 @@ static UserDefTwo *nested_struct_create(void)
{
UserDefTwo *udnp = g_malloc0(sizeof(*udnp));
udnp->string0 = strdup("test_string0");
- udnp->dict1.string1 = strdup("test_string1");
- udnp->dict1.dict2.userdef = g_malloc0(sizeof(UserDefOne));
- udnp->dict1.dict2.userdef->base = g_new0(UserDefZero, 1);
- udnp->dict1.dict2.userdef->base->integer = 42;
- udnp->dict1.dict2.userdef->string = strdup("test_string");
- udnp->dict1.dict2.string = strdup("test_string2");
- udnp->dict1.has_dict3 = true;
- udnp->dict1.dict3.userdef = g_malloc0(sizeof(UserDefOne));
- udnp->dict1.dict3.userdef->base = g_new0(UserDefZero, 1);
- udnp->dict1.dict3.userdef->base->integer = 43;
- udnp->dict1.dict3.userdef->string = strdup("test_string");
- udnp->dict1.dict3.string = strdup("test_string3");
+ udnp->dict1 = g_malloc0(sizeof(*udnp->dict1));
+ udnp->dict1->string1 = strdup("test_string1");
+ udnp->dict1->dict2 = g_malloc0(sizeof(*udnp->dict1->dict2));
+ udnp->dict1->dict2->userdef = g_malloc0(sizeof(UserDefOne));
+ udnp->dict1->dict2->userdef->base = g_new0(UserDefZero, 1);
+ udnp->dict1->dict2->userdef->base->integer = 42;
+ udnp->dict1->dict2->userdef->string = strdup("test_string");
+ udnp->dict1->dict2->string = strdup("test_string2");
+ udnp->dict1->dict3 = g_malloc0(sizeof(*udnp->dict1->dict3));
+ udnp->dict1->has_dict3 = true;
+ udnp->dict1->dict3->userdef = g_malloc0(sizeof(UserDefOne));
+ udnp->dict1->dict3->userdef->base = g_new0(UserDefZero, 1);
+ udnp->dict1->dict3->userdef->base->integer = 43;
+ udnp->dict1->dict3->userdef->string = strdup("test_string");
+ udnp->dict1->dict3->string = strdup("test_string3");
return udnp;
}
@@ -273,18 +276,20 @@ static void nested_struct_compare(UserDefTwo *udnp1, UserDefTwo *udnp2)
g_assert(udnp1);
g_assert(udnp2);
g_assert_cmpstr(udnp1->string0, ==, udnp2->string0);
- g_assert_cmpstr(udnp1->dict1.string1, ==, udnp2->dict1.string1);
- g_assert_cmpint(udnp1->dict1.dict2.userdef->base->integer, ==,
- udnp2->dict1.dict2.userdef->base->integer);
- g_assert_cmpstr(udnp1->dict1.dict2.userdef->string, ==,
- udnp2->dict1.dict2.userdef->string);
- g_assert_cmpstr(udnp1->dict1.dict2.string, ==, udnp2->dict1.dict2.string);
- g_assert(udnp1->dict1.has_dict3 == udnp2->dict1.has_dict3);
- g_assert_cmpint(udnp1->dict1.dict3.userdef->base->integer, ==,
- udnp2->dict1.dict3.userdef->base->integer);
- g_assert_cmpstr(udnp1->dict1.dict3.userdef->string, ==,
- udnp2->dict1.dict3.userdef->string);
- g_assert_cmpstr(udnp1->dict1.dict3.string, ==, udnp2->dict1.dict3.string);
+ g_assert_cmpstr(udnp1->dict1->string1, ==, udnp2->dict1->string1);
+ g_assert_cmpint(udnp1->dict1->dict2->userdef->base->integer, ==,
+ udnp2->dict1->dict2->userdef->base->integer);
+ g_assert_cmpstr(udnp1->dict1->dict2->userdef->string, ==,
+ udnp2->dict1->dict2->userdef->string);
+ g_assert_cmpstr(udnp1->dict1->dict2->string, ==,
+ udnp2->dict1->dict2->string);
+ g_assert(udnp1->dict1->has_dict3 == udnp2->dict1->has_dict3);
+ g_assert_cmpint(udnp1->dict1->dict3->userdef->base->integer, ==,
+ udnp2->dict1->dict3->userdef->base->integer);
+ g_assert_cmpstr(udnp1->dict1->dict3->userdef->string, ==,
+ udnp2->dict1->dict3->userdef->string);
+ g_assert_cmpstr(udnp1->dict1->dict3->string, ==,
+ udnp2->dict1->dict3->string);
}
static void nested_struct_cleanup(UserDefTwo *udnp)
--
1.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH v3 12/14] qapi: drop inline subtype in query-version
2014-08-06 1:14 [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs Eric Blake
` (10 preceding siblings ...)
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 11/14] qapi: drop tests for inline subtypes Eric Blake
@ 2014-08-06 1:14 ` Eric Blake
2014-08-14 11:45 ` Markus Armbruster
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 13/14] qapi: drop inline subtype in query-pci Eric Blake
` (4 subsequent siblings)
16 siblings, 1 reply; 29+ messages in thread
From: Eric Blake @ 2014-08-06 1:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng, Markus Armbruster, wenchaoqemu, Luiz Capitulino
A future patch will be using a 'name':{dictionary} entry in the
QAPI schema to specify a default value for an optional argument;
but existing use of inline substructs conflicts with that goal.
This patch fixes one of only two commands relying on nested
subtypes, by breaking the nesting into an explicit type.
Prefer the safer g_new0() while making the conversion.
* qapi/common.json (VersionInfo): Split inline data...
(VersionTriple): ...into new type.
* qmp.c (qmp_query_version): Update caller.
* hmp.c (hmp_info_version): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
hmp.c | 2 +-
qapi/common.json | 26 +++++++++++++++++++-------
qmp.c | 9 +++++----
3 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/hmp.c b/hmp.c
index 4d1838e..e637fbc 100644
--- a/hmp.c
+++ b/hmp.c
@@ -55,7 +55,7 @@ void hmp_info_version(Monitor *mon, const QDict *qdict)
info = qmp_query_version(NULL);
monitor_printf(mon, "%" PRId64 ".%" PRId64 ".%" PRId64 "%s\n",
- info->qemu.major, info->qemu.minor, info->qemu.micro,
+ info->qemu->major, info->qemu->minor, info->qemu->micro,
info->package);
qapi_free_VersionInfo(info);
diff --git a/qapi/common.json b/qapi/common.json
index 4e9a21f..d007095 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -29,15 +29,28 @@
'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap' ] }
##
+# @VersionTriple
+#
+# A three-part version number.
+#
+# @qemu.major: The major version number.
+#
+# @qemu.minor: The minor version number.
+#
+# @qemu.micro: The micro version number.
+#
+# Since: 2.2
+##
+{ 'type': 'VersionTriple',
+ 'data': {'major': 'int', 'minor': 'int', 'micro': 'int'} }
+
+
+##
# @VersionInfo:
#
# A description of QEMU's version.
#
-# @qemu.major: The major version of QEMU
-#
-# @qemu.minor: The minor version of QEMU
-#
-# @qemu.micro: The micro version of QEMU. By current convention, a micro
+# @qemu: The version of QEMU. By current convention, a micro
# version of 50 signifies a development branch. A micro version
# greater than or equal to 90 signifies a release candidate for
# the next minor version. A micro version of less than 50
@@ -51,8 +64,7 @@
# Since: 0.14.0
##
{ 'type': 'VersionInfo',
- 'data': {'qemu': {'major': 'int', 'minor': 'int', 'micro': 'int'},
- 'package': 'str'} }
+ 'data': {'qemu': 'VersionTriple', 'package': 'str'} }
##
# @query-version:
diff --git a/qmp.c b/qmp.c
index 0d2553a..35e6f7a 100644
--- a/qmp.c
+++ b/qmp.c
@@ -45,15 +45,16 @@ NameInfo *qmp_query_name(Error **errp)
VersionInfo *qmp_query_version(Error **errp)
{
- VersionInfo *info = g_malloc0(sizeof(*info));
+ VersionInfo *info = g_new0(VersionInfo, 1);
const char *version = QEMU_VERSION;
char *tmp;
- info->qemu.major = strtol(version, &tmp, 10);
+ info->qemu = g_new0(VersionTriple, 1);
+ info->qemu->major = strtol(version, &tmp, 10);
tmp++;
- info->qemu.minor = strtol(tmp, &tmp, 10);
+ info->qemu->minor = strtol(tmp, &tmp, 10);
tmp++;
- info->qemu.micro = strtol(tmp, &tmp, 10);
+ info->qemu->micro = strtol(tmp, &tmp, 10);
info->package = g_strdup(QEMU_PKGVERSION);
return info;
--
1.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH v3 13/14] qapi: drop inline subtype in query-pci
2014-08-06 1:14 [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs Eric Blake
` (11 preceding siblings ...)
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 12/14] qapi: drop inline subtype in query-version Eric Blake
@ 2014-08-06 1:14 ` Eric Blake
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 14/14] qapi: drop support for inline subtypes Eric Blake
` (3 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Eric Blake @ 2014-08-06 1:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng, Markus Armbruster, wenchaoqemu, Luiz Capitulino
A future patch will be using a 'name':{dictionary} entry in the
QAPI schema to specify a default value for an optional argument;
but existing use of inline substructs conflicts with that goal.
This patch fixes one of only two commands relying on nested
subtypes, by breaking the nesting into an explicit type.
Prefer the safer g_new0() while making the conversion, and reduce
some long lines.
* qapi-schema.json (PciBridgeInfo, PciDeviceInfo): Split inline
data...
(PciBusInfo, PciDeviceClass, PciDeviceId): ...into new types.
* hmp.c (hmp_info_pci_device): Update caller.
* hw/pci/pci.c (qmp_query_pci_bridge, qmp_query_pci_device):
Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
hmp.c | 26 ++++++++--------
hw/pci/pci.c | 42 ++++++++++++++------------
qapi-schema.json | 90 ++++++++++++++++++++++++++++++++++++++------------------
3 files changed, 98 insertions(+), 60 deletions(-)
diff --git a/hmp.c b/hmp.c
index e637fbc..00085b2 100644
--- a/hmp.c
+++ b/hmp.c
@@ -553,14 +553,14 @@ static void hmp_info_pci_device(Monitor *mon, const PciDeviceInfo *dev)
dev->slot, dev->function);
monitor_printf(mon, " ");
- if (dev->class_info.has_desc) {
- monitor_printf(mon, "%s", dev->class_info.desc);
+ if (dev->class_info->has_desc) {
+ monitor_printf(mon, "%s", dev->class_info->desc);
} else {
- monitor_printf(mon, "Class %04" PRId64, dev->class_info.q_class);
+ monitor_printf(mon, "Class %04" PRId64, dev->class_info->q_class);
}
monitor_printf(mon, ": PCI device %04" PRIx64 ":%04" PRIx64 "\n",
- dev->id.vendor, dev->id.device);
+ dev->id->vendor, dev->id->device);
if (dev->has_irq) {
monitor_printf(mon, " IRQ %" PRId64 ".\n", dev->irq);
@@ -568,25 +568,25 @@ static void hmp_info_pci_device(Monitor *mon, const PciDeviceInfo *dev)
if (dev->has_pci_bridge) {
monitor_printf(mon, " BUS %" PRId64 ".\n",
- dev->pci_bridge->bus.number);
+ dev->pci_bridge->bus->number);
monitor_printf(mon, " secondary bus %" PRId64 ".\n",
- dev->pci_bridge->bus.secondary);
+ dev->pci_bridge->bus->secondary);
monitor_printf(mon, " subordinate bus %" PRId64 ".\n",
- dev->pci_bridge->bus.subordinate);
+ dev->pci_bridge->bus->subordinate);
monitor_printf(mon, " IO range [0x%04"PRIx64", 0x%04"PRIx64"]\n",
- dev->pci_bridge->bus.io_range->base,
- dev->pci_bridge->bus.io_range->limit);
+ dev->pci_bridge->bus->io_range->base,
+ dev->pci_bridge->bus->io_range->limit);
monitor_printf(mon,
" memory range [0x%08"PRIx64", 0x%08"PRIx64"]\n",
- dev->pci_bridge->bus.memory_range->base,
- dev->pci_bridge->bus.memory_range->limit);
+ dev->pci_bridge->bus->memory_range->base,
+ dev->pci_bridge->bus->memory_range->limit);
monitor_printf(mon, " prefetchable memory range "
"[0x%08"PRIx64", 0x%08"PRIx64"]\n",
- dev->pci_bridge->bus.prefetchable_range->base,
- dev->pci_bridge->bus.prefetchable_range->limit);
+ dev->pci_bridge->bus->prefetchable_range->base,
+ dev->pci_bridge->bus->prefetchable_range->limit);
}
for (region = dev->regions; region; region = region->next) {
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 351d320..33d9b25 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1441,24 +1441,26 @@ static PciBridgeInfo *qmp_query_pci_bridge(PCIDevice *dev, PCIBus *bus,
int bus_num)
{
PciBridgeInfo *info;
+ PciMemoryRange *range;
- info = g_malloc0(sizeof(*info));
+ info = g_new0(PciBridgeInfo, 1);
- info->bus.number = dev->config[PCI_PRIMARY_BUS];
- info->bus.secondary = dev->config[PCI_SECONDARY_BUS];
- info->bus.subordinate = dev->config[PCI_SUBORDINATE_BUS];
+ info->bus = g_new0(PciBusInfo, 1);
+ info->bus->number = dev->config[PCI_PRIMARY_BUS];
+ info->bus->secondary = dev->config[PCI_SECONDARY_BUS];
+ info->bus->subordinate = dev->config[PCI_SUBORDINATE_BUS];
- info->bus.io_range = g_malloc0(sizeof(*info->bus.io_range));
- info->bus.io_range->base = pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_IO);
- info->bus.io_range->limit = pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_IO);
+ range = info->bus->io_range = g_new0(PciMemoryRange, 1);
+ range->base = pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_IO);
+ range->limit = pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_IO);
- info->bus.memory_range = g_malloc0(sizeof(*info->bus.memory_range));
- info->bus.memory_range->base = pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_MEMORY);
- info->bus.memory_range->limit = pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_MEMORY);
+ range = info->bus->memory_range = g_new0(PciMemoryRange, 1);
+ range->base = pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_MEMORY);
+ range->limit = pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_MEMORY);
- info->bus.prefetchable_range = g_malloc0(sizeof(*info->bus.prefetchable_range));
- info->bus.prefetchable_range->base = pci_bridge_get_base(dev, PCI_BASE_ADDRESS_MEM_PREFETCH);
- info->bus.prefetchable_range->limit = pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_MEM_PREFETCH);
+ range = info->bus->prefetchable_range = g_new0(PciMemoryRange, 1);
+ range->base = pci_bridge_get_base(dev, PCI_BASE_ADDRESS_MEM_PREFETCH);
+ range->limit = pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_MEM_PREFETCH);
if (dev->config[PCI_SECONDARY_BUS] != 0) {
PCIBus *child_bus = pci_find_bus_nr(bus, dev->config[PCI_SECONDARY_BUS]);
@@ -1479,21 +1481,23 @@ static PciDeviceInfo *qmp_query_pci_device(PCIDevice *dev, PCIBus *bus,
uint8_t type;
int class;
- info = g_malloc0(sizeof(*info));
+ info = g_new0(PciDeviceInfo, 1);
info->bus = bus_num;
info->slot = PCI_SLOT(dev->devfn);
info->function = PCI_FUNC(dev->devfn);
+ info->class_info = g_new0(PciDeviceClass, 1);
class = pci_get_word(dev->config + PCI_CLASS_DEVICE);
- info->class_info.q_class = class;
+ info->class_info->q_class = class;
desc = get_class_desc(class);
if (desc->desc) {
- info->class_info.has_desc = true;
- info->class_info.desc = g_strdup(desc->desc);
+ info->class_info->has_desc = true;
+ info->class_info->desc = g_strdup(desc->desc);
}
- info->id.vendor = pci_get_word(dev->config + PCI_VENDOR_ID);
- info->id.device = pci_get_word(dev->config + PCI_DEVICE_ID);
+ info->id = g_new0(PciDeviceId, 1);
+ info->id->vendor = pci_get_word(dev->config + PCI_VENDOR_ID);
+ info->id->device = pci_get_word(dev->config + PCI_DEVICE_ID);
info->regions = qmp_query_pci_regions(dev);
info->qdev_id = g_strdup(dev->qdev.id ? dev->qdev.id : "");
diff --git a/qapi-schema.json b/qapi-schema.json
index b11aad2..19109cf 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -943,36 +943,75 @@
'*prefetch': 'bool', '*mem_type_64': 'bool' } }
##
+# @PciBusInfo:
+#
+# Information about a bus of a PCI Bridge device
+#
+# @number: primary bus interface number. This should be the number of the
+# bus the device resides on.
+#
+# @secondary: secondary bus interface number. This is the number of the
+# main bus for the bridge
+#
+# @subordinate: This is the highest number bus that resides below the
+# bridge.
+#
+# @io_range: The PIO range for all devices on this bridge
+#
+# @memory_range: The MMIO range for all devices on this bridge
+#
+# @prefetchable_range: The range of prefetchable MMIO for all devices on
+# this bridge
+#
+# Since: 2.2
+##
+{ 'type': 'PciBusInfo',
+ 'data': {'number': 'int', 'secondary': 'int', 'subordinate': 'int',
+ 'io_range': 'PciMemoryRange',
+ 'memory_range': 'PciMemoryRange',
+ 'prefetchable_range': 'PciMemoryRange' } }
+
+##
# @PciBridgeInfo:
#
# Information about a PCI Bridge device
#
-# @bus.number: primary bus interface number. This should be the number of the
-# bus the device resides on.
-#
-# @bus.secondary: secondary bus interface number. This is the number of the
-# main bus for the bridge
-#
-# @bus.subordinate: This is the highest number bus that resides below the
-# bridge.
-#
-# @bus.io_range: The PIO range for all devices on this bridge
-#
-# @bus.memory_range: The MMIO range for all devices on this bridge
-#
-# @bus.prefetchable_range: The range of prefetchable MMIO for all devices on
-# this bridge
+# @bus: information about the bus the device resides on
#
# @devices: a list of @PciDeviceInfo for each device on this bridge
#
# Since: 0.14.0
##
{ 'type': 'PciBridgeInfo',
- 'data': {'bus': { 'number': 'int', 'secondary': 'int', 'subordinate': 'int',
- 'io_range': 'PciMemoryRange',
- 'memory_range': 'PciMemoryRange',
- 'prefetchable_range': 'PciMemoryRange' },
- '*devices': ['PciDeviceInfo']} }
+ 'data': {'bus': 'PciBusInfo', '*devices': ['PciDeviceInfo']} }
+
+##
+# @PciDeviceClass:
+#
+# Information about the Class of a PCI device
+#
+# @desc: #optional a string description of the device's class
+#
+# @class: the class code of the device
+#
+# Since: 2.2
+##
+{ 'type': 'PciDeviceClass',
+ 'data': {'*desc': 'str', 'class': 'int'} }
+
+##
+# @PciDeviceId:
+#
+# Information about the Id of a PCI device
+#
+# @device: the PCI device id
+#
+# @vendor: the PCI vendor id
+#
+# Since: 2.2
+##
+{ 'type': 'PciDeviceId',
+ 'data': {'device': 'int', 'vendor': 'int'} }
##
# @PciDeviceInfo:
@@ -985,13 +1024,9 @@
#
# @function: the function of the slot used by the device
#
-# @class_info.desc: #optional a string description of the device's class
+# @class_info: the class of the device
#
-# @class_info.class: the class code of the device
-#
-# @id.device: the PCI device id
-#
-# @id.vendor: the PCI vendor id
+# @id: the PCI device id
#
# @irq: #optional if an IRQ is assigned to the device, the IRQ number
#
@@ -1008,8 +1043,7 @@
##
{ 'type': 'PciDeviceInfo',
'data': {'bus': 'int', 'slot': 'int', 'function': 'int',
- 'class_info': {'*desc': 'str', 'class': 'int'},
- 'id': {'device': 'int', 'vendor': 'int'},
+ 'class_info': 'PciDeviceClass', 'id': 'PciDeviceId',
'*irq': 'int', 'qdev_id': 'str', '*pci_bridge': 'PciBridgeInfo',
'regions': ['PciMemoryRegion']} }
--
1.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH v3 14/14] qapi: drop support for inline subtypes
2014-08-06 1:14 [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs Eric Blake
` (12 preceding siblings ...)
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 13/14] qapi: drop inline subtype in query-pci Eric Blake
@ 2014-08-06 1:14 ` Eric Blake
2014-08-12 12:47 ` [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs Eric Blake
` (2 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Eric Blake @ 2014-08-06 1:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng, Markus Armbruster, wenchaoqemu, Luiz Capitulino
A future patch will be using a 'name':{dictionary} entry in the
QAPI schema to specify a default value for an optional argument;
but existing use of inline substructs conflicts with that goal.
Now that all commands have been fixed to avoid inline substructs,
nuke support for them, and turn it into a hard error.
* scripts/qapi.py (check_type): Enforce no nested dicts up front.
(parse_args): Drop structured support.
(check_event): Drop; no longer needed.
* scripts/qapi-types.py (generate_struct_fields): Adjust caller.
* scripts/qapi-visit.py (generate_visit_struct_fields): Likewise.
* scripts/qapi-event.py (_generate_event_api_name)
(generate_event_implement): Likewise.
* scripts/qapi-commands.py (generate_command_decl)
(gen_sync_call, gen_visitor_input_vars_decl)
(gen_visitor_input_block): Likewise.
* tests/qapi-schema/event-nest-struct.err: Update expected result.
* tests/qapi-schema/nested-struct.*: New files.
* tests/qapi-schema/nested-struct-returns.*: Likewise.
* tests/Makefile (check-qapi-schema-y): Run them.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi-commands.py | 8 +++---
scripts/qapi-event.py | 4 +--
scripts/qapi-types.py | 9 ++-----
scripts/qapi-visit.py | 37 ++++------------------------
scripts/qapi.py | 29 ++++++++++------------
tests/Makefile | 3 ++-
tests/qapi-schema/event-nest-struct.err | 2 +-
tests/qapi-schema/nested-struct-data.err | 1 +
tests/qapi-schema/nested-struct-data.exit | 1 +
tests/qapi-schema/nested-struct-data.json | 3 +++
tests/qapi-schema/nested-struct-data.out | 0
tests/qapi-schema/nested-struct-returns.err | 1 +
tests/qapi-schema/nested-struct-returns.exit | 1 +
tests/qapi-schema/nested-struct-returns.json | 2 ++
tests/qapi-schema/nested-struct-returns.out | 0
15 files changed, 38 insertions(+), 63 deletions(-)
create mode 100644 tests/qapi-schema/nested-struct-data.err
create mode 100644 tests/qapi-schema/nested-struct-data.exit
create mode 100644 tests/qapi-schema/nested-struct-data.json
create mode 100644 tests/qapi-schema/nested-struct-data.out
create mode 100644 tests/qapi-schema/nested-struct-returns.err
create mode 100644 tests/qapi-schema/nested-struct-returns.exit
create mode 100644 tests/qapi-schema/nested-struct-returns.json
create mode 100644 tests/qapi-schema/nested-struct-returns.out
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 053ba85..db81044 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -28,7 +28,7 @@ def type_visitor(name):
def generate_command_decl(name, args, ret_type):
arglist=""
- for argname, argtype, optional, structured in parse_args(args):
+ for argname, argtype, optional in parse_args(args):
argtype = c_type(argtype, is_param=True)
if optional:
arglist += "bool has_%s, " % c_var(argname)
@@ -53,7 +53,7 @@ def gen_sync_call(name, args, ret_type, indent=0):
retval=""
if ret_type:
retval = "retval = "
- for argname, argtype, optional, structured in parse_args(args):
+ for argname, argtype, optional in parse_args(args):
if optional:
arglist += "has_%s, " % c_var(argname)
arglist += "%s, " % (c_var(argname))
@@ -96,7 +96,7 @@ Visitor *v;
def gen_visitor_input_vars_decl(args):
ret = ""
push_indent()
- for argname, argtype, optional, structured in parse_args(args):
+ for argname, argtype, optional in parse_args(args):
if optional:
ret += mcgen('''
bool has_%(argname)s = false;
@@ -139,7 +139,7 @@ v = qapi_dealloc_get_visitor(md);
v = qmp_input_get_visitor(mi);
''')
- for argname, argtype, optional, structured in parse_args(args):
+ for argname, argtype, optional in parse_args(args):
if optional:
ret += mcgen('''
visit_optional(v, &has_%(c_name)s, "%(name)s", %(errp)s);
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index 601e307..47dc041 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -21,7 +21,7 @@ def _generate_event_api_name(event_name, params):
l = len(api_name)
if params:
- for argname, argentry, optional, structured in parse_args(params):
+ for argname, argentry, optional in parse_args(params):
if optional:
api_name += "bool has_%s,\n" % c_var(argname)
api_name += "".ljust(l)
@@ -93,7 +93,7 @@ def generate_event_implement(api_name, event_name, params):
""",
event_name = event_name)
- for argname, argentry, optional, structured in parse_args(params):
+ for argname, argentry, optional in parse_args(params):
if optional:
ret += mcgen("""
if (has_%(var)s) {
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index b463232..5d0e6e7 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -63,18 +63,13 @@ typedef struct %(name)sList
def generate_struct_fields(members):
ret = ''
- for argname, argentry, optional, structured in parse_args(members):
+ for argname, argentry, optional in parse_args(members):
if optional:
ret += mcgen('''
bool has_%(c_name)s;
''',
c_name=c_var(argname))
- if structured:
- push_indent()
- ret += generate_struct({ "field": argname, "data": argentry})
- pop_indent()
- else:
- ret += mcgen('''
+ ret += mcgen('''
%(c_type)s %(c_name)s;
''',
c_type=c_type(argentry), c_name=c_var(argname))
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index c129697..7674dd5 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -51,27 +51,6 @@ def generate_visit_struct_fields(name, field_prefix, fn_prefix, members, base =
else:
full_name = "%s_%s" % (name, fn_prefix)
- for argname, argentry, optional, structured in parse_args(members):
- if structured:
- if not fn_prefix:
- nested_fn_prefix = argname
- else:
- nested_fn_prefix = "%s_%s" % (fn_prefix, argname)
-
- nested_field_prefix = "%s%s." % (field_prefix, argname)
- ret += generate_visit_struct_fields(name, nested_field_prefix,
- nested_fn_prefix, argentry)
- ret += mcgen('''
-
-static void visit_type_%(full_name)s_field_%(c_name)s(Visitor *m, %(name)s **obj, Error **errp)
-{
-''',
- name=name, full_name=full_name, c_name=c_var(argname))
- ret += generate_visit_struct_body(full_name, argname, argentry)
- ret += mcgen('''
-}
-''')
-
if base:
ret += generate_visit_implicit_struct(base)
@@ -94,7 +73,7 @@ if (err) {
c_prefix=c_var(field_prefix),
type=type_name(base), c_name=c_var('base'))
- for argname, argentry, optional, structured in parse_args(members):
+ for argname, argentry, optional in parse_args(members):
if optional:
ret += mcgen('''
visit_optional(m, &(*obj)->%(c_prefix)shas_%(c_name)s, "%(name)s", &err);
@@ -104,18 +83,12 @@ if (!err && (*obj)->%(prefix)shas_%(c_name)s) {
c_name=c_var(argname), name=argname)
push_indent()
- if structured:
- ret += mcgen('''
-visit_type_%(full_name)s_field_%(c_name)s(m, obj, &err);
-''',
- full_name=full_name, c_name=c_var(argname))
- else:
- ret += mcgen('''
+ ret += mcgen('''
visit_type_%(type)s(m, &(*obj)->%(c_prefix)s%(c_name)s, "%(name)s", &err);
''',
- c_prefix=c_var(field_prefix), prefix=field_prefix,
- type=type_name(argentry), c_name=c_var(argname),
- name=argname)
+ c_prefix=c_var(field_prefix), prefix=field_prefix,
+ type=type_name(argentry), c_name=c_var(argname),
+ name=argname)
if optional:
pop_indent()
diff --git a/scripts/qapi.py b/scripts/qapi.py
index e4d27d6..7fec348 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -261,16 +261,6 @@ def expr_name(expr):
return expr['event']
return None
-def check_event(expr, expr_info):
- params = expr.get('data')
- if params:
- for argname, argentry, optional, structured in parse_args(params):
- if structured:
- raise QAPIExprError(expr_info,
- "Nested structure define in event is not "
- "supported, event '%s', argname '%s'"
- % (expr['event'], argname))
-
def check_union(expr, expr_info):
name = expr['union']
base = expr.get('base')
@@ -354,6 +344,14 @@ def check_type(expr_info, name, data, allow_native = False):
elif not isinstance(data, OrderedDict):
raise QAPIExprError(expr_info,
"Expecting dictionary for data of '%s'" % name)
+ else:
+ for (key, value) in data.items():
+ # Todo: allow dictionaries to represent default values of
+ # an optional argument.
+ if isinstance(value, OrderedDict):
+ raise QAPIExprError(expr_info,
+ "Nested structure not supported for field "
+ "'%s' of '%s'" % (key, name))
def check_exprs(schema):
for expr_elem in schema.exprs:
@@ -375,8 +373,8 @@ def check_exprs(schema):
if expr.has_key('union'):
check_union(expr, info)
- if expr.has_key('event'):
- check_event(expr, info)
+ # 'enum' must have 'data':[]; all other expressions have optional
+ # 'data'; if present it is either a typename or a dict
if expr.has_key('enum'):
if not isinstance(members, list):
raise QAPIExprError(info,
@@ -431,13 +429,12 @@ def parse_args(typeinfo):
argname = member
argentry = typeinfo[member]
optional = False
- structured = False
if member.startswith('*'):
argname = member[1:]
optional = True
- if isinstance(argentry, OrderedDict):
- structured = True
- yield (argname, argentry, optional, structured)
+ # Todo: allow argentry to be OrderedDict, for providing the
+ # value of an optional argument.
+ yield (argname, argentry, optional)
def de_camel_case(name):
new_name = ''
diff --git a/tests/Makefile b/tests/Makefile
index c36faca..cf60e23 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -196,7 +196,8 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
data-array-unknown.json data-unknown.json data-int.json \
returns-unknown.json returns-int.json returns-array-bad.json \
missing-colon.json missing-comma-list.json \
- missing-comma-object.json non-objects.json \
+ missing-comma-object.json nested-struct-data.json \
+ nested-struct-returns.json non-objects.json \
qapi-schema-test.json quoted-structural-chars.json \
trailing-comma-list.json trailing-comma-object.json \
unclosed-list.json unclosed-object.json unclosed-string.json \
diff --git a/tests/qapi-schema/event-nest-struct.err b/tests/qapi-schema/event-nest-struct.err
index 91bde1c..3c0fc2c 100644
--- a/tests/qapi-schema/event-nest-struct.err
+++ b/tests/qapi-schema/event-nest-struct.err
@@ -1 +1 @@
-tests/qapi-schema/event-nest-struct.json:1: Nested structure define in event is not supported, event 'EVENT_A', argname 'a'
+tests/qapi-schema/event-nest-struct.json:1: Nested structure not supported for field 'a' of 'EVENT_A'
diff --git a/tests/qapi-schema/nested-struct-data.err b/tests/qapi-schema/nested-struct-data.err
new file mode 100644
index 0000000..5d384ad
--- /dev/null
+++ b/tests/qapi-schema/nested-struct-data.err
@@ -0,0 +1 @@
+tests/qapi-schema/nested-struct-data.json:1: Nested structure not supported for field 'a' of 'foo'
diff --git a/tests/qapi-schema/nested-struct-data.exit b/tests/qapi-schema/nested-struct-data.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/nested-struct-data.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/nested-struct-data.json b/tests/qapi-schema/nested-struct-data.json
new file mode 100644
index 0000000..a18fc17
--- /dev/null
+++ b/tests/qapi-schema/nested-struct-data.json
@@ -0,0 +1,3 @@
+{ 'command': 'foo',
+ 'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' },
+ 'returns': {} }
diff --git a/tests/qapi-schema/nested-struct-data.out b/tests/qapi-schema/nested-struct-data.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/nested-struct-returns.err b/tests/qapi-schema/nested-struct-returns.err
new file mode 100644
index 0000000..ce5a675
--- /dev/null
+++ b/tests/qapi-schema/nested-struct-returns.err
@@ -0,0 +1 @@
+tests/qapi-schema/nested-struct-returns.json:1: Nested structure not supported for field 'a' of 'foo'
diff --git a/tests/qapi-schema/nested-struct-returns.exit b/tests/qapi-schema/nested-struct-returns.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/nested-struct-returns.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/nested-struct-returns.json b/tests/qapi-schema/nested-struct-returns.json
new file mode 100644
index 0000000..f7ddf80
--- /dev/null
+++ b/tests/qapi-schema/nested-struct-returns.json
@@ -0,0 +1,2 @@
+{ 'command': 'foo',
+ 'returns': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
diff --git a/tests/qapi-schema/nested-struct-returns.out b/tests/qapi-schema/nested-struct-returns.out
new file mode 100644
index 0000000..e69de29
--
1.9.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs
2014-08-06 1:14 [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs Eric Blake
` (13 preceding siblings ...)
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 14/14] qapi: drop support for inline subtypes Eric Blake
@ 2014-08-12 12:47 ` Eric Blake
2014-08-14 12:00 ` Markus Armbruster
2014-08-15 18:12 ` Luiz Capitulino
16 siblings, 0 replies; 29+ messages in thread
From: Eric Blake @ 2014-08-12 12:47 UTC (permalink / raw)
To: qemu-devel; +Cc: Luiz Capitulino, Fam Zheng, Markus Armbruster, wenchaoqemu
[-- Attachment #1: Type: text/plain, Size: 9464 bytes --]
ping
On 08/05/2014 07:14 PM, Eric Blake wrote:
> According to this email:
> https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg00708.html
> we want to repurpose 'data': { 'name': {dict...} } in qapi files
> for future use of designating default values of optional parameters.
> But to do that, we must first nuke existing use of that syntax for
> declaring nested structs. Enhancing the testsuite while at it
> never hurts.
>
> v3:
> No code changes, but fix mis-send of 11-14 [Peter]
>
> v2:
> New patches: 1-2, 5-9
> consistent TAB usage in Makefile [Fam]
> catch more bad coding constructs, and test them
> avoid code duplication in type validity checks (patch 14 [former 7] is simpler because of patch 9)
>
> Eric Blake (14):
> qapi: consistent whitespace in tests/Makefile
> qapi: ignore files created during make check
> qapi: add some enum tests
> qapi: better error message for bad enum
> qapi: add some expr tests
> qapi: require valid expressions
> qapi: add some type check tests
> qapi: add expr_name() helper
> qapi: add check_type helper function
> qapi: merge UserDefTwo and UserDefNested in tests
> qapi: drop tests for inline subtypes
> qapi: drop inline subtype in query-version
> qapi: drop inline subtype in query-pci
> qapi: drop support for inline subtypes
>
> hmp.c | 28 ++++----
> hw/pci/pci.c | 42 ++++++------
> qapi-schema.json | 90 ++++++++++++++++++--------
> qapi/common.json | 26 ++++++--
> qmp.c | 9 +--
> scripts/qapi-commands.py | 8 +--
> scripts/qapi-event.py | 4 +-
> scripts/qapi-types.py | 9 +--
> scripts/qapi-visit.py | 37 ++---------
> scripts/qapi.py | 96 ++++++++++++++++++++++------
> tests/.gitignore | 3 +
> tests/Makefile | 37 ++++++-----
> tests/qapi-schema/data-array-empty.err | 0
> tests/qapi-schema/data-array-empty.exit | 1 +
> tests/qapi-schema/data-array-empty.json | 1 +
> tests/qapi-schema/data-array-empty.out | 3 +
> tests/qapi-schema/data-array-unknown.err | 1 +
> tests/qapi-schema/data-array-unknown.exit | 1 +
> tests/qapi-schema/data-array-unknown.json | 1 +
> tests/qapi-schema/data-array-unknown.out | 0
> tests/qapi-schema/data-int.err | 1 +
> tests/qapi-schema/data-int.exit | 1 +
> tests/qapi-schema/data-int.json | 1 +
> tests/qapi-schema/data-int.out | 0
> tests/qapi-schema/data-unknown.err | 1 +
> tests/qapi-schema/data-unknown.exit | 1 +
> tests/qapi-schema/data-unknown.json | 1 +
> tests/qapi-schema/data-unknown.out | 0
> tests/qapi-schema/double-type.err | 1 +
> tests/qapi-schema/double-type.exit | 1 +
> tests/qapi-schema/double-type.json | 1 +
> tests/qapi-schema/double-type.out | 0
> tests/qapi-schema/enum-empty.err | 0
> tests/qapi-schema/enum-empty.exit | 1 +
> tests/qapi-schema/enum-empty.json | 1 +
> tests/qapi-schema/enum-empty.out | 3 +
> tests/qapi-schema/enum-missing-data.err | 1 +
> tests/qapi-schema/enum-missing-data.exit | 1 +
> tests/qapi-schema/enum-missing-data.json | 1 +
> tests/qapi-schema/enum-missing-data.out | 0
> tests/qapi-schema/enum-wrong-data.err | 1 +
> tests/qapi-schema/enum-wrong-data.exit | 1 +
> tests/qapi-schema/enum-wrong-data.json | 1 +
> tests/qapi-schema/enum-wrong-data.out | 0
> tests/qapi-schema/event-nest-struct.err | 2 +-
> tests/qapi-schema/indented-expr.json | 4 +-
> tests/qapi-schema/indented-expr.out | 2 +-
> tests/qapi-schema/missing-type.err | 1 +
> tests/qapi-schema/missing-type.exit | 1 +
> tests/qapi-schema/missing-type.json | 1 +
> tests/qapi-schema/missing-type.out | 0
> tests/qapi-schema/nested-struct-data.err | 1 +
> tests/qapi-schema/nested-struct-data.exit | 1 +
> tests/qapi-schema/nested-struct-data.json | 3 +
> tests/qapi-schema/nested-struct-data.out | 0
> tests/qapi-schema/nested-struct-returns.err | 1 +
> tests/qapi-schema/nested-struct-returns.exit | 1 +
> tests/qapi-schema/nested-struct-returns.json | 2 +
> tests/qapi-schema/nested-struct-returns.out | 0
> tests/qapi-schema/qapi-schema-test.json | 18 +++---
> tests/qapi-schema/qapi-schema-test.out | 10 +--
> tests/qapi-schema/returns-array-bad.err | 1 +
> tests/qapi-schema/returns-array-bad.exit | 1 +
> tests/qapi-schema/returns-array-bad.json | 1 +
> tests/qapi-schema/returns-array-bad.out | 0
> tests/qapi-schema/returns-int.err | 0
> tests/qapi-schema/returns-int.exit | 1 +
> tests/qapi-schema/returns-int.json | 1 +
> tests/qapi-schema/returns-int.out | 3 +
> tests/qapi-schema/returns-unknown.err | 1 +
> tests/qapi-schema/returns-unknown.exit | 1 +
> tests/qapi-schema/returns-unknown.json | 1 +
> tests/qapi-schema/returns-unknown.out | 0
> tests/test-qmp-commands.c | 35 +++++-----
> tests/test-qmp-input-strict.c | 19 +++---
> tests/test-qmp-input-visitor.c | 25 +++++---
> tests/test-qmp-output-visitor.c | 64 ++++++++++---------
> tests/test-visitor-serialization.c | 84 +++++++++++++-----------
> 78 files changed, 433 insertions(+), 270 deletions(-)
> create mode 100644 tests/qapi-schema/data-array-empty.err
> create mode 100644 tests/qapi-schema/data-array-empty.exit
> create mode 100644 tests/qapi-schema/data-array-empty.json
> create mode 100644 tests/qapi-schema/data-array-empty.out
> create mode 100644 tests/qapi-schema/data-array-unknown.err
> create mode 100644 tests/qapi-schema/data-array-unknown.exit
> create mode 100644 tests/qapi-schema/data-array-unknown.json
> create mode 100644 tests/qapi-schema/data-array-unknown.out
> create mode 100644 tests/qapi-schema/data-int.err
> create mode 100644 tests/qapi-schema/data-int.exit
> create mode 100644 tests/qapi-schema/data-int.json
> create mode 100644 tests/qapi-schema/data-int.out
> create mode 100644 tests/qapi-schema/data-unknown.err
> create mode 100644 tests/qapi-schema/data-unknown.exit
> create mode 100644 tests/qapi-schema/data-unknown.json
> create mode 100644 tests/qapi-schema/data-unknown.out
> create mode 100644 tests/qapi-schema/double-type.err
> create mode 100644 tests/qapi-schema/double-type.exit
> create mode 100644 tests/qapi-schema/double-type.json
> create mode 100644 tests/qapi-schema/double-type.out
> create mode 100644 tests/qapi-schema/enum-empty.err
> create mode 100644 tests/qapi-schema/enum-empty.exit
> create mode 100644 tests/qapi-schema/enum-empty.json
> create mode 100644 tests/qapi-schema/enum-empty.out
> create mode 100644 tests/qapi-schema/enum-missing-data.err
> create mode 100644 tests/qapi-schema/enum-missing-data.exit
> create mode 100644 tests/qapi-schema/enum-missing-data.json
> create mode 100644 tests/qapi-schema/enum-missing-data.out
> create mode 100644 tests/qapi-schema/enum-wrong-data.err
> create mode 100644 tests/qapi-schema/enum-wrong-data.exit
> create mode 100644 tests/qapi-schema/enum-wrong-data.json
> create mode 100644 tests/qapi-schema/enum-wrong-data.out
> create mode 100644 tests/qapi-schema/missing-type.err
> create mode 100644 tests/qapi-schema/missing-type.exit
> create mode 100644 tests/qapi-schema/missing-type.json
> create mode 100644 tests/qapi-schema/missing-type.out
> create mode 100644 tests/qapi-schema/nested-struct-data.err
> create mode 100644 tests/qapi-schema/nested-struct-data.exit
> create mode 100644 tests/qapi-schema/nested-struct-data.json
> create mode 100644 tests/qapi-schema/nested-struct-data.out
> create mode 100644 tests/qapi-schema/nested-struct-returns.err
> create mode 100644 tests/qapi-schema/nested-struct-returns.exit
> create mode 100644 tests/qapi-schema/nested-struct-returns.json
> create mode 100644 tests/qapi-schema/nested-struct-returns.out
> create mode 100644 tests/qapi-schema/returns-array-bad.err
> create mode 100644 tests/qapi-schema/returns-array-bad.exit
> create mode 100644 tests/qapi-schema/returns-array-bad.json
> create mode 100644 tests/qapi-schema/returns-array-bad.out
> create mode 100644 tests/qapi-schema/returns-int.err
> create mode 100644 tests/qapi-schema/returns-int.exit
> create mode 100644 tests/qapi-schema/returns-int.json
> create mode 100644 tests/qapi-schema/returns-int.out
> create mode 100644 tests/qapi-schema/returns-unknown.err
> create mode 100644 tests/qapi-schema/returns-unknown.exit
> create mode 100644 tests/qapi-schema/returns-unknown.json
> create mode 100644 tests/qapi-schema/returns-unknown.out
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH v3 03/14] qapi: add some enum tests
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 03/14] qapi: add some enum tests Eric Blake
@ 2014-08-14 9:23 ` Markus Armbruster
2014-08-14 12:21 ` Eric Blake
0 siblings, 1 reply; 29+ messages in thread
From: Markus Armbruster @ 2014-08-14 9:23 UTC (permalink / raw)
To: Eric Blake; +Cc: Luiz Capitulino, Fam Zheng, qemu-devel, wenchaoqemu
Eric Blake <eblake@redhat.com> writes:
> Demonstrate that the qapi generator doesn't deal well with enums
> that aren't up to par. Later patches will update the expected
> results as the generator is made stricter.
>
> * tests/qapi-schema/enum-empty.*: New files.
> * tests/qapi-schema/enum-missing-data.*: Likewise.
> * tests/qapi-schema/enum-wrong-data.*: Likewise.
> * tests/Makefile (check-qapi-schema-y): Run them.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
[...]
> diff --git a/tests/qapi-schema/enum-missing-data.err b/tests/qapi-schema/enum-missing-data.err
> new file mode 100644
> index 0000000..1fec213
> --- /dev/null
> +++ b/tests/qapi-schema/enum-missing-data.err
> @@ -0,0 +1,6 @@
> +Traceback (most recent call last):
> + File "tests/qapi-schema/test-qapi.py", line 19, in <module>
> + exprs = parse_schema(sys.argv[1])
> + File "scripts/qapi.py", line 339, in parse_schema
> + add_enum(expr['enum'], expr['data'])
> +KeyError: 'data'
> diff --git a/tests/qapi-schema/enum-missing-data.exit b/tests/qapi-schema/enum-missing-data.exit
> new file mode 100644
> index 0000000..d00491f
> --- /dev/null
> +++ b/tests/qapi-schema/enum-missing-data.exit
> @@ -0,0 +1 @@
> +1
> diff --git a/tests/qapi-schema/enum-missing-data.json b/tests/qapi-schema/enum-missing-data.json
> new file mode 100644
> index 0000000..6e2acd8
> --- /dev/null
> +++ b/tests/qapi-schema/enum-missing-data.json
> @@ -0,0 +1 @@
> +{ 'enum': 'MyEnum' }
> diff --git a/tests/qapi-schema/enum-missing-data.out b/tests/qapi-schema/enum-missing-data.out
> new file mode 100644
> index 0000000..e69de29
> diff --git a/tests/qapi-schema/enum-wrong-data.err b/tests/qapi-schema/enum-wrong-data.err
> new file mode 100644
> index 0000000..e69de29
> diff --git a/tests/qapi-schema/enum-wrong-data.exit b/tests/qapi-schema/enum-wrong-data.exit
> new file mode 100644
> index 0000000..573541a
> --- /dev/null
> +++ b/tests/qapi-schema/enum-wrong-data.exit
> @@ -0,0 +1 @@
> +0
> diff --git a/tests/qapi-schema/enum-wrong-data.json b/tests/qapi-schema/enum-wrong-data.json
> new file mode 100644
> index 0000000..4b7e90c
> --- /dev/null
> +++ b/tests/qapi-schema/enum-wrong-data.json
> @@ -0,0 +1 @@
> +{ 'enum': 'MyEnum', 'data': { 'value': 'str' } }
> diff --git a/tests/qapi-schema/enum-wrong-data.out b/tests/qapi-schema/enum-wrong-data.out
> new file mode 100644
> index 0000000..28d2211
> --- /dev/null
> +++ b/tests/qapi-schema/enum-wrong-data.out
> @@ -0,0 +1,3 @@
> +[OrderedDict([('enum', 'MyEnum'), ('data', OrderedDict([('value', 'str')]))])]
> +[{'enum_name': 'MyEnum', 'enum_values': OrderedDict([('value', 'str')])}]
> +[]
For tests demonstrating incorrect behavior, a comment describing the
expected behavior is always welcome.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH v3 06/14] qapi: require valid expressions
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 06/14] qapi: require valid expressions Eric Blake
@ 2014-08-14 9:38 ` Markus Armbruster
2014-08-14 12:26 ` Eric Blake
0 siblings, 1 reply; 29+ messages in thread
From: Markus Armbruster @ 2014-08-14 9:38 UTC (permalink / raw)
To: Eric Blake; +Cc: Luiz Capitulino, Fam Zheng, qemu-devel, wenchaoqemu
Eric Blake <eblake@redhat.com> writes:
> The previous patch demonstrated that the generator could get
> confused if an expression had conflicting meta-types, and
> silently ignored expressions that lacked a known meta-type.
> Fix both cases to give a sane error message.
>
> * scripts/qapi.py (check_exprs): Require a valid meta-type for
> every expression.
> * tests/qapi-schema/indented-expr.*: Use valid types.
> * tests/qapi-schema/missing-type.*: Update expected output.
> * tests/qapi-schema/double-type.*: Likewise.
Conventional git commit message followed by GNU change log. Unusual in
QEMU. Same elsewhere.
Explanation of the change to indented-expr.json hides in the change log
part, where I promptly missed it :)
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> scripts/qapi.py | 12 ++++++++++++
> tests/qapi-schema/double-type.err | 1 +
> tests/qapi-schema/double-type.exit | 2 +-
> tests/qapi-schema/double-type.out | 3 ---
> tests/qapi-schema/indented-expr.json | 4 ++--
> tests/qapi-schema/indented-expr.out | 2 +-
> tests/qapi-schema/missing-type.err | 1 +
> tests/qapi-schema/missing-type.exit | 2 +-
> tests/qapi-schema/missing-type.out | 3 ---
> 9 files changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 1082416..910e422 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -322,6 +322,18 @@ def check_exprs(schema):
> info = expr_elem['info']
> members = expr.get('data')
>
> + # 'include' has already been flattened; at this point, all exprs
> + # should have one of the remaining keys
> + keys = (expr.has_key('enum') + expr.has_key('union') +
> + expr.has_key('type') + expr.has_key('command') +
> + expr.has_key('event'))
> + if keys < 1:
> + raise QAPIExprError(info,
> + "Missing expression meta-type")
> + if keys > 1:
> + raise QAPIExprError(info,
> + "Conflicting expression meta-types")
> +
> if expr.has_key('union'):
> check_union(expr, info)
> if expr.has_key('event'):
Not the friendliest error messages, but they'll do.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH v3 07/14] qapi: add some type check tests
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 07/14] qapi: add some type check tests Eric Blake
@ 2014-08-14 9:47 ` Markus Armbruster
2014-08-14 12:26 ` Eric Blake
0 siblings, 1 reply; 29+ messages in thread
From: Markus Armbruster @ 2014-08-14 9:47 UTC (permalink / raw)
To: Eric Blake; +Cc: Luiz Capitulino, Fam Zheng, qemu-devel, wenchaoqemu
Eric Blake <eblake@redhat.com> writes:
> Demonstrate that the qapi generator silently parses confusing
> types, which may cause other errors later on. Later patches
> will update the expected results as the generator is made stricter.
>
> * tests/qapi-schema/data-array-empty.*: New files.
> * tests/qapi-schema/data-array-unknown.*: Likewise.
> * tests/qapi-schema/data-unknown.*: Likewise.
> * tests/qapi-schema/data-int.*: Likewise.
> * tests/qapi-schema/returns-unknown.*: Likewise.
> * tests/qapi-schema/returns-int.*: Likewise.
> * tests/qapi-schema/returns-array-bad.*: Likewise.
> * tests/Makefile (check-qapi-schema-y): Run them.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
[...]
> diff --git a/tests/qapi-schema/returns-array-bad.err b/tests/qapi-schema/returns-array-bad.err
> new file mode 100644
> index 0000000..e69de29
> diff --git a/tests/qapi-schema/returns-array-bad.exit b/tests/qapi-schema/returns-array-bad.exit
> new file mode 100644
> index 0000000..573541a
> --- /dev/null
> +++ b/tests/qapi-schema/returns-array-bad.exit
> @@ -0,0 +1 @@
> +0
> diff --git a/tests/qapi-schema/returns-array-bad.json b/tests/qapi-schema/returns-array-bad.json
> new file mode 100644
> index 0000000..76bf6df
> --- /dev/null
> +++ b/tests/qapi-schema/returns-array-bad.json
> @@ -0,0 +1 @@
> +{ 'command': 'oops', 'data': [ 'str', 'str' ] }
The file name suggests you want to test 'returns' rather than 'data' here.
[...]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH v3 08/14] qapi: add expr_name() helper
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 08/14] qapi: add expr_name() helper Eric Blake
@ 2014-08-14 9:49 ` Markus Armbruster
0 siblings, 0 replies; 29+ messages in thread
From: Markus Armbruster @ 2014-08-14 9:49 UTC (permalink / raw)
To: Eric Blake; +Cc: Luiz Capitulino, Fam Zheng, qemu-devel, wenchaoqemu
Eric Blake <eblake@redhat.com> writes:
> Now that we know every expression has a known meta-type, we
> can add a helper function that retrieves the name of an
> arbitrary expression, for use in future error messages.
>
> * scripts/qapi.py (expr_name): New function.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> scripts/qapi.py | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 910e422..e02fa0b 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -248,6 +248,19 @@ def discriminator_find_enum_define(expr):
>
> return find_enum(discriminator_type)
>
> +def expr_name(expr):
> + if expr.has_key('union'):
> + return expr['union']
> + if expr.has_key('type'):
> + return expr['type']
> + if expr.has_key('enum'):
> + return expr['enum']
> + if expr.has_key('command'):
> + return expr['command']
> + if expr.has_key('event'):
> + return expr['event']
> + return None
> +
> def check_event(expr, expr_info):
> params = expr.get('data')
> if params:
Squash into the next commit?
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH v3 09/14] qapi: add check_type helper function
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 09/14] qapi: add check_type helper function Eric Blake
@ 2014-08-14 10:10 ` Markus Armbruster
2014-08-14 12:42 ` Eric Blake
0 siblings, 1 reply; 29+ messages in thread
From: Markus Armbruster @ 2014-08-14 10:10 UTC (permalink / raw)
To: Eric Blake; +Cc: Luiz Capitulino, Fam Zheng, qemu-devel, wenchaoqemu
Subject suggests you're merely adding a helper function. You're
actually fixing bugs. Something like
qapi: More rigorous checking of type expressions
would be clearer.
While I'm nit-picking your subjects: start with a capital letter after
the subsystem: prefix.
Eric Blake <eblake@redhat.com> writes:
> Add a helper function for use in a later patch that will
> validate that a 'data':... or 'returns':... member of an
> expression evaluates to a valid type.
>
> * scripts/qapi.py (check_type): New function.
> (check_exprs): Use it.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> scripts/qapi.py | 30 ++++++++++++++++++++++++++++++
> tests/qapi-schema/data-array-unknown.err | 1 +
> tests/qapi-schema/data-array-unknown.exit | 2 +-
> tests/qapi-schema/data-array-unknown.out | 3 ---
> tests/qapi-schema/data-int.err | 1 +
> tests/qapi-schema/data-int.exit | 2 +-
> tests/qapi-schema/data-int.out | 3 ---
> tests/qapi-schema/data-unknown.err | 1 +
> tests/qapi-schema/data-unknown.exit | 2 +-
> tests/qapi-schema/data-unknown.out | 3 ---
> tests/qapi-schema/returns-array-bad.err | 1 +
> tests/qapi-schema/returns-array-bad.exit | 2 +-
> tests/qapi-schema/returns-array-bad.out | 3 ---
> tests/qapi-schema/returns-unknown.err | 1 +
> tests/qapi-schema/returns-unknown.exit | 2 +-
> tests/qapi-schema/returns-unknown.out | 3 ---
> 16 files changed, 40 insertions(+), 20 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index e02fa0b..e4d27d6 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -329,6 +329,32 @@ def check_union(expr, expr_info):
> # Todo: add checking for values. Key is checked as above, value can be
> # also checked here, but we need more functions to handle array case.
>
> +def check_type(expr_info, name, data, allow_native = False):
> + if not data:
> + return
> + if isinstance(data, list):
> + if len(data) != 1 or not isinstance(data[0], basestring):
> + raise QAPIExprError(expr_info,
> + "Use of array type in '%s' must contain "
> + "single element type name" % name)
> + data = data[0]
Peeling off the array here means error messages below won't mention it.
Visible in data-array-unknown.err below. But good enough is good
enough.
> +
> + if isinstance(data, basestring):
> + if find_struct(data) or find_enum(data) or find_union(data):
> + return
> + if data == 'str' or data == 'int' or data == 'visitor':
Is this complete? Should we be checking against builtin_types[]?
Pardon my ignorance: where does 'visitor' come from?
> + if allow_native:
> + return
> + raise QAPIExprError(expr_info,
> + "Primitive type '%s' not allowed as data "
> + "of '%s'" % (data, name))
> + raise QAPIExprError(expr_info,
> + "Unknown type '%s' as data of '%s'"
> + % (data, name))
"as data of" suggests the problem is in member 'data', even when it's
actually in member 'returns'. Visible in returns-unknown.err below.
> + elif not isinstance(data, OrderedDict):
> + raise QAPIExprError(expr_info,
> + "Expecting dictionary for data of '%s'" % name)
> +
> def check_exprs(schema):
> for expr_elem in schema.exprs:
> expr = expr_elem['expr']
> @@ -356,6 +382,10 @@ def check_exprs(schema):
> raise QAPIExprError(info,
> "enum '%s' requires an array for 'data'"
> % expr.get('enum'))
> + else:
This is for 'type' and 'command', right?
> + check_type(info, expr_name(expr), members)
> + if expr.has_key('command') and expr.has_key('returns'):
> + check_type(info, expr['command'], expr['returns'], True)
>
> def parse_schema(input_file):
> try:
Looks pretty good, but I didn't check systematically against
qapi-code-gen.txt for correctness and completeness. We can always
improve on top.
> diff --git a/tests/qapi-schema/data-array-unknown.err b/tests/qapi-schema/data-array-unknown.err
> index e69de29..e0433b8 100644
> --- a/tests/qapi-schema/data-array-unknown.err
> +++ b/tests/qapi-schema/data-array-unknown.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/data-array-unknown.json:1: Unknown type 'NoSuchType' as data of 'oops'
> diff --git a/tests/qapi-schema/data-array-unknown.exit b/tests/qapi-schema/data-array-unknown.exit
> index 573541a..d00491f 100644
> --- a/tests/qapi-schema/data-array-unknown.exit
> +++ b/tests/qapi-schema/data-array-unknown.exit
> @@ -1 +1 @@
> -0
> +1
> diff --git a/tests/qapi-schema/data-array-unknown.out b/tests/qapi-schema/data-array-unknown.out
> index c3bc05e..e69de29 100644
> --- a/tests/qapi-schema/data-array-unknown.out
> +++ b/tests/qapi-schema/data-array-unknown.out
> @@ -1,3 +0,0 @@
> -[OrderedDict([('command', 'oops'), ('data', ['NoSuchType'])])]
> -[]
> -[]
> diff --git a/tests/qapi-schema/data-int.err b/tests/qapi-schema/data-int.err
> index e69de29..30bbffc 100644
> --- a/tests/qapi-schema/data-int.err
> +++ b/tests/qapi-schema/data-int.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/data-int.json:1: Primitive type 'int' not allowed as data of 'oops'
> diff --git a/tests/qapi-schema/data-int.exit b/tests/qapi-schema/data-int.exit
> index 573541a..d00491f 100644
> --- a/tests/qapi-schema/data-int.exit
> +++ b/tests/qapi-schema/data-int.exit
> @@ -1 +1 @@
> -0
> +1
> diff --git a/tests/qapi-schema/data-int.out b/tests/qapi-schema/data-int.out
> index e589a4f..e69de29 100644
> --- a/tests/qapi-schema/data-int.out
> +++ b/tests/qapi-schema/data-int.out
> @@ -1,3 +0,0 @@
> -[OrderedDict([('command', 'oops'), ('data', 'int')])]
> -[]
> -[]
> diff --git a/tests/qapi-schema/data-unknown.err b/tests/qapi-schema/data-unknown.err
> index e69de29..f7fd635 100644
> --- a/tests/qapi-schema/data-unknown.err
> +++ b/tests/qapi-schema/data-unknown.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/data-unknown.json:1: Unknown type 'NoSuchType' as data of 'oops'
> diff --git a/tests/qapi-schema/data-unknown.exit b/tests/qapi-schema/data-unknown.exit
> index 573541a..d00491f 100644
> --- a/tests/qapi-schema/data-unknown.exit
> +++ b/tests/qapi-schema/data-unknown.exit
> @@ -1 +1 @@
> -0
> +1
> diff --git a/tests/qapi-schema/data-unknown.out b/tests/qapi-schema/data-unknown.out
> index 2c60726..e69de29 100644
> --- a/tests/qapi-schema/data-unknown.out
> +++ b/tests/qapi-schema/data-unknown.out
> @@ -1,3 +0,0 @@
> -[OrderedDict([('command', 'oops'), ('data', 'NoSuchType')])]
> -[]
> -[]
> diff --git a/tests/qapi-schema/returns-array-bad.err b/tests/qapi-schema/returns-array-bad.err
> index e69de29..d2a216f 100644
> --- a/tests/qapi-schema/returns-array-bad.err
> +++ b/tests/qapi-schema/returns-array-bad.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/returns-array-bad.json:1: Use of array type in 'oops' must contain single element type name
> diff --git a/tests/qapi-schema/returns-array-bad.exit b/tests/qapi-schema/returns-array-bad.exit
> index 573541a..d00491f 100644
> --- a/tests/qapi-schema/returns-array-bad.exit
> +++ b/tests/qapi-schema/returns-array-bad.exit
> @@ -1 +1 @@
> -0
> +1
> diff --git a/tests/qapi-schema/returns-array-bad.out b/tests/qapi-schema/returns-array-bad.out
> index cfc474e..e69de29 100644
> --- a/tests/qapi-schema/returns-array-bad.out
> +++ b/tests/qapi-schema/returns-array-bad.out
> @@ -1,3 +0,0 @@
> -[OrderedDict([('command', 'oops'), ('data', ['str', 'str'])])]
> -[]
> -[]
> diff --git a/tests/qapi-schema/returns-unknown.err b/tests/qapi-schema/returns-unknown.err
> index e69de29..d09fb92 100644
> --- a/tests/qapi-schema/returns-unknown.err
> +++ b/tests/qapi-schema/returns-unknown.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/returns-unknown.json:1: Unknown type 'NoSuchType' as data of 'oops'
> diff --git a/tests/qapi-schema/returns-unknown.exit b/tests/qapi-schema/returns-unknown.exit
> index 573541a..d00491f 100644
> --- a/tests/qapi-schema/returns-unknown.exit
> +++ b/tests/qapi-schema/returns-unknown.exit
> @@ -1 +1 @@
> -0
> +1
> diff --git a/tests/qapi-schema/returns-unknown.out b/tests/qapi-schema/returns-unknown.out
> index a482c83..e69de29 100644
> --- a/tests/qapi-schema/returns-unknown.out
> +++ b/tests/qapi-schema/returns-unknown.out
> @@ -1,3 +0,0 @@
> -[OrderedDict([('command', 'oops'), ('returns', 'NoSuchType')])]
> -[]
> -[]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH v3 12/14] qapi: drop inline subtype in query-version
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 12/14] qapi: drop inline subtype in query-version Eric Blake
@ 2014-08-14 11:45 ` Markus Armbruster
0 siblings, 0 replies; 29+ messages in thread
From: Markus Armbruster @ 2014-08-14 11:45 UTC (permalink / raw)
To: Eric Blake; +Cc: Luiz Capitulino, Fam Zheng, qemu-devel, wenchaoqemu
Eric Blake <eblake@redhat.com> writes:
> A future patch will be using a 'name':{dictionary} entry in the
> QAPI schema to specify a default value for an optional argument;
> but existing use of inline substructs conflicts with that goal.
> This patch fixes one of only two commands relying on nested
> subtypes, by breaking the nesting into an explicit type.
>
> Prefer the safer g_new0() while making the conversion.
>
> * qapi/common.json (VersionInfo): Split inline data...
> (VersionTriple): ...into new type.
> * qmp.c (qmp_query_version): Update caller.
> * hmp.c (hmp_info_version): Likewise.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> hmp.c | 2 +-
> qapi/common.json | 26 +++++++++++++++++++-------
> qmp.c | 9 +++++----
> 3 files changed, 25 insertions(+), 12 deletions(-)
>
> diff --git a/hmp.c b/hmp.c
> index 4d1838e..e637fbc 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -55,7 +55,7 @@ void hmp_info_version(Monitor *mon, const QDict *qdict)
> info = qmp_query_version(NULL);
>
> monitor_printf(mon, "%" PRId64 ".%" PRId64 ".%" PRId64 "%s\n",
> - info->qemu.major, info->qemu.minor, info->qemu.micro,
> + info->qemu->major, info->qemu->minor, info->qemu->micro,
> info->package);
>
> qapi_free_VersionInfo(info);
This demonstrates the difference between anonymous and named complex
member types: anonymous means unboxed, named means boxed. Already
visible in the previous patch, but it's easier to see here (less noise).
I'm no friend of excessive boxing in C, and QAPI code generator is too
boxing-happy for my taste already. However, I like tying boxed
vs. unboxed to named vs. anonymous even less.
> diff --git a/qapi/common.json b/qapi/common.json
> index 4e9a21f..d007095 100644
> --- a/qapi/common.json
> +++ b/qapi/common.json
> @@ -29,15 +29,28 @@
> 'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap' ] }
>
> ##
> +# @VersionTriple
> +#
> +# A three-part version number.
> +#
> +# @qemu.major: The major version number.
> +#
> +# @qemu.minor: The minor version number.
> +#
> +# @qemu.micro: The micro version number.
> +#
> +# Since: 2.2
> +##
> +{ 'type': 'VersionTriple',
> + 'data': {'major': 'int', 'minor': 'int', 'micro': 'int'} }
> +
> +
> +##
> # @VersionInfo:
> #
> # A description of QEMU's version.
> #
> -# @qemu.major: The major version of QEMU
> -#
> -# @qemu.minor: The minor version of QEMU
> -#
> -# @qemu.micro: The micro version of QEMU. By current convention, a micro
> +# @qemu: The version of QEMU. By current convention, a micro
> # version of 50 signifies a development branch. A micro version
> # greater than or equal to 90 signifies a release candidate for
> # the next minor version. A micro version of less than 50
> @@ -51,8 +64,7 @@
> # Since: 0.14.0
> ##
> { 'type': 'VersionInfo',
> - 'data': {'qemu': {'major': 'int', 'minor': 'int', 'micro': 'int'},
> - 'package': 'str'} }
> + 'data': {'qemu': 'VersionTriple', 'package': 'str'} }
>
> ##
> # @query-version:
> diff --git a/qmp.c b/qmp.c
> index 0d2553a..35e6f7a 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -45,15 +45,16 @@ NameInfo *qmp_query_name(Error **errp)
>
> VersionInfo *qmp_query_version(Error **errp)
> {
> - VersionInfo *info = g_malloc0(sizeof(*info));
> + VersionInfo *info = g_new0(VersionInfo, 1);
Unrelated improvement. I figure you want it for consistency with the
g_new0() you add. Okay.
> const char *version = QEMU_VERSION;
> char *tmp;
>
> - info->qemu.major = strtol(version, &tmp, 10);
> + info->qemu = g_new0(VersionTriple, 1);
> + info->qemu->major = strtol(version, &tmp, 10);
> tmp++;
> - info->qemu.minor = strtol(tmp, &tmp, 10);
> + info->qemu->minor = strtol(tmp, &tmp, 10);
> tmp++;
> - info->qemu.micro = strtol(tmp, &tmp, 10);
> + info->qemu->micro = strtol(tmp, &tmp, 10);
> info->package = g_strdup(QEMU_PKGVERSION);
>
> return info;
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs
2014-08-06 1:14 [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs Eric Blake
` (14 preceding siblings ...)
2014-08-12 12:47 ` [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs Eric Blake
@ 2014-08-14 12:00 ` Markus Armbruster
2014-08-15 18:12 ` Luiz Capitulino
16 siblings, 0 replies; 29+ messages in thread
From: Markus Armbruster @ 2014-08-14 12:00 UTC (permalink / raw)
To: Eric Blake; +Cc: Luiz Capitulino, Fam Zheng, qemu-devel, wenchaoqemu
Eric Blake <eblake@redhat.com> writes:
> According to this email:
> https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg00708.html
> we want to repurpose 'data': { 'name': {dict...} } in qapi files
> for future use of designating default values of optional parameters.
> But to do that, we must first nuke existing use of that syntax for
> declaring nested structs. Enhancing the testsuite while at it
> never hurts.
I had a few remarks here and there, but overall, this is solid work.
Thanks for tackling this!
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH v3 03/14] qapi: add some enum tests
2014-08-14 9:23 ` Markus Armbruster
@ 2014-08-14 12:21 ` Eric Blake
0 siblings, 0 replies; 29+ messages in thread
From: Eric Blake @ 2014-08-14 12:21 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Luiz Capitulino, Fam Zheng, qemu-devel, wenchaoqemu
[-- Attachment #1: Type: text/plain, Size: 1055 bytes --]
On 08/14/2014 03:23 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
>
>> Demonstrate that the qapi generator doesn't deal well with enums
>> that aren't up to par. Later patches will update the expected
>> results as the generator is made stricter.
>>
>> * tests/qapi-schema/enum-empty.*: New files.
>> * tests/qapi-schema/enum-missing-data.*: Likewise.
>> * tests/qapi-schema/enum-wrong-data.*: Likewise.
>> * tests/Makefile (check-qapi-schema-y): Run them.
>>
>> +++ b/tests/qapi-schema/enum-wrong-data.out
>> @@ -0,0 +1,3 @@
>> +[OrderedDict([('enum', 'MyEnum'), ('data', OrderedDict([('value', 'str')]))])]
>> +[{'enum_name': 'MyEnum', 'enum_values': OrderedDict([('value', 'str')])}]
>> +[]
>
> For tests demonstrating incorrect behavior, a comment describing the
> expected behavior is always welcome.
Okay, I'll add comments to the .json files that expose weaknesses, when
submitting v4
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH v3 06/14] qapi: require valid expressions
2014-08-14 9:38 ` Markus Armbruster
@ 2014-08-14 12:26 ` Eric Blake
0 siblings, 0 replies; 29+ messages in thread
From: Eric Blake @ 2014-08-14 12:26 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Luiz Capitulino, Fam Zheng, qemu-devel, wenchaoqemu
[-- Attachment #1: Type: text/plain, Size: 2043 bytes --]
On 08/14/2014 03:38 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
>
>> The previous patch demonstrated that the generator could get
>> confused if an expression had conflicting meta-types, and
>> silently ignored expressions that lacked a known meta-type.
>> Fix both cases to give a sane error message.
>>
>> * scripts/qapi.py (check_exprs): Require a valid meta-type for
>> every expression.
>> * tests/qapi-schema/indented-expr.*: Use valid types.
>> * tests/qapi-schema/missing-type.*: Update expected output.
>> * tests/qapi-schema/double-type.*: Likewise.
>
> Conventional git commit message followed by GNU change log. Unusual in
> QEMU. Same elsewhere.
Habits die hard :)
>
> Explanation of the change to indented-expr.json hides in the change log
> part, where I promptly missed it :)
Okay, I'll make the subject line capitalized (which is more popular,
even if not universal) and ditch the GNU style suffix (inlining any meat
into the paragraph form) on my respin.
>> + # 'include' has already been flattened; at this point, all exprs
>> + # should have one of the remaining keys
>> + keys = (expr.has_key('enum') + expr.has_key('union') +
>> + expr.has_key('type') + expr.has_key('command') +
>> + expr.has_key('event'))
>> + if keys < 1:
>> + raise QAPIExprError(info,
>> + "Missing expression meta-type")
>> + if keys > 1:
>> + raise QAPIExprError(info,
>> + "Conflicting expression meta-types")
>> +
>> if expr.has_key('union'):
>> check_union(expr, info)
>> if expr.has_key('event'):
>
> Not the friendliest error messages, but they'll do.
I'm open to bike-shedding suggestions :) But in the absence of a
suggestion, I'll keep this part unchanged in v4.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH v3 07/14] qapi: add some type check tests
2014-08-14 9:47 ` Markus Armbruster
@ 2014-08-14 12:26 ` Eric Blake
0 siblings, 0 replies; 29+ messages in thread
From: Eric Blake @ 2014-08-14 12:26 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Luiz Capitulino, Fam Zheng, qemu-devel, wenchaoqemu
[-- Attachment #1: Type: text/plain, Size: 1089 bytes --]
On 08/14/2014 03:47 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
>
>> Demonstrate that the qapi generator silently parses confusing
>> types, which may cause other errors later on. Later patches
>> will update the expected results as the generator is made stricter.
>>
>> * tests/qapi-schema/data-array-empty.*: New files.
>> * tests/qapi-schema/data-array-unknown.*: Likewise.
>> * tests/qapi-schema/data-unknown.*: Likewise.
>> * tests/qapi-schema/data-int.*: Likewise.
>> * tests/qapi-schema/returns-unknown.*: Likewise.
>> * tests/qapi-schema/returns-int.*: Likewise.
>> * tests/qapi-schema/returns-array-bad.*: Likewise.
>> * tests/Makefile (check-qapi-schema-y): Run them.
>>
>> +++ b/tests/qapi-schema/returns-array-bad.json
>> @@ -0,0 +1 @@
>> +{ 'command': 'oops', 'data': [ 'str', 'str' ] }
>
> The file name suggests you want to test 'returns' rather than 'data' here.
D'oh. Definite reason to respin the series.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH v3 09/14] qapi: add check_type helper function
2014-08-14 10:10 ` Markus Armbruster
@ 2014-08-14 12:42 ` Eric Blake
2014-08-14 16:10 ` Markus Armbruster
0 siblings, 1 reply; 29+ messages in thread
From: Eric Blake @ 2014-08-14 12:42 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Luiz Capitulino, Fam Zheng, qemu-devel, wenchaoqemu
[-- Attachment #1: Type: text/plain, Size: 4862 bytes --]
On 08/14/2014 04:10 AM, Markus Armbruster wrote:
> Subject suggests you're merely adding a helper function. You're
> actually fixing bugs. Something like
>
> qapi: More rigorous checking of type expressions
>
> would be clearer.
Along with squashing in 8/14, you are correct about this fixing bugs
(serves me right for refactoring, then realizing that we needed more
tests, then fixing the tests without checking that the subject line
stayed appropriate).
>> +++ b/scripts/qapi.py
>> @@ -329,6 +329,32 @@ def check_union(expr, expr_info):
>> # Todo: add checking for values. Key is checked as above, value can be
>> # also checked here, but we need more functions to handle array case.
>>
>> +def check_type(expr_info, name, data, allow_native = False):
>> + if not data:
>> + return
>> + if isinstance(data, list):
>> + if len(data) != 1 or not isinstance(data[0], basestring):
>> + raise QAPIExprError(expr_info,
>> + "Use of array type in '%s' must contain "
>> + "single element type name" % name)
>> + data = data[0]
>
> Peeling off the array here means error messages below won't mention it.
> Visible in data-array-unknown.err below. But good enough is good
> enough.
>
>> +
>> + if isinstance(data, basestring):
>> + if find_struct(data) or find_enum(data) or find_union(data):
>> + return
>> + if data == 'str' or data == 'int' or data == 'visitor':
>
> Is this complete? Should we be checking against builtin_types[]?
It was complete insofar that it passes with the current
qapi-schema.json. Checking builtin_types[] would indeed be nicer (I
didn't know that existed).
>
> Pardon my ignorance: where does 'visitor' come from?
qom-set, qom-get. Nasty commands that aren't typesafe, and which
explicitly use 'gen':'no' to abuse the qapi system by bypassing the code
generator while still exposing a backdoor command that can break the rules.
netdev_add ('*props':'**') and object-add ('*props':'dict') are the only
other 'gen':'no' clients; I'm not sure why they didn't cause the parser
to barf the way 'visitor' did. Maybe my approach should instead be to
unify all four 'gen':'no' expressions to use the same syntax of "**" to
represent the fact that the QMP code is not type-safe, as well as
actually documenting this (ab)use of ** (the fact that none of the
documentation mentions 'gen' is telling). Looks like I've just added a
pre-req patch into my v4 :)
>
>> + if allow_native:
>> + return
>> + raise QAPIExprError(expr_info,
>> + "Primitive type '%s' not allowed as data "
>> + "of '%s'" % (data, name))
>> + raise QAPIExprError(expr_info,
>> + "Unknown type '%s' as data of '%s'"
>> + % (data, name))
>
> "as data of" suggests the problem is in member 'data', even when it's
> actually in member 'returns'. Visible in returns-unknown.err below.
The function is declared as:
>> +def check_type(expr_info, name, data, allow_native = False):
but allow_native is True only for the 'returns' case. Maybe I should
just repurpose that parameter for what it really is - 'data' vs.
'returns', and use it for improving the error message in addition to
deciding whether native types are allowed.
>
>> + elif not isinstance(data, OrderedDict):
>> + raise QAPIExprError(expr_info,
>> + "Expecting dictionary for data of '%s'" % name)
>> +
>> def check_exprs(schema):
>> for expr_elem in schema.exprs:
>> expr = expr_elem['expr']
>> @@ -356,6 +382,10 @@ def check_exprs(schema):
>> raise QAPIExprError(info,
>> "enum '%s' requires an array for 'data'"
>> % expr.get('enum'))
>> + else:
>
> This is for 'type' and 'command', right?
'type', 'union', 'event', and 'command' ('data' is a dict for those
four, and an array for the fifth meta-type of 'enum'). I'll add a comment.
>
>> + check_type(info, expr_name(expr), members)
>> + if expr.has_key('command') and expr.has_key('returns'):
>> + check_type(info, expr['command'], expr['returns'], True)
>>
>> def parse_schema(input_file):
>> try:
>
> Looks pretty good, but I didn't check systematically against
> qapi-code-gen.txt for correctness and completeness. We can always
> improve on top.
I'll double-check things as well, when posting v4.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH v3 09/14] qapi: add check_type helper function
2014-08-14 12:42 ` Eric Blake
@ 2014-08-14 16:10 ` Markus Armbruster
0 siblings, 0 replies; 29+ messages in thread
From: Markus Armbruster @ 2014-08-14 16:10 UTC (permalink / raw)
To: Eric Blake; +Cc: Fam Zheng, qemu-devel, wenchaoqemu, Luiz Capitulino
Eric Blake <eblake@redhat.com> writes:
> On 08/14/2014 04:10 AM, Markus Armbruster wrote:
>> Subject suggests you're merely adding a helper function. You're
>> actually fixing bugs. Something like
>>
>> qapi: More rigorous checking of type expressions
>>
>> would be clearer.
>
> Along with squashing in 8/14, you are correct about this fixing bugs
> (serves me right for refactoring, then realizing that we needed more
> tests, then fixing the tests without checking that the subject line
> stayed appropriate).
>
>>> +++ b/scripts/qapi.py
>>> @@ -329,6 +329,32 @@ def check_union(expr, expr_info):
>>> # Todo: add checking for values. Key is checked as above,
>>> value can be
>>> # also checked here, but we need more functions to handle
>>> array case.
>>>
>>> +def check_type(expr_info, name, data, allow_native = False):
>>> + if not data:
>>> + return
>>> + if isinstance(data, list):
>>> + if len(data) != 1 or not isinstance(data[0], basestring):
>>> + raise QAPIExprError(expr_info,
>>> + "Use of array type in '%s' must contain "
>>> + "single element type name" % name)
>>> + data = data[0]
>>
>> Peeling off the array here means error messages below won't mention it.
>> Visible in data-array-unknown.err below. But good enough is good
>> enough.
>>
>>> +
>>> + if isinstance(data, basestring):
>>> + if find_struct(data) or find_enum(data) or find_union(data):
>>> + return
>>> + if data == 'str' or data == 'int' or data == 'visitor':
>>
>> Is this complete? Should we be checking against builtin_types[]?
>
> It was complete insofar that it passes with the current
> qapi-schema.json. Checking builtin_types[] would indeed be nicer (I
> didn't know that existed).
>
>>
>> Pardon my ignorance: where does 'visitor' come from?
>
> qom-set, qom-get. Nasty commands that aren't typesafe, and which
> explicitly use 'gen':'no' to abuse the qapi system by bypassing the code
> generator while still exposing a backdoor command that can break the rules.
>
> netdev_add ('*props':'**') and object-add ('*props':'dict') are the only
> other 'gen':'no' clients; I'm not sure why they didn't cause the parser
> to barf the way 'visitor' did. Maybe my approach should instead be to
> unify all four 'gen':'no' expressions to use the same syntax of "**" to
> represent the fact that the QMP code is not type-safe, as well as
> actually documenting this (ab)use of ** (the fact that none of the
> documentation mentions 'gen' is telling). Looks like I've just added a
> pre-req patch into my v4 :)
Documentation for 'gen': 'no', '**', and 'visitor' is most welcome.
I wonder why 'no', and not simply false. Hmm, looks like the code
generator ignores the value. 'gen': 'sure, why not, if you feel like
it'.
Should 'visitor' be in builtin_types[]?
>>> + if allow_native:
>>> + return
>>> + raise QAPIExprError(expr_info,
>>> + "Primitive type '%s' not allowed as data "
>>> + "of '%s'" % (data, name))
>>> + raise QAPIExprError(expr_info,
>>> + "Unknown type '%s' as data of '%s'"
>>> + % (data, name))
>>
>> "as data of" suggests the problem is in member 'data', even when it's
>> actually in member 'returns'. Visible in returns-unknown.err below.
>
> The function is declared as:
>
>>> +def check_type(expr_info, name, data, allow_native = False):
>
> but allow_native is True only for the 'returns' case. Maybe I should
> just repurpose that parameter for what it really is - 'data' vs.
> 'returns', and use it for improving the error message in addition to
> deciding whether native types are allowed.
Yeah.
>>> + elif not isinstance(data, OrderedDict):
>>> + raise QAPIExprError(expr_info,
>>> + "Expecting dictionary for data of '%s'" % name)
>>> +
>>> def check_exprs(schema):
>>> for expr_elem in schema.exprs:
>>> expr = expr_elem['expr']
>>> @@ -356,6 +382,10 @@ def check_exprs(schema):
>>> raise QAPIExprError(info,
>>> "enum '%s' requires an array for 'data'"
>>> % expr.get('enum'))
>>> + else:
>>
>> This is for 'type' and 'command', right?
>
> 'type', 'union', 'event', and 'command' ('data' is a dict for those
> four, and an array for the fifth meta-type of 'enum'). I'll add a comment.
You're right. I misread the sequence of if statements as one big
if... elif... elif... else statement.
A blank line before the if... else would probably do.
>>
>>> + check_type(info, expr_name(expr), members)
>>> + if expr.has_key('command') and expr.has_key('returns'):
>>> + check_type(info, expr['command'], expr['returns'], True)
>>>
>>> def parse_schema(input_file):
>>> try:
>>
>> Looks pretty good, but I didn't check systematically against
>> qapi-code-gen.txt for correctness and completeness. We can always
>> improve on top.
>
> I'll double-check things as well, when posting v4.
Thanks!
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs
2014-08-06 1:14 [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs Eric Blake
` (15 preceding siblings ...)
2014-08-14 12:00 ` Markus Armbruster
@ 2014-08-15 18:12 ` Luiz Capitulino
16 siblings, 0 replies; 29+ messages in thread
From: Luiz Capitulino @ 2014-08-15 18:12 UTC (permalink / raw)
To: Eric Blake; +Cc: Fam Zheng, qemu-devel, wenchaoqemu, Markus Armbruster
On Tue, 5 Aug 2014 19:14:19 -0600
Eric Blake <eblake@redhat.com> wrote:
> According to this email:
> https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg00708.html
> we want to repurpose 'data': { 'name': {dict...} } in qapi files
> for future use of designating default values of optional parameters.
> But to do that, we must first nuke existing use of that syntax for
> declaring nested structs. Enhancing the testsuite while at it
> never hurts.
I've skimmed over the series (as Markus is on top of it anyways) and it
looks great to me. Waiting for v4 :)
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2014-08-15 18:12 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-06 1:14 [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs Eric Blake
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 01/14] qapi: consistent whitespace in tests/Makefile Eric Blake
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 02/14] qapi: ignore files created during make check Eric Blake
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 03/14] qapi: add some enum tests Eric Blake
2014-08-14 9:23 ` Markus Armbruster
2014-08-14 12:21 ` Eric Blake
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 04/14] qapi: better error message for bad enum Eric Blake
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 05/14] qapi: add some expr tests Eric Blake
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 06/14] qapi: require valid expressions Eric Blake
2014-08-14 9:38 ` Markus Armbruster
2014-08-14 12:26 ` Eric Blake
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 07/14] qapi: add some type check tests Eric Blake
2014-08-14 9:47 ` Markus Armbruster
2014-08-14 12:26 ` Eric Blake
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 08/14] qapi: add expr_name() helper Eric Blake
2014-08-14 9:49 ` Markus Armbruster
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 09/14] qapi: add check_type helper function Eric Blake
2014-08-14 10:10 ` Markus Armbruster
2014-08-14 12:42 ` Eric Blake
2014-08-14 16:10 ` Markus Armbruster
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 10/14] qapi: merge UserDefTwo and UserDefNested in tests Eric Blake
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 11/14] qapi: drop tests for inline subtypes Eric Blake
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 12/14] qapi: drop inline subtype in query-version Eric Blake
2014-08-14 11:45 ` Markus Armbruster
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 13/14] qapi: drop inline subtype in query-pci Eric Blake
2014-08-06 1:14 ` [Qemu-devel] [PATCH v3 14/14] qapi: drop support for inline subtypes Eric Blake
2014-08-12 12:47 ` [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs Eric Blake
2014-08-14 12:00 ` Markus Armbruster
2014-08-15 18:12 ` Luiz Capitulino
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.