* [Qemu-devel] [PULL v2 01/20] docs: Move files from docs/qmp/ to docs/
2015-10-12 18:19 [Qemu-devel] [PULL v2 00/20] QAPI patches Markus Armbruster
@ 2015-10-12 18:19 ` Markus Armbruster
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 02/20] MAINTAINERS: Specify QObject include and test files Markus Armbruster
` (19 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2015-10-12 18:19 UTC (permalink / raw)
To: qemu-devel
Giving QMP its own subdirectory in docs/ is hardly worthwhile when we
have just four files, and one of them isn't even in the subdirectory.
Move the files from docs/qmp/ to docs/, renaming docs/qmp/README to
docs/qmp-intro.
Update MAINTAINERS. The new pattern also captures the fourth file
docs/writing-qmp-commands.txt.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1443111117-29831-2-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
---
MAINTAINERS | 2 +-
docs/{qmp => }/qmp-events.txt | 0
docs/{qmp/README => qmp-intro.txt} | 0
docs/{qmp => }/qmp-spec.txt | 0
4 files changed, 1 insertion(+), 1 deletion(-)
rename docs/{qmp => }/qmp-events.txt (100%)
rename docs/{qmp/README => qmp-intro.txt} (100%)
rename docs/{qmp => }/qmp-spec.txt (100%)
diff --git a/MAINTAINERS b/MAINTAINERS
index 9bde832..79ffe88 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1095,7 +1095,7 @@ S: Supported
F: qmp.c
F: monitor.c
F: qmp-commands.hx
-F: docs/qmp/
+F: docs/*qmp-*
F: scripts/qmp/
T: git git://repo.or.cz/qemu/armbru.git qapi-next
diff --git a/docs/qmp/qmp-events.txt b/docs/qmp-events.txt
similarity index 100%
rename from docs/qmp/qmp-events.txt
rename to docs/qmp-events.txt
diff --git a/docs/qmp/README b/docs/qmp-intro.txt
similarity index 100%
rename from docs/qmp/README
rename to docs/qmp-intro.txt
diff --git a/docs/qmp/qmp-spec.txt b/docs/qmp-spec.txt
similarity index 100%
rename from docs/qmp/qmp-spec.txt
rename to docs/qmp-spec.txt
--
2.4.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PULL v2 02/20] MAINTAINERS: Specify QObject include and test files
2015-10-12 18:19 [Qemu-devel] [PULL v2 00/20] QAPI patches Markus Armbruster
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 01/20] docs: Move files from docs/qmp/ to docs/ Markus Armbruster
@ 2015-10-12 18:19 ` Markus Armbruster
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 03/20] MAINTAINERS: Specify QAPI " Markus Armbruster
` (18 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2015-10-12 18:19 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1443111117-29831-3-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
---
MAINTAINERS | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 79ffe88..f9a46ec 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1071,6 +1071,14 @@ QObject
M: Luiz Capitulino <lcapitulino@redhat.com>
S: Maintained
F: qobject/
+F: include/qapi/qmp/
+X: include/qapi/qmp/dispatch.h
+F: tests/check-qdict.c
+F: tests/check-qfloat.c
+F: tests/check-qint.c
+F: tests/check-qjson.c
+F: tests/check-qlist.c
+F: tests/check-qstring.c
T: git git://repo.or.cz/qemu/qmp-unstable.git queue/qmp
QEMU Guest Agent
--
2.4.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PULL v2 03/20] MAINTAINERS: Specify QAPI include and test files
2015-10-12 18:19 [Qemu-devel] [PULL v2 00/20] QAPI patches Markus Armbruster
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 01/20] docs: Move files from docs/qmp/ to docs/ Markus Armbruster
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 02/20] MAINTAINERS: Specify QObject include and test files Markus Armbruster
@ 2015-10-12 18:19 ` Markus Armbruster
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 04/20] qapi: Sort qapi-schema tests Markus Armbruster
` (17 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2015-10-12 18:19 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1443111117-29831-4-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
---
MAINTAINERS | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index f9a46ec..9bd2b8f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1054,7 +1054,12 @@ M: Michael Roth <mdroth@linux.vnet.ibm.com>
S: Supported
F: qapi/
X: qapi/*.json
+F: include/qapi/
+X: include/qapi/qmp/
+F: include/qapi/qmp/dispatch.h
F: tests/qapi-schema/
+F: tests/test-*-visitor.c
+F: tests/test-qmp-*.c
F: scripts/qapi*
F: docs/qapi*
T: git git://repo.or.cz/qemu/armbru.git qapi-next
--
2.4.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PULL v2 04/20] qapi: Sort qapi-schema tests
2015-10-12 18:19 [Qemu-devel] [PULL v2 00/20] QAPI patches Markus Armbruster
` (2 preceding siblings ...)
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 03/20] MAINTAINERS: Specify QAPI " Markus Armbruster
@ 2015-10-12 18:19 ` Markus Armbruster
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 05/20] qapi: Improve 'include' error message Markus Armbruster
` (16 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2015-10-12 18:19 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
Recent changes to qapi have provided quite a bit of churn in
the makefile, because we are inconsistent on what order test
names appear in, and on whether to re-wrap the list of tests or
just add arbitrary line lengths. Writing the list in a sorted
fashion, one test per line, will make future patches easier
to see what tests are being added or removed by a patch.
Although it is tempting to use $(wildcard qapi-schema/*.json)
for a more compact listing, such an approach would risk picking
up leftover garbage .json files in the directory; so keeping
the list explicit is safer for ensuring reproducible tarballs
and test results.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-2-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/Makefile | 160 ++++++++++++++++++++++++++++++++++++++++-----------------
1 file changed, 114 insertions(+), 46 deletions(-)
diff --git a/tests/Makefile b/tests/Makefile
index dbd32a6..164dea3 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -224,52 +224,120 @@ check-qtest-xtensaeb-y = $(check-qtest-xtensa-y)
check-qtest-generic-y += tests/qom-test$(EXESUF)
-check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
- comments.json empty.json enum-empty.json enum-missing-data.json \
- enum-wrong-data.json enum-int-member.json enum-dict-member.json \
- enum-clash-member.json enum-max-member.json enum-union-clash.json \
- enum-bad-name.json enum-bad-prefix.json \
- funny-char.json indented-expr.json \
- missing-type.json bad-ident.json ident-with-escape.json \
- escape-outside-string.json unknown-escape.json \
- escape-too-short.json escape-too-big.json unicode-str.json \
- double-type.json bad-base.json bad-type-bool.json bad-type-int.json \
- bad-type-dict.json double-data.json unknown-expr-key.json \
- redefined-type.json redefined-command.json redefined-builtin.json \
- redefined-event.json command-int.json bad-data.json event-max.json \
- type-bypass-bad-gen.json \
- args-invalid.json \
- args-array-empty.json args-array-unknown.json args-int.json \
- args-unknown.json args-member-unknown.json args-member-array.json \
- args-member-array-bad.json args-alternate.json args-union.json \
- args-any.json \
- returns-array-bad.json returns-int.json returns-dict.json \
- returns-unknown.json returns-alternate.json returns-whitelist.json \
- missing-colon.json missing-comma-list.json missing-comma-object.json \
- struct-data-invalid.json struct-member-invalid.json \
- nested-struct-data.json non-objects.json \
- qapi-schema-test.json quoted-structural-chars.json \
- leading-comma-list.json leading-comma-object.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 union-bad-branch.json \
- union-optional-branch.json union-unknown.json union-max.json \
- flat-union-optional-discriminator.json flat-union-no-base.json \
- flat-union-invalid-discriminator.json flat-union-inline.json \
- flat-union-invalid-branch-key.json flat-union-reverse-define.json \
- flat-union-string-discriminator.json union-base-no-discriminator.json \
- flat-union-bad-discriminator.json flat-union-bad-base.json \
- flat-union-base-any.json \
- flat-union-array-branch.json flat-union-int-branch.json \
- flat-union-base-union.json flat-union-branch-clash.json \
- alternate-nested.json alternate-unknown.json alternate-clash.json \
- alternate-good.json alternate-base.json alternate-array.json \
- alternate-conflict-string.json alternate-conflict-dict.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 event-case.json \
- struct-base-clash.json struct-base-clash-deep.json )
+qapi-schema += alternate-array.json
+qapi-schema += alternate-base.json
+qapi-schema += alternate-clash.json
+qapi-schema += alternate-conflict-dict.json
+qapi-schema += alternate-conflict-string.json
+qapi-schema += alternate-good.json
+qapi-schema += alternate-nested.json
+qapi-schema += alternate-unknown.json
+qapi-schema += args-alternate.json
+qapi-schema += args-any.json
+qapi-schema += args-array-empty.json
+qapi-schema += args-array-unknown.json
+qapi-schema += args-int.json
+qapi-schema += args-invalid.json
+qapi-schema += args-member-array-bad.json
+qapi-schema += args-member-array.json
+qapi-schema += args-member-unknown.json
+qapi-schema += args-union.json
+qapi-schema += args-unknown.json
+qapi-schema += bad-base.json
+qapi-schema += bad-data.json
+qapi-schema += bad-ident.json
+qapi-schema += bad-type-bool.json
+qapi-schema += bad-type-dict.json
+qapi-schema += bad-type-int.json
+qapi-schema += command-int.json
+qapi-schema += comments.json
+qapi-schema += double-data.json
+qapi-schema += double-type.json
+qapi-schema += duplicate-key.json
+qapi-schema += empty.json
+qapi-schema += enum-bad-name.json
+qapi-schema += enum-bad-prefix.json
+qapi-schema += enum-clash-member.json
+qapi-schema += enum-dict-member.json
+qapi-schema += enum-empty.json
+qapi-schema += enum-int-member.json
+qapi-schema += enum-max-member.json
+qapi-schema += enum-missing-data.json
+qapi-schema += enum-union-clash.json
+qapi-schema += enum-wrong-data.json
+qapi-schema += escape-outside-string.json
+qapi-schema += escape-too-big.json
+qapi-schema += escape-too-short.json
+qapi-schema += event-case.json
+qapi-schema += event-max.json
+qapi-schema += event-nest-struct.json
+qapi-schema += flat-union-array-branch.json
+qapi-schema += flat-union-bad-base.json
+qapi-schema += flat-union-bad-discriminator.json
+qapi-schema += flat-union-base-any.json
+qapi-schema += flat-union-base-union.json
+qapi-schema += flat-union-branch-clash.json
+qapi-schema += flat-union-inline.json
+qapi-schema += flat-union-int-branch.json
+qapi-schema += flat-union-invalid-branch-key.json
+qapi-schema += flat-union-invalid-discriminator.json
+qapi-schema += flat-union-no-base.json
+qapi-schema += flat-union-optional-discriminator.json
+qapi-schema += flat-union-reverse-define.json
+qapi-schema += flat-union-string-discriminator.json
+qapi-schema += funny-char.json
+qapi-schema += ident-with-escape.json
+qapi-schema += include-before-err.json
+qapi-schema += include-cycle.json
+qapi-schema += include-format-err.json
+qapi-schema += include-nested-err.json
+qapi-schema += include-no-file.json
+qapi-schema += include-non-file.json
+qapi-schema += include-relpath.json
+qapi-schema += include-repetition.json
+qapi-schema += include-self-cycle.json
+qapi-schema += include-simple.json
+qapi-schema += indented-expr.json
+qapi-schema += leading-comma-list.json
+qapi-schema += leading-comma-object.json
+qapi-schema += missing-colon.json
+qapi-schema += missing-comma-list.json
+qapi-schema += missing-comma-object.json
+qapi-schema += missing-type.json
+qapi-schema += nested-struct-data.json
+qapi-schema += non-objects.json
+qapi-schema += qapi-schema-test.json
+qapi-schema += quoted-structural-chars.json
+qapi-schema += redefined-builtin.json
+qapi-schema += redefined-command.json
+qapi-schema += redefined-event.json
+qapi-schema += redefined-type.json
+qapi-schema += returns-alternate.json
+qapi-schema += returns-array-bad.json
+qapi-schema += returns-dict.json
+qapi-schema += returns-int.json
+qapi-schema += returns-unknown.json
+qapi-schema += returns-whitelist.json
+qapi-schema += struct-base-clash-deep.json
+qapi-schema += struct-base-clash.json
+qapi-schema += struct-data-invalid.json
+qapi-schema += struct-member-invalid.json
+qapi-schema += trailing-comma-list.json
+qapi-schema += trailing-comma-object.json
+qapi-schema += type-bypass-bad-gen.json
+qapi-schema += unclosed-list.json
+qapi-schema += unclosed-object.json
+qapi-schema += unclosed-string.json
+qapi-schema += unicode-str.json
+qapi-schema += union-bad-branch.json
+qapi-schema += union-base-no-discriminator.json
+qapi-schema += union-invalid-base.json
+qapi-schema += union-max.json
+qapi-schema += union-optional-branch.json
+qapi-schema += union-unknown.json
+qapi-schema += unknown-escape.json
+qapi-schema += unknown-expr-key.json
+check-qapi-schema-y := $(addprefix tests/qapi-schema/, $(qapi-schema))
GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h \
tests/test-qmp-commands.h tests/test-qapi-event.h \
--
2.4.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PULL v2 05/20] qapi: Improve 'include' error message
2015-10-12 18:19 [Qemu-devel] [PULL v2 00/20] QAPI patches Markus Armbruster
` (3 preceding siblings ...)
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 04/20] qapi: Sort qapi-schema tests Markus Armbruster
@ 2015-10-12 18:19 ` Markus Armbruster
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 06/20] qapi: Invoke exception superclass initializer Markus Armbruster
` (15 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2015-10-12 18:19 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
Use of '"...%s" % include' to print non-strings can lead to
ugly messages, such as this (if the .json change is applied
without the qapi.py change):
Expected a file name (string), got: OrderedDict()
Better is to just omit the actual non-string value in the
message.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-3-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/qapi.py | 3 +--
tests/qapi-schema/include-non-file.err | 2 +-
tests/qapi-schema/include-non-file.json | 2 +-
3 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 06478bb..362e007 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -132,8 +132,7 @@ class QAPISchemaParser(object):
include = expr["include"]
if not isinstance(include, str):
raise QAPIExprError(expr_info,
- 'Expected a file name (string), got: %s'
- % include)
+ "Value of 'include' must be a string")
incl_abs_fname = os.path.join(os.path.dirname(abs_fname),
include)
# catch inclusion cycle
diff --git a/tests/qapi-schema/include-non-file.err b/tests/qapi-schema/include-non-file.err
index 9658c78..faae1ea 100644
--- a/tests/qapi-schema/include-non-file.err
+++ b/tests/qapi-schema/include-non-file.err
@@ -1 +1 @@
-tests/qapi-schema/include-non-file.json:1: Expected a file name (string), got: ['foo', 'bar']
+tests/qapi-schema/include-non-file.json:1: Value of 'include' must be a string
diff --git a/tests/qapi-schema/include-non-file.json b/tests/qapi-schema/include-non-file.json
index cd43c3f..4711aa4 100644
--- a/tests/qapi-schema/include-non-file.json
+++ b/tests/qapi-schema/include-non-file.json
@@ -1 +1 @@
-{ 'include': [ 'foo', 'bar' ] }
+{ 'include': {} }
--
2.4.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PULL v2 06/20] qapi: Invoke exception superclass initializer
2015-10-12 18:19 [Qemu-devel] [PULL v2 00/20] QAPI patches Markus Armbruster
` (4 preceding siblings ...)
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 05/20] qapi: Improve 'include' error message Markus Armbruster
@ 2015-10-12 18:19 ` Markus Armbruster
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 07/20] qapi: Clean up qapi.py per pep8 Markus Armbruster
` (14 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2015-10-12 18:19 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
pylint recommends that every exception class should explicitly
invoke the superclass __init__, even though things seem to work
fine without it.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-4-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/qapi.py | 2 ++
1 file changed, 2 insertions(+)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 362e007..a2d869e 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -81,6 +81,7 @@ def error_path(parent):
class QAPISchemaError(Exception):
def __init__(self, schema, msg):
+ Exception.__init__(self)
self.fname = schema.fname
self.msg = msg
self.col = 1
@@ -98,6 +99,7 @@ class QAPISchemaError(Exception):
class QAPIExprError(Exception):
def __init__(self, expr_info, msg):
+ Exception.__init__(self)
self.info = expr_info
self.msg = msg
--
2.4.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PULL v2 07/20] qapi: Clean up qapi.py per pep8
2015-10-12 18:19 [Qemu-devel] [PULL v2 00/20] QAPI patches Markus Armbruster
` (5 preceding siblings ...)
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 06/20] qapi: Invoke exception superclass initializer Markus Armbruster
@ 2015-10-12 18:19 ` Markus Armbruster
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 08/20] qapi: Test for various name collisions Markus Armbruster
` (13 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2015-10-12 18:19 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
Silence pep8, and make pylint a bit happier. Just style cleanups,
plus killing a useless comment in camel_to_upper(); no semantic
changes.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-5-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/ordereddict.py | 3 +-
scripts/qapi.py | 161 ++++++++++++++++++++++++++++++++-----------------
2 files changed, 106 insertions(+), 58 deletions(-)
diff --git a/scripts/ordereddict.py b/scripts/ordereddict.py
index 7242b50..2d1d813 100644
--- a/scripts/ordereddict.py
+++ b/scripts/ordereddict.py
@@ -22,6 +22,7 @@
from UserDict import DictMixin
+
class OrderedDict(dict, DictMixin):
def __init__(self, *args, **kwds):
@@ -117,7 +118,7 @@ class OrderedDict(dict, DictMixin):
if isinstance(other, OrderedDict):
if len(self) != len(other):
return False
- for p, q in zip(self.items(), other.items()):
+ for p, q in zip(self.items(), other.items()):
if p != q:
return False
return True
diff --git a/scripts/qapi.py b/scripts/qapi.py
index a2d869e..4b5d574 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -71,6 +71,7 @@ all_names = {}
# Parsing the schema into expressions
#
+
def error_path(parent):
res = ""
while parent:
@@ -79,6 +80,7 @@ def error_path(parent):
parent = parent['parent']
return res
+
class QAPISchemaError(Exception):
def __init__(self, schema, msg):
Exception.__init__(self)
@@ -97,6 +99,7 @@ class QAPISchemaError(Exception):
return error_path(self.info) + \
"%s:%d:%d: %s" % (self.fname, self.line, self.col, self.msg)
+
class QAPIExprError(Exception):
def __init__(self, expr_info, msg):
Exception.__init__(self)
@@ -107,9 +110,10 @@ class QAPIExprError(Exception):
return error_path(self.info['parent']) + \
"%s:%d: %s" % (self.info['file'], self.info['line'], self.msg)
+
class QAPISchemaParser(object):
- def __init__(self, fp, previously_included = [], incl_info = None):
+ def __init__(self, fp, previously_included=[], incl_info=None):
abs_fname = os.path.abspath(fp.name)
fname = fp.name
self.fname = fname
@@ -124,13 +128,14 @@ class QAPISchemaParser(object):
self.exprs = []
self.accept()
- while self.tok != None:
+ while self.tok is not None:
expr_info = {'file': fname, 'line': self.line,
'parent': self.incl_info}
expr = self.get_expr(False)
if isinstance(expr, dict) and "include" in expr:
if len(expr) != 1:
- raise QAPIExprError(expr_info, "Invalid 'include' directive")
+ raise QAPIExprError(expr_info,
+ "Invalid 'include' directive")
include = expr["include"]
if not isinstance(include, str):
raise QAPIExprError(expr_info,
@@ -193,7 +198,7 @@ class QAPISchemaParser(object):
string += '\t'
elif ch == 'u':
value = 0
- for x in range(0, 4):
+ for _ in range(0, 4):
ch = self.src[self.cursor]
self.cursor += 1
if ch not in "0123456789abcdefABCDEF":
@@ -215,7 +220,7 @@ class QAPISchemaParser(object):
string += ch
else:
raise QAPISchemaError(self,
- "Unknown escape \\%s" %ch)
+ "Unknown escape \\%s" % ch)
esc = False
elif ch == "\\":
esc = True
@@ -275,7 +280,7 @@ class QAPISchemaParser(object):
if self.tok == ']':
self.accept()
return expr
- if not self.tok in "{['tfn":
+ if self.tok not in "{['tfn":
raise QAPISchemaError(self, 'Expected "{", "[", "]", string, '
'boolean or "null"')
while True:
@@ -309,15 +314,17 @@ class QAPISchemaParser(object):
# TODO catching name collisions in generated code would be nice
#
+
def find_base_fields(base):
base_struct_define = find_struct(base)
if not base_struct_define:
return None
return base_struct_define['data']
+
# Return the qtype of an alternate branch, or None on error.
def find_alternate_member_qtype(qapi_type):
- if builtin_types.has_key(qapi_type):
+ if qapi_type in builtin_types:
return builtin_types[qapi_type]
elif find_struct(qapi_type):
return "QTYPE_QDICT"
@@ -327,6 +334,7 @@ def find_alternate_member_qtype(qapi_type):
return "QTYPE_QDICT"
return None
+
# Return the discriminator enum define if discriminator is specified as an
# enum type, otherwise return None.
def discriminator_find_enum_define(expr):
@@ -346,11 +354,14 @@ def discriminator_find_enum_define(expr):
return find_enum(discriminator_type)
+
# FIXME should enforce "other than downstream extensions [...], all
# names should begin with a letter".
valid_name = re.compile('^[a-zA-Z_][a-zA-Z0-9_.-]*$')
-def check_name(expr_info, source, name, allow_optional = False,
- enum_member = False):
+
+
+def check_name(expr_info, source, name, allow_optional=False,
+ enum_member=False):
global valid_name
membername = name
@@ -371,7 +382,8 @@ def check_name(expr_info, source, name, allow_optional = False,
raise QAPIExprError(expr_info,
"%s uses invalid name '%s'" % (source, name))
-def add_name(name, info, meta, implicit = False):
+
+def add_name(name, info, meta, implicit=False):
global all_names
check_name(info, "'%s'" % meta, name)
# FIXME should reject names that differ only in '_' vs. '.'
@@ -386,12 +398,14 @@ def add_name(name, info, meta, implicit = False):
% (meta, name))
all_names[name] = meta
+
def add_struct(definition, info):
global struct_types
name = definition['struct']
add_name(name, info, 'struct')
struct_types.append(definition)
+
def find_struct(name):
global struct_types
for struct in struct_types:
@@ -399,12 +413,14 @@ def find_struct(name):
return struct
return None
+
def add_union(definition, info):
global union_types
name = definition['union']
add_name(name, info, 'union')
union_types.append(definition)
+
def find_union(name):
global union_types
for union in union_types:
@@ -412,11 +428,13 @@ def find_union(name):
return union
return None
-def add_enum(name, info, enum_values = None, implicit = False):
+
+def add_enum(name, info, enum_values=None, implicit=False):
global enum_types
add_name(name, info, 'enum', implicit)
enum_types.append({"enum_name": name, "enum_values": enum_values})
+
def find_enum(name):
global enum_types
for enum in enum_types:
@@ -424,12 +442,14 @@ def find_enum(name):
return enum
return None
+
def is_enum(name):
- return find_enum(name) != None
+ return find_enum(name) is not None
-def check_type(expr_info, source, value, allow_array = False,
- allow_dict = False, allow_optional = False,
- allow_metas = []):
+
+def check_type(expr_info, source, value, allow_array=False,
+ allow_dict=False, allow_optional=False,
+ allow_metas=[]):
global all_names
if value is None:
@@ -448,7 +468,7 @@ def check_type(expr_info, source, value, allow_array = False,
# Check if type name for value is okay
if isinstance(value, str):
- if not value in all_names:
+ if value not in all_names:
raise QAPIExprError(expr_info,
"%s uses unknown type '%s'"
% (source, value))
@@ -477,7 +497,8 @@ def check_type(expr_info, source, value, allow_array = False,
allow_metas=['built-in', 'union', 'alternate', 'struct',
'enum'])
-def check_member_clash(expr_info, base_name, data, source = ""):
+
+def check_member_clash(expr_info, base_name, data, source=""):
base = find_struct(base_name)
assert base
base_members = base['data']
@@ -491,6 +512,7 @@ def check_member_clash(expr_info, base_name, data, source = ""):
if base.get('base'):
check_member_clash(expr_info, base['base'], data, source)
+
def check_command(expr, expr_info):
name = expr['command']
@@ -504,6 +526,7 @@ def check_command(expr, expr_info):
expr.get('returns'), allow_array=True,
allow_optional=True, allow_metas=returns_meta)
+
def check_event(expr, expr_info):
global events
name = expr['event']
@@ -515,19 +538,20 @@ def check_event(expr, expr_info):
expr.get('data'), allow_dict=True, allow_optional=True,
allow_metas=['struct'])
+
def check_union(expr, expr_info):
name = expr['union']
base = expr.get('base')
discriminator = expr.get('discriminator')
members = expr['data']
- values = { 'MAX': '(automatic)' }
+ values = {'MAX': '(automatic)'}
# Two types of unions, determined by discriminator.
# With no discriminator it is a simple union.
if discriminator is None:
enum_define = None
- allow_metas=['built-in', 'union', 'alternate', 'struct', 'enum']
+ allow_metas = ['built-in', 'union', 'alternate', 'struct', 'enum']
if base is not None:
raise QAPIExprError(expr_info,
"Simple union '%s' must not have a base"
@@ -557,7 +581,7 @@ def check_union(expr, expr_info):
"struct '%s'"
% (discriminator, base))
enum_define = find_enum(discriminator_type)
- allow_metas=['struct']
+ allow_metas = ['struct']
# Do not allow string discriminator
if not enum_define:
raise QAPIExprError(expr_info,
@@ -581,7 +605,7 @@ def check_union(expr, expr_info):
# If the discriminator names an enum type, then all members
# of 'data' must also be members of the enum type.
if enum_define:
- if not key in enum_define['enum_values']:
+ if key not in enum_define['enum_values']:
raise QAPIExprError(expr_info,
"Discriminator value '%s' is not found in "
"enum '%s'" %
@@ -596,10 +620,11 @@ def check_union(expr, expr_info):
% (name, key, values[c_key]))
values[c_key] = key
+
def check_alternate(expr, expr_info):
name = expr['alternate']
members = expr['data']
- values = { 'MAX': '(automatic)' }
+ values = {'MAX': '(automatic)'}
types_seen = {}
# Check every branch
@@ -627,11 +652,12 @@ def check_alternate(expr, expr_info):
% (name, key, types_seen[qtype]))
types_seen[qtype] = key
+
def check_enum(expr, expr_info):
name = expr['enum']
members = expr.get('data')
prefix = expr.get('prefix')
- values = { 'MAX': '(automatic)' }
+ values = {'MAX': '(automatic)'}
if not isinstance(members, list):
raise QAPIExprError(expr_info,
@@ -640,7 +666,7 @@ def check_enum(expr, expr_info):
raise QAPIExprError(expr_info,
"Enum '%s' requires a string for 'prefix'" % name)
for member in members:
- check_name(expr_info, "Member of enum '%s'" %name, member,
+ check_name(expr_info, "Member of enum '%s'" % name, member,
enum_member=True)
key = camel_to_upper(member)
if key in values:
@@ -649,6 +675,7 @@ def check_enum(expr, expr_info):
% (name, member, values[key]))
values[key] = member
+
def check_struct(expr, expr_info):
name = expr['struct']
members = expr['data']
@@ -660,6 +687,7 @@ def check_struct(expr, expr_info):
if expr.get('base'):
check_member_clash(expr_info, expr['base'], expr['data'])
+
def check_keys(expr_elem, meta, required, optional=[]):
expr = expr_elem['expr']
info = expr_elem['info']
@@ -667,22 +695,23 @@ def check_keys(expr_elem, meta, required, optional=[]):
if not isinstance(name, str):
raise QAPIExprError(info,
"'%s' key must have a string value" % meta)
- required = required + [ meta ]
+ required = required + [meta]
for (key, value) in expr.items():
- if not key in required and not key in optional:
+ if key not in required and key not in optional:
raise QAPIExprError(info,
"Unknown key '%s' in %s '%s'"
% (key, meta, name))
- if (key == 'gen' or key == 'success-response') and value != False:
+ if (key == 'gen' or key == 'success-response') and value is not False:
raise QAPIExprError(info,
"'%s' of %s '%s' should only use false value"
% (key, meta, name))
for key in required:
- if not expr.has_key(key):
+ if key not in expr:
raise QAPIExprError(info,
"Key '%s' is missing from %s '%s'"
% (key, meta, name))
+
def check_exprs(exprs):
global all_names
@@ -692,24 +721,24 @@ def check_exprs(exprs):
for expr_elem in exprs:
expr = expr_elem['expr']
info = expr_elem['info']
- if expr.has_key('enum'):
+ if 'enum' in expr:
check_keys(expr_elem, 'enum', ['data'], ['prefix'])
add_enum(expr['enum'], info, expr['data'])
- elif expr.has_key('union'):
+ elif 'union' in expr:
check_keys(expr_elem, 'union', ['data'],
['base', 'discriminator'])
add_union(expr, info)
- elif expr.has_key('alternate'):
+ elif 'alternate' in expr:
check_keys(expr_elem, 'alternate', ['data'])
add_name(expr['alternate'], info, 'alternate')
- elif expr.has_key('struct'):
+ elif 'struct' in expr:
check_keys(expr_elem, 'struct', ['data'], ['base'])
add_struct(expr, info)
- elif expr.has_key('command'):
+ elif 'command' in expr:
check_keys(expr_elem, 'command', [],
['data', 'returns', 'gen', 'success-response'])
add_name(expr['command'], info, 'command')
- elif expr.has_key('event'):
+ elif 'event' in expr:
check_keys(expr_elem, 'event', [], ['data'])
add_name(expr['event'], info, 'event')
else:
@@ -719,11 +748,11 @@ def check_exprs(exprs):
# Try again for hidden UnionKind enum
for expr_elem in exprs:
expr = expr_elem['expr']
- if expr.has_key('union'):
+ if 'union' in expr:
if not discriminator_find_enum_define(expr):
add_enum('%sKind' % expr['union'], expr_elem['info'],
implicit=True)
- elif expr.has_key('alternate'):
+ elif 'alternate' in expr:
add_enum('%sKind' % expr['alternate'], expr_elem['info'],
implicit=True)
@@ -732,17 +761,17 @@ def check_exprs(exprs):
expr = expr_elem['expr']
info = expr_elem['info']
- if expr.has_key('enum'):
+ if 'enum' in expr:
check_enum(expr, info)
- elif expr.has_key('union'):
+ elif 'union' in expr:
check_union(expr, info)
- elif expr.has_key('alternate'):
+ elif 'alternate' in expr:
check_alternate(expr, info)
- elif expr.has_key('struct'):
+ elif 'struct' in expr:
check_struct(expr, info)
- elif expr.has_key('command'):
+ elif 'command' in expr:
check_command(expr, info)
- elif expr.has_key('event'):
+ elif 'event' in expr:
check_event(expr, info)
else:
assert False, 'unexpected meta type'
@@ -994,6 +1023,7 @@ class QAPISchemaObjectTypeVariants(object):
vseen = dict(seen)
v.check(schema, self.tag_member.type, vseen)
+
class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
def __init__(self, name, typ):
QAPISchemaObjectTypeMember.__init__(self, name, typ, False)
@@ -1293,6 +1323,7 @@ def camel_case(name):
new_name += ch.lower()
return new_name
+
# ENUMName -> ENUM_NAME, EnumName1 -> ENUM_NAME1
# ENUM_NAME -> ENUM_NAME, ENUM_NAME1 -> ENUM_NAME1, ENUM_Name2 -> ENUM_NAME2
# ENUM24_Name -> ENUM24_NAME
@@ -1307,14 +1338,14 @@ def camel_to_upper(value):
c = c_fun_str[i]
# When c is upper and no "_" appears before, do more checks
if c.isupper() and (i > 0) and c_fun_str[i - 1] != "_":
- # Case 1: next string is lower
- # Case 2: previous string is digit
- if (i < (l - 1) and c_fun_str[i + 1].islower()) or \
- c_fun_str[i - 1].isdigit():
+ if i < l - 1 and c_fun_str[i + 1].islower():
+ new_name += '_'
+ elif c_fun_str[i - 1].isdigit():
new_name += '_'
new_name += c
return new_name.lstrip('_').upper()
+
def c_enum_const(type_name, const_name, prefix=None):
if prefix is not None:
type_name = prefix
@@ -1322,6 +1353,7 @@ def c_enum_const(type_name, const_name, prefix=None):
c_name_trans = string.maketrans('.-', '__')
+
# Map @name to a valid C identifier.
# If @protect, avoid returning certain ticklish identifiers (like
# C keywords) by prepending "q_".
@@ -1334,15 +1366,16 @@ c_name_trans = string.maketrans('.-', '__')
def c_name(name, protect=True):
# ANSI X3J11/88-090, 3.1.1
c89_words = set(['auto', 'break', 'case', 'char', 'const', 'continue',
- 'default', 'do', 'double', 'else', 'enum', 'extern', 'float',
- 'for', 'goto', 'if', 'int', 'long', 'register', 'return',
- 'short', 'signed', 'sizeof', 'static', 'struct', 'switch',
- 'typedef', 'union', 'unsigned', 'void', 'volatile', 'while'])
+ 'default', 'do', 'double', 'else', 'enum', 'extern',
+ 'float', 'for', 'goto', 'if', 'int', 'long', 'register',
+ 'return', 'short', 'signed', 'sizeof', 'static',
+ 'struct', 'switch', 'typedef', 'union', 'unsigned',
+ 'void', 'volatile', 'while'])
# ISO/IEC 9899:1999, 6.4.1
c99_words = set(['inline', 'restrict', '_Bool', '_Complex', '_Imaginary'])
# ISO/IEC 9899:2011, 6.4.1
- c11_words = set(['_Alignas', '_Alignof', '_Atomic', '_Generic', '_Noreturn',
- '_Static_assert', '_Thread_local'])
+ c11_words = set(['_Alignas', '_Alignof', '_Atomic', '_Generic',
+ '_Noreturn', '_Static_assert', '_Thread_local'])
# GCC http://gcc.gnu.org/onlinedocs/gcc-4.7.1/gcc/C-Extensions.html
# excluding _.*
gcc_words = set(['asm', 'typeof'])
@@ -1358,29 +1391,34 @@ def c_name(name, protect=True):
'not_eq', 'or', 'or_eq', 'xor', 'xor_eq'])
# namespace pollution:
polluted_words = set(['unix', 'errno'])
- if protect and (name in c89_words | c99_words | c11_words | gcc_words | cpp_words | polluted_words):
+ if protect and (name in c89_words | c99_words | c11_words | gcc_words
+ | cpp_words | polluted_words):
return "q_" + name
return name.translate(c_name_trans)
eatspace = '\033EATSPACE.'
pointer_suffix = ' *' + eatspace
+
def genindent(count):
ret = ""
- for i in range(count):
+ for _ in range(count):
ret += " "
return ret
indent_level = 0
+
def push_indent(indent_amount=4):
global indent_level
indent_level += indent_amount
+
def pop_indent(indent_amount=4):
global indent_level
indent_level -= indent_amount
+
# Generate @code with @kwds interpolated.
# Obey indent_level, and strip eatspace.
def cgen(code, **kwds):
@@ -1393,6 +1431,7 @@ def cgen(code, **kwds):
raw = raw[0]
return re.sub(re.escape(eatspace) + ' *', '', raw)
+
def mcgen(code, **kwds):
if code[0] == '\n':
code = code[1:]
@@ -1402,6 +1441,7 @@ def mcgen(code, **kwds):
def guardname(filename):
return c_name(filename, protect=False).upper()
+
def guardstart(name):
return mcgen('''
@@ -1411,6 +1451,7 @@ def guardstart(name):
''',
name=guardname(name))
+
def guardend(name):
return mcgen('''
@@ -1419,6 +1460,7 @@ def guardend(name):
''',
name=guardname(name))
+
def gen_enum_lookup(name, values, prefix=None):
ret = mcgen('''
@@ -1440,6 +1482,7 @@ const char *const %(c_name)s_lookup[] = {
max_index=max_index)
return ret
+
def gen_enum(name, values, prefix=None):
# append automatically generated _MAX value
enum_values = values + ['MAX']
@@ -1471,6 +1514,7 @@ extern const char *const %(c_name)s_lookup[];
c_name=c_name(name))
return ret
+
def gen_params(arg_type, extra):
if not arg_type:
return extra
@@ -1491,7 +1535,8 @@ def gen_params(arg_type, extra):
# Common command line parsing
#
-def parse_command_line(extra_options = "", extra_long_options = []):
+
+def parse_command_line(extra_options="", extra_long_options=[]):
try:
opts, args = getopt.gnu_getopt(sys.argv[1:],
@@ -1542,6 +1587,7 @@ def parse_command_line(extra_options = "", extra_long_options = []):
# Generate output files with boilerplate
#
+
def open_output(output_dir, do_c, do_h, prefix, c_file, h_file,
c_comment, h_comment):
guard = guardname(prefix + h_file)
@@ -1569,7 +1615,7 @@ def open_output(output_dir, do_c, do_h, prefix, c_file, h_file,
/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
%(comment)s
''',
- comment = c_comment))
+ comment=c_comment))
fdecl.write(mcgen('''
/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
@@ -1578,10 +1624,11 @@ def open_output(output_dir, do_c, do_h, prefix, c_file, h_file,
#define %(guard)s
''',
- comment = h_comment, guard = guard))
+ comment=h_comment, guard=guard))
return (fdef, fdecl)
+
def close_output(fdef, fdecl):
fdecl.write('''
#endif
--
2.4.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PULL v2 08/20] qapi: Test for various name collisions
2015-10-12 18:19 [Qemu-devel] [PULL v2 00/20] QAPI patches Markus Armbruster
` (6 preceding siblings ...)
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 07/20] qapi: Clean up qapi.py per pep8 Markus Armbruster
@ 2015-10-12 18:19 ` Markus Armbruster
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 09/20] qapi: Avoid assertion failure on union 'type' collision Markus Armbruster
` (12 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2015-10-12 18:19 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
Expose some weaknesses in the generator: we don't always forbid
the generation of structs that contain multiple members that map
to the same C or QMP name. This has already been marked FIXME in
qapi.py in commit d90675f, but having more tests will make sure
future patches produce desired behavior; and updating existing
patches to better document things doesn't hurt, either. Some of
these collisions are already caught in the old-style parser
checks, but ultimately we want all collisions to be caught in the
new-style QAPISchema*.check() methods.
This patch focuses on C struct members, and does not consider
collisions between commands and events (affecting C function
names), or even collisions between generated C type names with
user type names (for things like automatic FOOList struct
representing array types or FOOKind for an implicit enum).
There are two types of struct collisions we want to catch:
1) Collision between two keys in a JSON object. qapi.py prevents
that within a single struct (see test duplicate-key), but it is
possible to have collisions between a type's members and its
base type's members (existing tests struct-base-clash,
struct-base-clash-deep), and its flat union variant members
(renamed test flat-union-clash-member).
2) Collision between two members of the C struct that is generated
for a given QAPI type:
a) Multiple QAPI names map to the same C name (new test
args-name-clash)
b) A QAPI name maps to a C name that is used for another purpose
(new tests flat-union-clash-branch, struct-base-clash-base,
union-clash-data). We already fixed some such cases in commit
0f61af3e and 1e6c1616, but more remain.
c) Two C names generated for other purposes clash
(updated test alternate-clash, new test union-clash-branches,
union-clash-type, flat-union-clash-type)
Ultimately, if we need to have a flat union where a tag value
clashes with a base member name, we could change the generator to
name the union (using 'foo.u.value' rather than 'foo.value') or
otherwise munge the C name corresponding to tag values. But
unless such a need arises, it will probably be easier to just
forbid these collisions.
Some of these negative tests will be deleted later, and positive
tests added to qapi-schema-test.json in their place, when the
generator code is reworked to avoid particular code generation
collisions in class 2).
[Note that viewing this patch with git rename detection enabled
may see some confusion due to renaming some tests while adding
others, but where the content is similar enough that git picks
the wrong pre- and post-patch files to associate]
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-6-git-send-email-eblake@redhat.com>
[Improve commit message and comments a bit, drop an unrelated test]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/Makefile | 9 ++++++++-
tests/qapi-schema/alternate-clash.err | 2 +-
tests/qapi-schema/alternate-clash.json | 9 +++++++--
...flat-union-branch-clash.out => args-name-clash.err} | 0
tests/qapi-schema/args-name-clash.exit | 1 +
tests/qapi-schema/args-name-clash.json | 5 +++++
tests/qapi-schema/args-name-clash.out | 6 ++++++
tests/qapi-schema/duplicate-key.err | 2 +-
tests/qapi-schema/duplicate-key.json | 1 +
tests/qapi-schema/flat-union-base-union.err | 2 +-
tests/qapi-schema/flat-union-base-union.json | 5 ++++-
tests/qapi-schema/flat-union-branch-clash.err | 1 -
tests/qapi-schema/flat-union-clash-branch.err | 0
tests/qapi-schema/flat-union-clash-branch.exit | 1 +
tests/qapi-schema/flat-union-clash-branch.json | 18 ++++++++++++++++++
tests/qapi-schema/flat-union-clash-branch.out | 14 ++++++++++++++
tests/qapi-schema/flat-union-clash-member.err | 1 +
...-branch-clash.exit => flat-union-clash-member.exit} | 0
...-branch-clash.json => flat-union-clash-member.json} | 3 ++-
tests/qapi-schema/flat-union-clash-member.out | 0
tests/qapi-schema/flat-union-clash-type.err | 16 ++++++++++++++++
tests/qapi-schema/flat-union-clash-type.exit | 1 +
tests/qapi-schema/flat-union-clash-type.json | 16 ++++++++++++++++
tests/qapi-schema/flat-union-clash-type.out | 0
tests/qapi-schema/qapi-schema-test.json | 7 +++++--
tests/qapi-schema/qapi-schema-test.out | 2 ++
tests/qapi-schema/struct-base-clash-base.err | 0
tests/qapi-schema/struct-base-clash-base.exit | 1 +
tests/qapi-schema/struct-base-clash-base.json | 9 +++++++++
tests/qapi-schema/struct-base-clash-base.out | 5 +++++
tests/qapi-schema/struct-base-clash-deep.err | 2 +-
tests/qapi-schema/struct-base-clash-deep.json | 5 ++++-
tests/qapi-schema/struct-base-clash.err | 2 +-
tests/qapi-schema/struct-base-clash.json | 3 ++-
tests/qapi-schema/union-clash-branches.err | 1 +
tests/qapi-schema/union-clash-branches.exit | 1 +
tests/qapi-schema/union-clash-branches.json | 5 +++++
tests/qapi-schema/union-clash-branches.out | 0
tests/qapi-schema/union-clash-data.err | 0
tests/qapi-schema/union-clash-data.exit | 1 +
tests/qapi-schema/union-clash-data.json | 7 +++++++
tests/qapi-schema/union-clash-data.out | 6 ++++++
tests/qapi-schema/union-clash-type.err | 16 ++++++++++++++++
tests/qapi-schema/union-clash-type.exit | 1 +
tests/qapi-schema/union-clash-type.json | 10 ++++++++++
tests/qapi-schema/union-clash-type.out | 0
46 files changed, 182 insertions(+), 15 deletions(-)
rename tests/qapi-schema/{flat-union-branch-clash.out => args-name-clash.err} (100%)
create mode 100644 tests/qapi-schema/args-name-clash.exit
create mode 100644 tests/qapi-schema/args-name-clash.json
create mode 100644 tests/qapi-schema/args-name-clash.out
delete mode 100644 tests/qapi-schema/flat-union-branch-clash.err
create mode 100644 tests/qapi-schema/flat-union-clash-branch.err
create mode 100644 tests/qapi-schema/flat-union-clash-branch.exit
create mode 100644 tests/qapi-schema/flat-union-clash-branch.json
create mode 100644 tests/qapi-schema/flat-union-clash-branch.out
create mode 100644 tests/qapi-schema/flat-union-clash-member.err
rename tests/qapi-schema/{flat-union-branch-clash.exit => flat-union-clash-member.exit} (100%)
rename tests/qapi-schema/{flat-union-branch-clash.json => flat-union-clash-member.json} (77%)
create mode 100644 tests/qapi-schema/flat-union-clash-member.out
create mode 100644 tests/qapi-schema/flat-union-clash-type.err
create mode 100644 tests/qapi-schema/flat-union-clash-type.exit
create mode 100644 tests/qapi-schema/flat-union-clash-type.json
create mode 100644 tests/qapi-schema/flat-union-clash-type.out
create mode 100644 tests/qapi-schema/struct-base-clash-base.err
create mode 100644 tests/qapi-schema/struct-base-clash-base.exit
create mode 100644 tests/qapi-schema/struct-base-clash-base.json
create mode 100644 tests/qapi-schema/struct-base-clash-base.out
create mode 100644 tests/qapi-schema/union-clash-branches.err
create mode 100644 tests/qapi-schema/union-clash-branches.exit
create mode 100644 tests/qapi-schema/union-clash-branches.json
create mode 100644 tests/qapi-schema/union-clash-branches.out
create mode 100644 tests/qapi-schema/union-clash-data.err
create mode 100644 tests/qapi-schema/union-clash-data.exit
create mode 100644 tests/qapi-schema/union-clash-data.json
create mode 100644 tests/qapi-schema/union-clash-data.out
create mode 100644 tests/qapi-schema/union-clash-type.err
create mode 100644 tests/qapi-schema/union-clash-type.exit
create mode 100644 tests/qapi-schema/union-clash-type.json
create mode 100644 tests/qapi-schema/union-clash-type.out
diff --git a/tests/Makefile b/tests/Makefile
index 164dea3..49fdbe2 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -241,6 +241,7 @@ qapi-schema += args-invalid.json
qapi-schema += args-member-array-bad.json
qapi-schema += args-member-array.json
qapi-schema += args-member-unknown.json
+qapi-schema += args-name-clash.json
qapi-schema += args-union.json
qapi-schema += args-unknown.json
qapi-schema += bad-base.json
@@ -276,7 +277,9 @@ qapi-schema += flat-union-bad-base.json
qapi-schema += flat-union-bad-discriminator.json
qapi-schema += flat-union-base-any.json
qapi-schema += flat-union-base-union.json
-qapi-schema += flat-union-branch-clash.json
+qapi-schema += flat-union-clash-branch.json
+qapi-schema += flat-union-clash-member.json
+qapi-schema += flat-union-clash-type.json
qapi-schema += flat-union-inline.json
qapi-schema += flat-union-int-branch.json
qapi-schema += flat-union-invalid-branch-key.json
@@ -318,6 +321,7 @@ qapi-schema += returns-dict.json
qapi-schema += returns-int.json
qapi-schema += returns-unknown.json
qapi-schema += returns-whitelist.json
+qapi-schema += struct-base-clash-base.json
qapi-schema += struct-base-clash-deep.json
qapi-schema += struct-base-clash.json
qapi-schema += struct-data-invalid.json
@@ -331,6 +335,9 @@ qapi-schema += unclosed-string.json
qapi-schema += unicode-str.json
qapi-schema += union-bad-branch.json
qapi-schema += union-base-no-discriminator.json
+qapi-schema += union-clash-branches.json
+qapi-schema += union-clash-data.json
+qapi-schema += union-clash-type.json
qapi-schema += union-invalid-base.json
qapi-schema += union-max.json
qapi-schema += union-optional-branch.json
diff --git a/tests/qapi-schema/alternate-clash.err b/tests/qapi-schema/alternate-clash.err
index 51bea3e..a475ab6 100644
--- a/tests/qapi-schema/alternate-clash.err
+++ b/tests/qapi-schema/alternate-clash.err
@@ -1 +1 @@
-tests/qapi-schema/alternate-clash.json:2: Alternate 'Alt1' member 'ONE' clashes with 'one'
+tests/qapi-schema/alternate-clash.json:7: Alternate 'Alt1' member 'a_b' clashes with 'a-b'
diff --git a/tests/qapi-schema/alternate-clash.json b/tests/qapi-schema/alternate-clash.json
index 3947935..6d73bc5 100644
--- a/tests/qapi-schema/alternate-clash.json
+++ b/tests/qapi-schema/alternate-clash.json
@@ -1,3 +1,8 @@
-# we detect C enum collisions in an alternate
+# Alternate branch name collision
+# Reject an alternate that would result in a collision in generated C
+# names (this would try to generate two enum values 'ALT1_KIND_A_B').
+# TODO: In the future, if alternates are simplified to not generate
+# the implicit Alt1Kind enum, we would still have a collision with the
+# resulting C union trying to have two members named 'a_b'.
{ 'alternate': 'Alt1',
- 'data': { 'one': 'str', 'ONE': 'int' } }
+ 'data': { 'a-b': 'str', 'a_b': 'int' } }
diff --git a/tests/qapi-schema/flat-union-branch-clash.out b/tests/qapi-schema/args-name-clash.err
similarity index 100%
rename from tests/qapi-schema/flat-union-branch-clash.out
rename to tests/qapi-schema/args-name-clash.err
diff --git a/tests/qapi-schema/args-name-clash.exit b/tests/qapi-schema/args-name-clash.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/args-name-clash.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/args-name-clash.json b/tests/qapi-schema/args-name-clash.json
new file mode 100644
index 0000000..9e8f889
--- /dev/null
+++ b/tests/qapi-schema/args-name-clash.json
@@ -0,0 +1,5 @@
+# C member name collision
+# FIXME - This parses, but fails to compile, because the C struct is given
+# two 'a_b' members. Either reject this at parse time, or munge the C names
+# to avoid the collision.
+{ 'command': 'oops', 'data': { 'a-b': 'str', 'a_b': 'str' } }
diff --git a/tests/qapi-schema/args-name-clash.out b/tests/qapi-schema/args-name-clash.out
new file mode 100644
index 0000000..9b2f6e4
--- /dev/null
+++ b/tests/qapi-schema/args-name-clash.out
@@ -0,0 +1,6 @@
+object :empty
+object :obj-oops-arg
+ member a-b: str optional=False
+ member a_b: str optional=False
+command oops :obj-oops-arg -> None
+ gen=True success_response=True
diff --git a/tests/qapi-schema/duplicate-key.err b/tests/qapi-schema/duplicate-key.err
index 768b276..6d02f83 100644
--- a/tests/qapi-schema/duplicate-key.err
+++ b/tests/qapi-schema/duplicate-key.err
@@ -1 +1 @@
-tests/qapi-schema/duplicate-key.json:2:10: Duplicate key "key"
+tests/qapi-schema/duplicate-key.json:3:10: Duplicate key "key"
diff --git a/tests/qapi-schema/duplicate-key.json b/tests/qapi-schema/duplicate-key.json
index 1b55d88..14ac0e8 100644
--- a/tests/qapi-schema/duplicate-key.json
+++ b/tests/qapi-schema/duplicate-key.json
@@ -1,2 +1,3 @@
+# QAPI cannot include the same key more than once in any {}
{ 'key': 'value',
'key': 'value' }
diff --git a/tests/qapi-schema/flat-union-base-union.err b/tests/qapi-schema/flat-union-base-union.err
index ede9859..28725ed 100644
--- a/tests/qapi-schema/flat-union-base-union.err
+++ b/tests/qapi-schema/flat-union-base-union.err
@@ -1 +1 @@
-tests/qapi-schema/flat-union-base-union.json:11: Base 'UnionBase' is not a valid struct
+tests/qapi-schema/flat-union-base-union.json:14: Base 'UnionBase' is not a valid struct
diff --git a/tests/qapi-schema/flat-union-base-union.json b/tests/qapi-schema/flat-union-base-union.json
index 6a8ea68..98b4eba 100644
--- a/tests/qapi-schema/flat-union-base-union.json
+++ b/tests/qapi-schema/flat-union-base-union.json
@@ -1,4 +1,7 @@
-# we require the base to be a struct
+# For now, we require the base to be a struct without variants
+# TODO: It would be possible to allow a union as a base, as long as all
+# permutations of QMP names exposed by base do not clash with any QMP
+# member names added by local variants.
{ 'enum': 'TestEnum',
'data': [ 'value1', 'value2' ] }
{ 'struct': 'TestTypeA',
diff --git a/tests/qapi-schema/flat-union-branch-clash.err b/tests/qapi-schema/flat-union-branch-clash.err
deleted file mode 100644
index f112766..0000000
--- a/tests/qapi-schema/flat-union-branch-clash.err
+++ /dev/null
@@ -1 +0,0 @@
-tests/qapi-schema/flat-union-branch-clash.json:10: Member name 'name' of branch 'value1' clashes with base 'Base'
diff --git a/tests/qapi-schema/flat-union-clash-branch.err b/tests/qapi-schema/flat-union-clash-branch.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/flat-union-clash-branch.exit b/tests/qapi-schema/flat-union-clash-branch.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/flat-union-clash-branch.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/flat-union-clash-branch.json b/tests/qapi-schema/flat-union-clash-branch.json
new file mode 100644
index 0000000..e593336
--- /dev/null
+++ b/tests/qapi-schema/flat-union-clash-branch.json
@@ -0,0 +1,18 @@
+# Flat union branch name collision
+# FIXME: this parses, but then fails to compile due to a duplicate 'c_d'
+# (one from the base member, the other from the branch name). We should
+# either reject the collision at parse time, or munge the generated branch
+# name to allow this to compile.
+{ 'enum': 'TestEnum',
+ 'data': [ 'base', 'c-d' ] }
+{ 'struct': 'Base',
+ 'data': { 'enum1': 'TestEnum', '*c_d': 'str' } }
+{ 'struct': 'Branch1',
+ 'data': { 'string': 'str' } }
+{ 'struct': 'Branch2',
+ 'data': { 'value': 'int' } }
+{ 'union': 'TestUnion',
+ 'base': 'Base',
+ 'discriminator': 'enum1',
+ 'data': { 'base': 'Branch1',
+ 'c-d': 'Branch2' } }
diff --git a/tests/qapi-schema/flat-union-clash-branch.out b/tests/qapi-schema/flat-union-clash-branch.out
new file mode 100644
index 0000000..8e0da73
--- /dev/null
+++ b/tests/qapi-schema/flat-union-clash-branch.out
@@ -0,0 +1,14 @@
+object :empty
+object Base
+ member enum1: TestEnum optional=False
+ member c_d: str optional=True
+object Branch1
+ member string: str optional=False
+object Branch2
+ member value: int optional=False
+enum TestEnum ['base', 'c-d']
+object TestUnion
+ base Base
+ tag enum1
+ case base: Branch1
+ case c-d: Branch2
diff --git a/tests/qapi-schema/flat-union-clash-member.err b/tests/qapi-schema/flat-union-clash-member.err
new file mode 100644
index 0000000..2f0397a
--- /dev/null
+++ b/tests/qapi-schema/flat-union-clash-member.err
@@ -0,0 +1 @@
+tests/qapi-schema/flat-union-clash-member.json:11: Member name 'name' of branch 'value1' clashes with base 'Base'
diff --git a/tests/qapi-schema/flat-union-branch-clash.exit b/tests/qapi-schema/flat-union-clash-member.exit
similarity index 100%
rename from tests/qapi-schema/flat-union-branch-clash.exit
rename to tests/qapi-schema/flat-union-clash-member.exit
diff --git a/tests/qapi-schema/flat-union-branch-clash.json b/tests/qapi-schema/flat-union-clash-member.json
similarity index 77%
rename from tests/qapi-schema/flat-union-branch-clash.json
rename to tests/qapi-schema/flat-union-clash-member.json
index 8fb054f..9efc771 100644
--- a/tests/qapi-schema/flat-union-branch-clash.json
+++ b/tests/qapi-schema/flat-union-clash-member.json
@@ -1,4 +1,5 @@
-# we check for no duplicate keys between branches and base
+# We check for no duplicate keys between branch members and base
+# base's member 'name' clashes with Branch1's
{ 'enum': 'TestEnum',
'data': [ 'value1', 'value2' ] }
{ 'struct': 'Base',
diff --git a/tests/qapi-schema/flat-union-clash-member.out b/tests/qapi-schema/flat-union-clash-member.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/flat-union-clash-type.err b/tests/qapi-schema/flat-union-clash-type.err
new file mode 100644
index 0000000..6e64d1d
--- /dev/null
+++ b/tests/qapi-schema/flat-union-clash-type.err
@@ -0,0 +1,16 @@
+Traceback (most recent call last):
+ File "tests/qapi-schema/test-qapi.py", line 55, in <module>
+ schema = QAPISchema(sys.argv[1])
+ File "scripts/qapi.py", line 1116, in __init__
+ self.check()
+ File "scripts/qapi.py", line 1299, in check
+ ent.check(self)
+ File "scripts/qapi.py", line 962, in check
+ self.variants.check(schema, members, seen)
+ File "scripts/qapi.py", line 1024, in check
+ v.check(schema, self.tag_member.type, vseen)
+ File "scripts/qapi.py", line 1032, in check
+ QAPISchemaObjectTypeMember.check(self, schema, [], seen)
+ File "scripts/qapi.py", line 994, in check
+ assert self.name not in seen
+AssertionError
diff --git a/tests/qapi-schema/flat-union-clash-type.exit b/tests/qapi-schema/flat-union-clash-type.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/flat-union-clash-type.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/flat-union-clash-type.json b/tests/qapi-schema/flat-union-clash-type.json
new file mode 100644
index 0000000..3db6ea0
--- /dev/null
+++ b/tests/qapi-schema/flat-union-clash-type.json
@@ -0,0 +1,16 @@
+# Flat union branch 'type'
+# FIXME: this triggers an assertion failure. But even with that fixed,
+# we would have a clash in generated C, between the member 'type'
+# inherited from 'Base' and the branch name 'type' within the
+# union. We should either reject this, or munge the generated C to let
+# it compile.
+{ 'enum': 'TestEnum',
+ 'data': [ 'type' ] }
+{ 'struct': 'Base',
+ 'data': { 'type': 'TestEnum' } }
+{ 'struct': 'Branch1',
+ 'data': { 'string': 'str' } }
+{ 'union': 'TestUnion',
+ 'base': 'Base',
+ 'discriminator': 'type',
+ 'data': { 'type': 'Branch1' } }
diff --git a/tests/qapi-schema/flat-union-clash-type.out b/tests/qapi-schema/flat-union-clash-type.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 6897a6e..c511338 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -32,11 +32,14 @@
'dict1': 'UserDefTwoDict' } }
# for testing unions
+# Among other things, test that a name collision between branches does
+# not cause any problems (since only one branch can be in use at a time),
+# by intentionally using two branches that both have a C member 'a_b'
{ 'struct': 'UserDefA',
- 'data': { 'boolean': 'bool' } }
+ 'data': { 'boolean': 'bool', '*a_b': 'int' } }
{ 'struct': 'UserDefB',
- 'data': { 'intb': 'int' } }
+ 'data': { 'intb': 'int', '*a-b': 'bool' } }
{ 'union': 'UserDefFlatUnion',
'base': 'UserDefUnionBase', # intentional forward reference
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 1f6e858..28a0b3c 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -71,6 +71,7 @@ enum QEnumTwo ['value1', 'value2']
prefix QENUM_TWO
object UserDefA
member boolean: bool optional=False
+ member a_b: int optional=True
alternate UserDefAlternate
case uda: UserDefA
case s: str
@@ -78,6 +79,7 @@ alternate UserDefAlternate
enum UserDefAlternateKind ['uda', 's', 'i']
object UserDefB
member intb: int optional=False
+ member a-b: bool optional=True
object UserDefC
member string1: str optional=False
member string2: str optional=False
diff --git a/tests/qapi-schema/struct-base-clash-base.err b/tests/qapi-schema/struct-base-clash-base.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/struct-base-clash-base.exit b/tests/qapi-schema/struct-base-clash-base.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/struct-base-clash-base.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/struct-base-clash-base.json b/tests/qapi-schema/struct-base-clash-base.json
new file mode 100644
index 0000000..0c84025
--- /dev/null
+++ b/tests/qapi-schema/struct-base-clash-base.json
@@ -0,0 +1,9 @@
+# Struct member 'base'
+# FIXME: this parses, but then fails to compile due to a duplicate 'base'
+# (one explicit in QMP, the other used to box the base class members).
+# We should either reject the collision at parse time, or change the
+# generated struct to allow this to compile.
+{ 'struct': 'Base', 'data': {} }
+{ 'struct': 'Sub',
+ 'base': 'Base',
+ 'data': { 'base': 'str' } }
diff --git a/tests/qapi-schema/struct-base-clash-base.out b/tests/qapi-schema/struct-base-clash-base.out
new file mode 100644
index 0000000..e69a416
--- /dev/null
+++ b/tests/qapi-schema/struct-base-clash-base.out
@@ -0,0 +1,5 @@
+object :empty
+object Base
+object Sub
+ base Base
+ member base: str optional=False
diff --git a/tests/qapi-schema/struct-base-clash-deep.err b/tests/qapi-schema/struct-base-clash-deep.err
index e3e9f8d..f7a25a3 100644
--- a/tests/qapi-schema/struct-base-clash-deep.err
+++ b/tests/qapi-schema/struct-base-clash-deep.err
@@ -1 +1 @@
-tests/qapi-schema/struct-base-clash-deep.json:7: Member name 'name' clashes with base 'Base'
+tests/qapi-schema/struct-base-clash-deep.json:10: Member name 'name' clashes with base 'Base'
diff --git a/tests/qapi-schema/struct-base-clash-deep.json b/tests/qapi-schema/struct-base-clash-deep.json
index 552fe94..fa873ab 100644
--- a/tests/qapi-schema/struct-base-clash-deep.json
+++ b/tests/qapi-schema/struct-base-clash-deep.json
@@ -1,4 +1,7 @@
-# we check for no duplicate keys with indirect base
+# Reject attempts to duplicate QMP members
+# Here, 'name' would have to appear twice on the wire, locally and
+# indirectly for the grandparent base; the collision doesn't care that
+# one instance is optional.
{ 'struct': 'Base',
'data': { 'name': 'str' } }
{ 'struct': 'Mid',
diff --git a/tests/qapi-schema/struct-base-clash.err b/tests/qapi-schema/struct-base-clash.err
index 3ac37fb..3a9f66b 100644
--- a/tests/qapi-schema/struct-base-clash.err
+++ b/tests/qapi-schema/struct-base-clash.err
@@ -1 +1 @@
-tests/qapi-schema/struct-base-clash.json:4: Member name 'name' clashes with base 'Base'
+tests/qapi-schema/struct-base-clash.json:5: Member name 'name' clashes with base 'Base'
diff --git a/tests/qapi-schema/struct-base-clash.json b/tests/qapi-schema/struct-base-clash.json
index f2afc9b..11aec80 100644
--- a/tests/qapi-schema/struct-base-clash.json
+++ b/tests/qapi-schema/struct-base-clash.json
@@ -1,4 +1,5 @@
-# we check for no duplicate keys with base
+# Reject attempts to duplicate QMP members
+# Here, 'name' would have to appear twice on the wire, locally and for base.
{ 'struct': 'Base',
'data': { 'name': 'str' } }
{ 'struct': 'Sub',
diff --git a/tests/qapi-schema/union-clash-branches.err b/tests/qapi-schema/union-clash-branches.err
new file mode 100644
index 0000000..005c48d
--- /dev/null
+++ b/tests/qapi-schema/union-clash-branches.err
@@ -0,0 +1 @@
+tests/qapi-schema/union-clash-branches.json:4: Union 'TestUnion' member 'a_b' clashes with 'a-b'
diff --git a/tests/qapi-schema/union-clash-branches.exit b/tests/qapi-schema/union-clash-branches.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/union-clash-branches.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/union-clash-branches.json b/tests/qapi-schema/union-clash-branches.json
new file mode 100644
index 0000000..31d135f
--- /dev/null
+++ b/tests/qapi-schema/union-clash-branches.json
@@ -0,0 +1,5 @@
+# Union branch name collision
+# Reject a union that would result in a collision in generated C names (this
+# would try to generate two enum values 'TEST_UNION_KIND_A_B').
+{ 'union': 'TestUnion',
+ 'data': { 'a-b': 'int', 'a_b': 'str' } }
diff --git a/tests/qapi-schema/union-clash-branches.out b/tests/qapi-schema/union-clash-branches.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/union-clash-data.err b/tests/qapi-schema/union-clash-data.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/union-clash-data.exit b/tests/qapi-schema/union-clash-data.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/union-clash-data.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/union-clash-data.json b/tests/qapi-schema/union-clash-data.json
new file mode 100644
index 0000000..7308e69
--- /dev/null
+++ b/tests/qapi-schema/union-clash-data.json
@@ -0,0 +1,7 @@
+# Union branch 'data'
+# FIXME: this parses, but then fails to compile due to a duplicate 'data'
+# (one from the branch name, another as a filler to avoid an empty union).
+# we should either detect the collision at parse time, or change the
+# generated struct to allow this to compile.
+{ 'union': 'TestUnion',
+ 'data': { 'data': 'int' } }
diff --git a/tests/qapi-schema/union-clash-data.out b/tests/qapi-schema/union-clash-data.out
new file mode 100644
index 0000000..6277239
--- /dev/null
+++ b/tests/qapi-schema/union-clash-data.out
@@ -0,0 +1,6 @@
+object :empty
+object :obj-int-wrapper
+ member data: int optional=False
+object TestUnion
+ case data: :obj-int-wrapper
+enum TestUnionKind ['data']
diff --git a/tests/qapi-schema/union-clash-type.err b/tests/qapi-schema/union-clash-type.err
new file mode 100644
index 0000000..6e64d1d
--- /dev/null
+++ b/tests/qapi-schema/union-clash-type.err
@@ -0,0 +1,16 @@
+Traceback (most recent call last):
+ File "tests/qapi-schema/test-qapi.py", line 55, in <module>
+ schema = QAPISchema(sys.argv[1])
+ File "scripts/qapi.py", line 1116, in __init__
+ self.check()
+ File "scripts/qapi.py", line 1299, in check
+ ent.check(self)
+ File "scripts/qapi.py", line 962, in check
+ self.variants.check(schema, members, seen)
+ File "scripts/qapi.py", line 1024, in check
+ v.check(schema, self.tag_member.type, vseen)
+ File "scripts/qapi.py", line 1032, in check
+ QAPISchemaObjectTypeMember.check(self, schema, [], seen)
+ File "scripts/qapi.py", line 994, in check
+ assert self.name not in seen
+AssertionError
diff --git a/tests/qapi-schema/union-clash-type.exit b/tests/qapi-schema/union-clash-type.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/union-clash-type.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/union-clash-type.json b/tests/qapi-schema/union-clash-type.json
new file mode 100644
index 0000000..52c21f7
--- /dev/null
+++ b/tests/qapi-schema/union-clash-type.json
@@ -0,0 +1,10 @@
+# Union branch 'type'
+# FIXME: this triggers an assertion failure. But even with that fixed,
+# we would have a clash in generated C, between the simple union's
+# implicit tag member 'kind' and the branch name 'kind' within the
+# union. We should either reject this, or munge the generated C to let
+# it compile.
+# TODO: Even when the generated C is switched to use 'type' rather than
+# 'kind', to match the QMP spelling, the collision should still be detected.
+{ 'union': 'TestUnion',
+ 'data': { 'kind': 'int', 'type': 'str' } }
diff --git a/tests/qapi-schema/union-clash-type.out b/tests/qapi-schema/union-clash-type.out
new file mode 100644
index 0000000..e69de29
--
2.4.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PULL v2 09/20] qapi: Avoid assertion failure on union 'type' collision
2015-10-12 18:19 [Qemu-devel] [PULL v2 00/20] QAPI patches Markus Armbruster
` (7 preceding siblings ...)
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 08/20] qapi: Test for various name collisions Markus Armbruster
@ 2015-10-12 18:19 ` Markus Armbruster
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 10/20] qapi: Add tests for empty unions Markus Armbruster
` (11 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2015-10-12 18:19 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
The previous commit added two tests that triggered an assertion
failure. It's fairly straightforward to avoid the failure by
just outright forbidding the collision between a union's tag
values and its discriminator name (including the implicit name
'kind' supplied for simple unions [*]). Ultimately, we'd like
to move the collision detection into QAPISchema*.check(), but
for now it is easier just to enhance the existing checks.
[*] Of course, down the road, we have plans to rename the simple
union tag name to 'type' to match the QMP wire name, but the
idea of the collision will still be present even then.
Technically, we could avoid the collision by naming the C union
members representing each enum value as '_case_value' rather
than 'value'; but until we have an actual qapi client (and not
just our testsuite) that has a legitimate reason to match a
case label to the name of a QMP key and needs the name munging
to satisfy the compiler, it's easier to just reject the qapi
as invalid.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-7-git-send-email-eblake@redhat.com>
[Polished a few comments]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/qapi.py | 10 ++++++++--
tests/qapi-schema/flat-union-clash-type.err | 17 +----------------
tests/qapi-schema/flat-union-clash-type.json | 8 +++-----
tests/qapi-schema/union-clash-type.err | 17 +----------------
tests/qapi-schema/union-clash-type.json | 9 ++++-----
5 files changed, 17 insertions(+), 44 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 4b5d574..8d2681b 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -544,7 +544,7 @@ def check_union(expr, expr_info):
base = expr.get('base')
discriminator = expr.get('discriminator')
members = expr['data']
- values = {'MAX': '(automatic)'}
+ values = {'MAX': '(automatic)', 'KIND': '(automatic)'}
# Two types of unions, determined by discriminator.
@@ -603,13 +603,19 @@ def check_union(expr, expr_info):
" of branch '%s'" % key)
# If the discriminator names an enum type, then all members
- # of 'data' must also be members of the enum type.
+ # of 'data' must also be members of the enum type, which in turn
+ # must not collide with the discriminator name.
if enum_define:
if key not in enum_define['enum_values']:
raise QAPIExprError(expr_info,
"Discriminator value '%s' is not found in "
"enum '%s'" %
(key, enum_define["enum_name"]))
+ if discriminator in enum_define['enum_values']:
+ raise QAPIExprError(expr_info,
+ "Discriminator name '%s' collides with "
+ "enum value in '%s'" %
+ (discriminator, enum_define["enum_name"]))
# Otherwise, check for conflicts in the generated enum
else:
diff --git a/tests/qapi-schema/flat-union-clash-type.err b/tests/qapi-schema/flat-union-clash-type.err
index 6e64d1d..b44dd40 100644
--- a/tests/qapi-schema/flat-union-clash-type.err
+++ b/tests/qapi-schema/flat-union-clash-type.err
@@ -1,16 +1 @@
-Traceback (most recent call last):
- File "tests/qapi-schema/test-qapi.py", line 55, in <module>
- schema = QAPISchema(sys.argv[1])
- File "scripts/qapi.py", line 1116, in __init__
- self.check()
- File "scripts/qapi.py", line 1299, in check
- ent.check(self)
- File "scripts/qapi.py", line 962, in check
- self.variants.check(schema, members, seen)
- File "scripts/qapi.py", line 1024, in check
- v.check(schema, self.tag_member.type, vseen)
- File "scripts/qapi.py", line 1032, in check
- QAPISchemaObjectTypeMember.check(self, schema, [], seen)
- File "scripts/qapi.py", line 994, in check
- assert self.name not in seen
-AssertionError
+tests/qapi-schema/flat-union-clash-type.json:11: Discriminator name 'type' collides with enum value in 'TestEnum'
diff --git a/tests/qapi-schema/flat-union-clash-type.json b/tests/qapi-schema/flat-union-clash-type.json
index 3db6ea0..8f710f0 100644
--- a/tests/qapi-schema/flat-union-clash-type.json
+++ b/tests/qapi-schema/flat-union-clash-type.json
@@ -1,9 +1,7 @@
# Flat union branch 'type'
-# FIXME: this triggers an assertion failure. But even with that fixed,
-# we would have a clash in generated C, between the member 'type'
-# inherited from 'Base' and the branch name 'type' within the
-# union. We should either reject this, or munge the generated C to let
-# it compile.
+# Reject this, because we would have a clash in generated C, between the
+# outer tag 'type' and the branch name 'type' within the union.
+# TODO: We could munge the generated C branch name to let it compile.
{ 'enum': 'TestEnum',
'data': [ 'type' ] }
{ 'struct': 'Base',
diff --git a/tests/qapi-schema/union-clash-type.err b/tests/qapi-schema/union-clash-type.err
index 6e64d1d..a5dead1 100644
--- a/tests/qapi-schema/union-clash-type.err
+++ b/tests/qapi-schema/union-clash-type.err
@@ -1,16 +1 @@
-Traceback (most recent call last):
- File "tests/qapi-schema/test-qapi.py", line 55, in <module>
- schema = QAPISchema(sys.argv[1])
- File "scripts/qapi.py", line 1116, in __init__
- self.check()
- File "scripts/qapi.py", line 1299, in check
- ent.check(self)
- File "scripts/qapi.py", line 962, in check
- self.variants.check(schema, members, seen)
- File "scripts/qapi.py", line 1024, in check
- v.check(schema, self.tag_member.type, vseen)
- File "scripts/qapi.py", line 1032, in check
- QAPISchemaObjectTypeMember.check(self, schema, [], seen)
- File "scripts/qapi.py", line 994, in check
- assert self.name not in seen
-AssertionError
+tests/qapi-schema/union-clash-type.json:8: Union 'TestUnion' member 'kind' clashes with '(automatic)'
diff --git a/tests/qapi-schema/union-clash-type.json b/tests/qapi-schema/union-clash-type.json
index 52c21f7..cfc256b 100644
--- a/tests/qapi-schema/union-clash-type.json
+++ b/tests/qapi-schema/union-clash-type.json
@@ -1,10 +1,9 @@
# Union branch 'type'
-# FIXME: this triggers an assertion failure. But even with that fixed,
-# we would have a clash in generated C, between the simple union's
-# implicit tag member 'kind' and the branch name 'kind' within the
-# union. We should either reject this, or munge the generated C to let
-# it compile.
+# Reject this, because we would have a clash in generated C, between the
+# simple union's implicit tag member 'kind' and the branch name 'kind'
+# within the union.
# TODO: Even when the generated C is switched to use 'type' rather than
# 'kind', to match the QMP spelling, the collision should still be detected.
+# Or, we could munge the branch name to allow compilation.
{ 'union': 'TestUnion',
'data': { 'kind': 'int', 'type': 'str' } }
--
2.4.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PULL v2 10/20] qapi: Add tests for empty unions
2015-10-12 18:19 [Qemu-devel] [PULL v2 00/20] QAPI patches Markus Armbruster
` (8 preceding siblings ...)
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 09/20] qapi: Avoid assertion failure on union 'type' collision Markus Armbruster
@ 2015-10-12 18:19 ` Markus Armbruster
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 11/20] qapi: Test use of 'number' within alternates Markus Armbruster
` (10 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2015-10-12 18:19 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
The documentation claims that alternates are useful for
allowing two or more types, although nothing enforces this.
Meanwhile, it is silent on whether empty unions are allowed.
In practice, the generated code will compile, in part because
we have a 'void *data' branch; but attempting to visit such a
type will cause an abort(). While there's no technical reason
that a degenerate union could not be made to work, it's harder
to justify the time spent in chasing known (the current
abort() during visit) and unknown corner cases, than it would
be to just outlaw them. A future patch will probably take the
approach of forbidding them; in the meantime, we can at least
add testsuite coverage to make it obvious where things stand.
In addition to adding tests to expose the problems, we also
need to adjust existing tests that are meant to test something
else, but which could fail for the wrong reason if we reject
degenerate alternates/unions.
Note that empty structs are explicitly supported (for example,
right now they are the only way to specify that one branch of a
flat union adds no additional members), and empty enums are
covered by the testsuite as working (even if they do not seem
to have much use).
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-8-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/Makefile | 3 +++
tests/qapi-schema/alternate-empty.err | 0
tests/qapi-schema/alternate-empty.exit | 1 +
tests/qapi-schema/alternate-empty.json | 2 ++
tests/qapi-schema/alternate-empty.out | 4 ++++
tests/qapi-schema/alternate-nested.json | 2 +-
tests/qapi-schema/alternate-unknown.json | 2 +-
tests/qapi-schema/flat-union-empty.err | 0
tests/qapi-schema/flat-union-empty.exit | 1 +
tests/qapi-schema/flat-union-empty.json | 4 ++++
tests/qapi-schema/flat-union-empty.out | 7 +++++++
tests/qapi-schema/union-empty.err | 0
tests/qapi-schema/union-empty.exit | 1 +
tests/qapi-schema/union-empty.json | 2 ++
tests/qapi-schema/union-empty.out | 3 +++
15 files changed, 30 insertions(+), 2 deletions(-)
create mode 100644 tests/qapi-schema/alternate-empty.err
create mode 100644 tests/qapi-schema/alternate-empty.exit
create mode 100644 tests/qapi-schema/alternate-empty.json
create mode 100644 tests/qapi-schema/alternate-empty.out
create mode 100644 tests/qapi-schema/flat-union-empty.err
create mode 100644 tests/qapi-schema/flat-union-empty.exit
create mode 100644 tests/qapi-schema/flat-union-empty.json
create mode 100644 tests/qapi-schema/flat-union-empty.out
create mode 100644 tests/qapi-schema/union-empty.err
create mode 100644 tests/qapi-schema/union-empty.exit
create mode 100644 tests/qapi-schema/union-empty.json
create mode 100644 tests/qapi-schema/union-empty.out
diff --git a/tests/Makefile b/tests/Makefile
index 49fdbe2..209eca9 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -229,6 +229,7 @@ qapi-schema += alternate-base.json
qapi-schema += alternate-clash.json
qapi-schema += alternate-conflict-dict.json
qapi-schema += alternate-conflict-string.json
+qapi-schema += alternate-empty.json
qapi-schema += alternate-good.json
qapi-schema += alternate-nested.json
qapi-schema += alternate-unknown.json
@@ -280,6 +281,7 @@ qapi-schema += flat-union-base-union.json
qapi-schema += flat-union-clash-branch.json
qapi-schema += flat-union-clash-member.json
qapi-schema += flat-union-clash-type.json
+qapi-schema += flat-union-empty.json
qapi-schema += flat-union-inline.json
qapi-schema += flat-union-int-branch.json
qapi-schema += flat-union-invalid-branch-key.json
@@ -338,6 +340,7 @@ qapi-schema += union-base-no-discriminator.json
qapi-schema += union-clash-branches.json
qapi-schema += union-clash-data.json
qapi-schema += union-clash-type.json
+qapi-schema += union-empty.json
qapi-schema += union-invalid-base.json
qapi-schema += union-max.json
qapi-schema += union-optional-branch.json
diff --git a/tests/qapi-schema/alternate-empty.err b/tests/qapi-schema/alternate-empty.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/alternate-empty.exit b/tests/qapi-schema/alternate-empty.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/alternate-empty.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/alternate-empty.json b/tests/qapi-schema/alternate-empty.json
new file mode 100644
index 0000000..db3820f
--- /dev/null
+++ b/tests/qapi-schema/alternate-empty.json
@@ -0,0 +1,2 @@
+# FIXME - alternates should list at least two types to be useful
+{ 'alternate': 'Alt', 'data': { 'i': 'int' } }
diff --git a/tests/qapi-schema/alternate-empty.out b/tests/qapi-schema/alternate-empty.out
new file mode 100644
index 0000000..0f153b6
--- /dev/null
+++ b/tests/qapi-schema/alternate-empty.out
@@ -0,0 +1,4 @@
+object :empty
+alternate Alt
+ case i: int
+enum AltKind ['i']
diff --git a/tests/qapi-schema/alternate-nested.json b/tests/qapi-schema/alternate-nested.json
index c4233b9..8e22186 100644
--- a/tests/qapi-schema/alternate-nested.json
+++ b/tests/qapi-schema/alternate-nested.json
@@ -2,4 +2,4 @@
{ 'alternate': 'Alt1',
'data': { 'name': 'str', 'value': 'int' } }
{ 'alternate': 'Alt2',
- 'data': { 'nested': 'Alt1' } }
+ 'data': { 'nested': 'Alt1', 'b': 'bool' } }
diff --git a/tests/qapi-schema/alternate-unknown.json b/tests/qapi-schema/alternate-unknown.json
index ad5c103..08c80dc 100644
--- a/tests/qapi-schema/alternate-unknown.json
+++ b/tests/qapi-schema/alternate-unknown.json
@@ -1,3 +1,3 @@
# we reject an alternate with unknown type in branch
{ 'alternate': 'Alt',
- 'data': { 'unknown': 'MissingType' } }
+ 'data': { 'unknown': 'MissingType', 'i': 'int' } }
diff --git a/tests/qapi-schema/flat-union-empty.err b/tests/qapi-schema/flat-union-empty.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/flat-union-empty.exit b/tests/qapi-schema/flat-union-empty.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/flat-union-empty.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/flat-union-empty.json b/tests/qapi-schema/flat-union-empty.json
new file mode 100644
index 0000000..67dd297
--- /dev/null
+++ b/tests/qapi-schema/flat-union-empty.json
@@ -0,0 +1,4 @@
+# FIXME - flat unions should not be empty
+{ 'enum': 'Empty', 'data': [ ] }
+{ 'struct': 'Base', 'data': { 'type': 'Empty' } }
+{ 'union': 'Union', 'base': 'Base', 'discriminator': 'type', 'data': { } }
diff --git a/tests/qapi-schema/flat-union-empty.out b/tests/qapi-schema/flat-union-empty.out
new file mode 100644
index 0000000..0e0665a
--- /dev/null
+++ b/tests/qapi-schema/flat-union-empty.out
@@ -0,0 +1,7 @@
+object :empty
+object Base
+ member type: Empty optional=False
+enum Empty []
+object Union
+ base Base
+ tag type
diff --git a/tests/qapi-schema/union-empty.err b/tests/qapi-schema/union-empty.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/union-empty.exit b/tests/qapi-schema/union-empty.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/union-empty.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/union-empty.json b/tests/qapi-schema/union-empty.json
new file mode 100644
index 0000000..1785007
--- /dev/null
+++ b/tests/qapi-schema/union-empty.json
@@ -0,0 +1,2 @@
+# FIXME - unions should not be empty
+{ 'union': 'Union', 'data': { } }
diff --git a/tests/qapi-schema/union-empty.out b/tests/qapi-schema/union-empty.out
new file mode 100644
index 0000000..8b5a7bf
--- /dev/null
+++ b/tests/qapi-schema/union-empty.out
@@ -0,0 +1,3 @@
+object :empty
+object Union
+enum UnionKind []
--
2.4.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PULL v2 11/20] qapi: Test use of 'number' within alternates
2015-10-12 18:19 [Qemu-devel] [PULL v2 00/20] QAPI patches Markus Armbruster
` (9 preceding siblings ...)
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 10/20] qapi: Add tests for empty unions Markus Armbruster
@ 2015-10-12 18:19 ` Markus Armbruster
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 12/20] qapi: Reuse code for flat union base validation Markus Armbruster
` (9 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2015-10-12 18:19 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
Add some testsuite exposure for use of a 'number' as part of
an alternate. The current state of the tree has a few bugs
exposed by this: our input parser depends on the ordering of
how the qapi schema declared the alternate, and the parser
does not accept integers for a 'number' in an alternate even
though it does for numbers outside of an alternate.
Mixing 'int' and 'number' in the same alternate is unusual,
since both are supplied by json-numbers, but there does not
seem to be a technical reason to forbid it given that our
json lexer distinguishes between json-numbers that can be
represented as an int vs. those that cannot.
Improve the existing test_visitor_in_alternate() to match the
style of the new test_visitor_in_alternate_number(), and to
ensure full coverage of all possible qtype parsing.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-9-git-send-email-eblake@redhat.com>
[Eric's follow-up fixes squashed in]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/qapi-schema/qapi-schema-test.json | 8 ++
tests/qapi-schema/qapi-schema-test.out | 24 ++++++
tests/test-qmp-input-visitor.c | 131 +++++++++++++++++++++++++++++++-
3 files changed, 160 insertions(+), 3 deletions(-)
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index c511338..abe59fd 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -67,6 +67,14 @@
{ 'struct': 'UserDefC',
'data': { 'string1': 'str', 'string2': 'str' } }
+# for testing use of 'number' within alternates
+{ 'alternate': 'AltStrBool', 'data': { 's': 'str', 'b': 'bool' } }
+{ 'alternate': 'AltStrNum', 'data': { 's': 'str', 'n': 'number' } }
+{ 'alternate': 'AltNumStr', 'data': { 'n': 'number', 's': 'str' } }
+{ 'alternate': 'AltStrInt', 'data': { 's': 'str', 'i': 'int' } }
+{ 'alternate': 'AltIntNum', 'data': { 'i': 'int', 'n': 'number' } }
+{ 'alternate': 'AltNumInt', 'data': { 'n': 'number', 'i': 'int' } }
+
# for testing native lists
{ 'union': 'UserDefNativeListUnion',
'data': { 'integer': ['int'],
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 28a0b3c..8f81784 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -53,6 +53,30 @@ object :obj-user_def_cmd2-arg
object :obj-user_def_cmd3-arg
member a: int optional=False
member b: int optional=True
+alternate AltIntNum
+ case i: int
+ case n: number
+enum AltIntNumKind ['i', 'n']
+alternate AltNumInt
+ case n: number
+ case i: int
+enum AltNumIntKind ['n', 'i']
+alternate AltNumStr
+ case n: number
+ case s: str
+enum AltNumStrKind ['n', 's']
+alternate AltStrBool
+ case s: str
+ case b: bool
+enum AltStrBoolKind ['s', 'b']
+alternate AltStrInt
+ case s: str
+ case i: int
+enum AltStrIntKind ['s', 'i']
+alternate AltStrNum
+ case s: str
+ case n: number
+enum AltStrNumKind ['s', 'n']
event EVENT_A None
event EVENT_B None
event EVENT_C :obj-EVENT_C-arg
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 61715b3..8941963 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -371,12 +371,135 @@ static void test_visitor_in_alternate(TestInputVisitorData *data,
UserDefAlternate *tmp;
v = visitor_input_test_init(data, "42");
-
- visit_type_UserDefAlternate(v, &tmp, NULL, &err);
- g_assert(err == NULL);
+ visit_type_UserDefAlternate(v, &tmp, NULL, &error_abort);
g_assert_cmpint(tmp->kind, ==, USER_DEF_ALTERNATE_KIND_I);
g_assert_cmpint(tmp->i, ==, 42);
qapi_free_UserDefAlternate(tmp);
+ visitor_input_teardown(data, NULL);
+
+ v = visitor_input_test_init(data, "'string'");
+ visit_type_UserDefAlternate(v, &tmp, NULL, &error_abort);
+ g_assert_cmpint(tmp->kind, ==, USER_DEF_ALTERNATE_KIND_S);
+ g_assert_cmpstr(tmp->s, ==, "string");
+ qapi_free_UserDefAlternate(tmp);
+ visitor_input_teardown(data, NULL);
+
+ v = visitor_input_test_init(data, "false");
+ visit_type_UserDefAlternate(v, &tmp, NULL, &err);
+ g_assert(err);
+ error_free(err);
+ err = NULL;
+ qapi_free_UserDefAlternate(tmp);
+ visitor_input_teardown(data, NULL);
+}
+
+static void test_visitor_in_alternate_number(TestInputVisitorData *data,
+ const void *unused)
+{
+ Visitor *v;
+ Error *err = NULL;
+ AltStrBool *asb;
+ AltStrNum *asn;
+ AltNumStr *ans;
+ AltStrInt *asi;
+ AltIntNum *ain;
+ AltNumInt *ani;
+
+ /* Parsing an int */
+
+ v = visitor_input_test_init(data, "42");
+ visit_type_AltStrBool(v, &asb, NULL, &err);
+ g_assert(err);
+ error_free(err);
+ err = NULL;
+ qapi_free_AltStrBool(asb);
+ visitor_input_teardown(data, NULL);
+
+ /* FIXME: Order of alternate should not affect semantics; asn should
+ * parse the same as ans */
+ v = visitor_input_test_init(data, "42");
+ visit_type_AltStrNum(v, &asn, NULL, &err);
+ /* FIXME g_assert_cmpint(asn->kind, == ALT_STR_NUM_KIND_N); */
+ /* FIXME g_assert_cmpfloat(asn->n, ==, 42); */
+ g_assert(err);
+ error_free(err);
+ err = NULL;
+ qapi_free_AltStrNum(asn);
+ visitor_input_teardown(data, NULL);
+
+ v = visitor_input_test_init(data, "42");
+ visit_type_AltNumStr(v, &ans, NULL, &error_abort);
+ g_assert_cmpint(ans->kind, ==, ALT_NUM_STR_KIND_N);
+ g_assert_cmpfloat(ans->n, ==, 42);
+ qapi_free_AltNumStr(ans);
+ visitor_input_teardown(data, NULL);
+
+ v = visitor_input_test_init(data, "42");
+ visit_type_AltStrInt(v, &asi, NULL, &error_abort);
+ g_assert_cmpint(asi->kind, ==, ALT_STR_INT_KIND_I);
+ g_assert_cmpint(asi->i, ==, 42);
+ qapi_free_AltStrInt(asi);
+ visitor_input_teardown(data, NULL);
+
+ v = visitor_input_test_init(data, "42");
+ visit_type_AltIntNum(v, &ain, NULL, &error_abort);
+ g_assert_cmpint(ain->kind, ==, ALT_INT_NUM_KIND_I);
+ g_assert_cmpint(ain->i, ==, 42);
+ qapi_free_AltIntNum(ain);
+ visitor_input_teardown(data, NULL);
+
+ v = visitor_input_test_init(data, "42");
+ visit_type_AltNumInt(v, &ani, NULL, &error_abort);
+ g_assert_cmpint(ani->kind, ==, ALT_NUM_INT_KIND_I);
+ g_assert_cmpint(ani->i, ==, 42);
+ qapi_free_AltNumInt(ani);
+ visitor_input_teardown(data, NULL);
+
+ /* Parsing a double */
+
+ v = visitor_input_test_init(data, "42.5");
+ visit_type_AltStrBool(v, &asb, NULL, &err);
+ g_assert(err);
+ error_free(err);
+ err = NULL;
+ qapi_free_AltStrBool(asb);
+ visitor_input_teardown(data, NULL);
+
+ v = visitor_input_test_init(data, "42.5");
+ visit_type_AltStrNum(v, &asn, NULL, &error_abort);
+ g_assert_cmpint(asn->kind, ==, ALT_STR_NUM_KIND_N);
+ g_assert_cmpfloat(asn->n, ==, 42.5);
+ qapi_free_AltStrNum(asn);
+ visitor_input_teardown(data, NULL);
+
+ v = visitor_input_test_init(data, "42.5");
+ visit_type_AltNumStr(v, &ans, NULL, &error_abort);
+ g_assert_cmpint(ans->kind, ==, ALT_NUM_STR_KIND_N);
+ g_assert_cmpfloat(ans->n, ==, 42.5);
+ qapi_free_AltNumStr(ans);
+ visitor_input_teardown(data, NULL);
+
+ v = visitor_input_test_init(data, "42.5");
+ visit_type_AltStrInt(v, &asi, NULL, &err);
+ g_assert(err);
+ error_free(err);
+ err = NULL;
+ qapi_free_AltStrInt(asi);
+ visitor_input_teardown(data, NULL);
+
+ v = visitor_input_test_init(data, "42.5");
+ visit_type_AltIntNum(v, &ain, NULL, &error_abort);
+ g_assert_cmpint(ain->kind, ==, ALT_INT_NUM_KIND_N);
+ g_assert_cmpfloat(ain->n, ==, 42.5);
+ qapi_free_AltIntNum(ain);
+ visitor_input_teardown(data, NULL);
+
+ v = visitor_input_test_init(data, "42.5");
+ visit_type_AltNumInt(v, &ani, NULL, &error_abort);
+ g_assert_cmpint(ani->kind, ==, ALT_NUM_INT_KIND_N);
+ g_assert_cmpfloat(ani->n, ==, 42.5);
+ qapi_free_AltNumInt(ani);
+ visitor_input_teardown(data, NULL);
}
static void test_native_list_integer_helper(TestInputVisitorData *data,
@@ -720,6 +843,8 @@ int main(int argc, char **argv)
&in_visitor_data, test_visitor_in_alternate);
input_visitor_test_add("/visitor/input/errors",
&in_visitor_data, test_visitor_in_errors);
+ input_visitor_test_add("/visitor/input/alternate-number",
+ &in_visitor_data, test_visitor_in_alternate_number);
input_visitor_test_add("/visitor/input/native_list/int",
&in_visitor_data,
test_visitor_in_native_list_int);
--
2.4.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PULL v2 12/20] qapi: Reuse code for flat union base validation
2015-10-12 18:19 [Qemu-devel] [PULL v2 00/20] QAPI patches Markus Armbruster
` (10 preceding siblings ...)
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 11/20] qapi: Test use of 'number' within alternates Markus Armbruster
@ 2015-10-12 18:19 ` Markus Armbruster
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 13/20] qapi: Consistent generated code: prefer error 'err' Markus Armbruster
` (8 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2015-10-12 18:19 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
Rather than open-code the check for a valid base type, we
should reuse the common functionality. This allows for
consistent error messages, and also makes it easier for a
later patch to turn on support for inline anonymous base
structures.
Test flat-union-inline is updated to test only one feature
(anonymous branch dictionaries), which can be implemented
independently (test flat-union-bad-base already covers the
idea of an anonymous base dictionary).
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-10-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/qapi.py | 11 +++++------
tests/qapi-schema/flat-union-bad-base.err | 2 +-
tests/qapi-schema/flat-union-base-any.err | 2 +-
tests/qapi-schema/flat-union-base-union.err | 2 +-
tests/qapi-schema/flat-union-inline.err | 2 +-
tests/qapi-schema/flat-union-inline.json | 4 ++--
tests/qapi-schema/flat-union-no-base.err | 2 +-
tests/qapi-schema/union-invalid-base.err | 2 +-
8 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 8d2681b..c0728d7 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -560,15 +560,14 @@ def check_union(expr, expr_info):
# Else, it's a flat union.
else:
# The object must have a string member 'base'.
- if not isinstance(base, str):
+ check_type(expr_info, "'base' for union '%s'" % name,
+ base, allow_metas=['struct'])
+ if not base:
raise QAPIExprError(expr_info,
- "Flat union '%s' must have a string base field"
+ "Flat union '%s' must have a base"
% name)
base_fields = find_base_fields(base)
- if not base_fields:
- raise QAPIExprError(expr_info,
- "Base '%s' is not a valid struct"
- % base)
+ assert base_fields
# The value of member 'discriminator' must name a non-optional
# member of the base struct.
diff --git a/tests/qapi-schema/flat-union-bad-base.err b/tests/qapi-schema/flat-union-bad-base.err
index f9c31b2..79b8a71 100644
--- a/tests/qapi-schema/flat-union-bad-base.err
+++ b/tests/qapi-schema/flat-union-bad-base.err
@@ -1 +1 @@
-tests/qapi-schema/flat-union-bad-base.json:9: Flat union 'TestUnion' must have a string base field
+tests/qapi-schema/flat-union-bad-base.json:9: 'base' for union 'TestUnion' should be a type name
diff --git a/tests/qapi-schema/flat-union-base-any.err b/tests/qapi-schema/flat-union-base-any.err
index ad4d629..646f1c9 100644
--- a/tests/qapi-schema/flat-union-base-any.err
+++ b/tests/qapi-schema/flat-union-base-any.err
@@ -1 +1 @@
-tests/qapi-schema/flat-union-base-any.json:8: Base 'any' is not a valid struct
+tests/qapi-schema/flat-union-base-any.json:8: 'base' for union 'TestUnion' cannot use built-in type 'any'
diff --git a/tests/qapi-schema/flat-union-base-union.err b/tests/qapi-schema/flat-union-base-union.err
index 28725ed..f138395 100644
--- a/tests/qapi-schema/flat-union-base-union.err
+++ b/tests/qapi-schema/flat-union-base-union.err
@@ -1 +1 @@
-tests/qapi-schema/flat-union-base-union.json:14: Base 'UnionBase' is not a valid struct
+tests/qapi-schema/flat-union-base-union.json:14: 'base' for union 'TestUnion' cannot use union type 'UnionBase'
diff --git a/tests/qapi-schema/flat-union-inline.err b/tests/qapi-schema/flat-union-inline.err
index ec58627..2333358 100644
--- a/tests/qapi-schema/flat-union-inline.err
+++ b/tests/qapi-schema/flat-union-inline.err
@@ -1 +1 @@
-tests/qapi-schema/flat-union-inline.json:7: Flat union 'TestUnion' must have a string base field
+tests/qapi-schema/flat-union-inline.json:7: Member 'value1' of union 'TestUnion' should be a type name
diff --git a/tests/qapi-schema/flat-union-inline.json b/tests/qapi-schema/flat-union-inline.json
index 6bfdd65..62c7cda 100644
--- a/tests/qapi-schema/flat-union-inline.json
+++ b/tests/qapi-schema/flat-union-inline.json
@@ -1,11 +1,11 @@
# we require branches to be a struct name
-# TODO: should we allow anonymous inline types?
+# TODO: should we allow anonymous inline branch types?
{ 'enum': 'TestEnum',
'data': [ 'value1', 'value2' ] }
{ 'struct': 'Base',
'data': { 'enum1': 'TestEnum', 'kind': 'str' } }
{ 'union': 'TestUnion',
- 'base': { 'enum1': 'TestEnum', 'kind': 'str' },
+ 'base': 'Base',
'discriminator': 'enum1',
'data': { 'value1': { 'string': 'str' },
'value2': { 'integer': 'int' } } }
diff --git a/tests/qapi-schema/flat-union-no-base.err b/tests/qapi-schema/flat-union-no-base.err
index bb3f708..841c93b 100644
--- a/tests/qapi-schema/flat-union-no-base.err
+++ b/tests/qapi-schema/flat-union-no-base.err
@@ -1 +1 @@
-tests/qapi-schema/flat-union-no-base.json:9: Flat union 'TestUnion' must have a string base field
+tests/qapi-schema/flat-union-no-base.json:9: Flat union 'TestUnion' must have a base
diff --git a/tests/qapi-schema/union-invalid-base.err b/tests/qapi-schema/union-invalid-base.err
index 9f63796..03d7b97 100644
--- a/tests/qapi-schema/union-invalid-base.err
+++ b/tests/qapi-schema/union-invalid-base.err
@@ -1 +1 @@
-tests/qapi-schema/union-invalid-base.json:8: Base 'int' is not a valid struct
+tests/qapi-schema/union-invalid-base.json:8: 'base' for union 'TestUnion' cannot use built-in type 'int'
--
2.4.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PULL v2 13/20] qapi: Consistent generated code: prefer error 'err'
2015-10-12 18:19 [Qemu-devel] [PULL v2 00/20] QAPI patches Markus Armbruster
` (11 preceding siblings ...)
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 12/20] qapi: Reuse code for flat union base validation Markus Armbruster
@ 2015-10-12 18:19 ` Markus Armbruster
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 14/20] qapi: Consistent generated code: prefer visitor 'v' Markus Armbruster
` (7 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2015-10-12 18:19 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
We had some pointless differences in the generated code for visit,
command marshalling, and events; unifying them makes it easier for
future patches to consolidate to common helper functions.
This is one patch of a series to clean up these differences.
This patch consistently names the local error variable 'err' rather
than 'local_err'.
No change in semantics to the generated code.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-11-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
docs/qapi-code-gen.txt | 28 ++++++++++++++--------------
scripts/qapi-commands.py | 22 +++++++++++-----------
scripts/qapi-event.py | 18 +++++++++---------
3 files changed, 34 insertions(+), 34 deletions(-)
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index b1c8361..b99e6fa 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -916,20 +916,20 @@ Example:
static void qmp_marshal_output_UserDefOne(UserDefOne *ret_in, QObject **ret_out, Error **errp)
{
- Error *local_err = NULL;
+ Error *err = NULL;
QmpOutputVisitor *mo = qmp_output_visitor_new();
QapiDeallocVisitor *md;
Visitor *v;
v = qmp_output_get_visitor(mo);
- visit_type_UserDefOne(v, &ret_in, "unused", &local_err);
- if (local_err) {
+ visit_type_UserDefOne(v, &ret_in, "unused", &err);
+ if (err) {
goto out;
}
*ret_out = qmp_output_get_qobject(mo);
out:
- error_propagate(errp, local_err);
+ error_propagate(errp, err);
qmp_output_visitor_cleanup(mo);
md = qapi_dealloc_visitor_new();
v = qapi_dealloc_get_visitor(md);
@@ -939,7 +939,7 @@ Example:
static void qmp_marshal_my_command(QDict *args, QObject **ret, Error **errp)
{
- Error *local_err = NULL;
+ Error *err = NULL;
UserDefOne *retval;
QmpInputVisitor *mi = qmp_input_visitor_new_strict(QOBJECT(args));
QapiDeallocVisitor *md;
@@ -947,20 +947,20 @@ Example:
UserDefOne *arg1 = NULL;
v = qmp_input_get_visitor(mi);
- visit_type_UserDefOne(v, &arg1, "arg1", &local_err);
- if (local_err) {
+ visit_type_UserDefOne(v, &arg1, "arg1", &err);
+ if (err) {
goto out;
}
- retval = qmp_my_command(arg1, &local_err);
- if (local_err) {
+ retval = qmp_my_command(arg1, &err);
+ if (err) {
goto out;
}
- qmp_marshal_output_UserDefOne(retval, ret, &local_err);
+ qmp_marshal_output_UserDefOne(retval, ret, &err);
out:
- error_propagate(errp, local_err);
+ error_propagate(errp, err);
qmp_input_visitor_cleanup(mi);
md = qapi_dealloc_visitor_new();
v = qapi_dealloc_get_visitor(md);
@@ -1007,7 +1007,7 @@ Example:
void qapi_event_send_my_event(Error **errp)
{
QDict *qmp;
- Error *local_err = NULL;
+ Error *err = NULL;
QMPEventFuncEmit emit;
emit = qmp_event_get_func_emit();
if (!emit) {
@@ -1016,9 +1016,9 @@ Example:
qmp = qmp_event_build_dict("MY_EVENT");
- emit(EXAMPLE_QAPI_EVENT_MY_EVENT, qmp, &local_err);
+ emit(EXAMPLE_QAPI_EVENT_MY_EVENT, qmp, &err);
- error_propagate(errp, local_err);
+ error_propagate(errp, err);
QDECREF(qmp);
}
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 810a897..2151120 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -53,14 +53,14 @@ def gen_call(name, arg_type, ret_type):
push_indent()
ret = mcgen('''
-%(lhs)sqmp_%(c_name)s(%(args)s&local_err);
+%(lhs)sqmp_%(c_name)s(%(args)s&err);
''',
c_name=c_name(name), args=argstr, lhs=lhs)
if ret_type:
- ret += gen_err_check('local_err')
+ ret += gen_err_check('err')
ret += mcgen('''
-qmp_marshal_output_%(c_name)s(retval, ret, &local_err);
+qmp_marshal_output_%(c_name)s(retval, ret, &err);
''',
c_name=ret_type.c_name())
pop_indent()
@@ -69,7 +69,7 @@ qmp_marshal_output_%(c_name)s(retval, ret, &local_err);
def gen_marshal_vars(arg_type, ret_type):
ret = mcgen('''
- Error *local_err = NULL;
+ Error *err = NULL;
''')
push_indent()
@@ -127,8 +127,8 @@ md = qapi_dealloc_visitor_new();
v = qapi_dealloc_get_visitor(md);
''')
else:
- errparg = '&local_err'
- errarg = 'local_err'
+ errparg = '&err'
+ errarg = 'err'
ret += mcgen('''
v = qmp_input_get_visitor(mi);
''')
@@ -171,20 +171,20 @@ def gen_marshal_output(ret_type):
static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out, Error **errp)
{
- Error *local_err = NULL;
+ Error *err = NULL;
QmpOutputVisitor *mo = qmp_output_visitor_new();
QapiDeallocVisitor *md;
Visitor *v;
v = qmp_output_get_visitor(mo);
- visit_type_%(c_name)s(v, &ret_in, "unused", &local_err);
- if (local_err) {
+ visit_type_%(c_name)s(v, &ret_in, "unused", &err);
+ if (err) {
goto out;
}
*ret_out = qmp_output_get_qobject(mo);
out:
- error_propagate(errp, local_err);
+ error_propagate(errp, err);
qmp_output_visitor_cleanup(mo);
md = qapi_dealloc_visitor_new();
v = qapi_dealloc_get_visitor(md);
@@ -227,7 +227,7 @@ def gen_marshal(name, arg_type, ret_type):
out:
''')
ret += mcgen('''
- error_propagate(errp, local_err);
+ error_propagate(errp, err);
''')
ret += gen_marshal_input_visit(arg_type, dealloc=True)
ret += mcgen('''
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index d15fad9..d41af40 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -34,7 +34,7 @@ def gen_event_send(name, arg_type):
%(proto)s
{
QDict *qmp;
- Error *local_err = NULL;
+ Error *err = NULL;
QMPEventFuncEmit emit;
''',
proto=gen_event_send_proto(name, arg_type))
@@ -67,8 +67,8 @@ def gen_event_send(name, arg_type):
g_assert(v);
/* Fake visit, as if all members are under a structure */
- visit_start_struct(v, NULL, "", "%(name)s", 0, &local_err);
- if (local_err) {
+ visit_start_struct(v, NULL, "", "%(name)s", 0, &err);
+ if (err) {
goto clean;
}
@@ -90,8 +90,8 @@ def gen_event_send(name, arg_type):
cast = ''
ret += mcgen('''
- visit_type_%(c_type)s(v, %(cast)s&%(c_name)s, "%(name)s", &local_err);
- if (local_err) {
+ visit_type_%(c_type)s(v, %(cast)s&%(c_name)s, "%(name)s", &err);
+ if (err) {
goto clean;
}
''',
@@ -108,8 +108,8 @@ def gen_event_send(name, arg_type):
ret += mcgen('''
- visit_end_struct(v, &local_err);
- if (local_err) {
+ visit_end_struct(v, &err);
+ if (err) {
goto clean;
}
@@ -120,7 +120,7 @@ def gen_event_send(name, arg_type):
''')
ret += mcgen('''
- emit(%(c_enum)s, qmp, &local_err);
+ emit(%(c_enum)s, qmp, &err);
''',
c_enum=c_enum_const(event_enum_name, name))
@@ -131,7 +131,7 @@ def gen_event_send(name, arg_type):
qmp_output_visitor_cleanup(qov);
''')
ret += mcgen('''
- error_propagate(errp, local_err);
+ error_propagate(errp, err);
QDECREF(qmp);
}
''')
--
2.4.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PULL v2 14/20] qapi: Consistent generated code: prefer visitor 'v'
2015-10-12 18:19 [Qemu-devel] [PULL v2 00/20] QAPI patches Markus Armbruster
` (12 preceding siblings ...)
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 13/20] qapi: Consistent generated code: prefer error 'err' Markus Armbruster
@ 2015-10-12 18:19 ` Markus Armbruster
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 15/20] qapi: Consistent generated code: prefer common labels Markus Armbruster
` (6 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2015-10-12 18:19 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
We had some pointless differences in the generated code for visit,
command marshalling, and events; unifying them makes it easier for
future patches to consolidate to common helper functions.
This is one patch of a series to clean up these differences.
This patch names the local visitor variable 'v' rather than 'm'.
Related objects, such as 'QapiDeallocVisitor', are also named by
their initials instead of an unrelated leading m.
No change in semantics to the generated code.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-12-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
docs/qapi-code-gen.txt | 74 ++++++++++++++++++++++++------------------------
qom/object.c | 18 ++++++------
qom/qom-qobject.c | 18 ++++++------
scripts/qapi-commands.py | 30 ++++++++++----------
scripts/qapi-types.py | 8 +++---
scripts/qapi-visit.py | 70 ++++++++++++++++++++++-----------------------
6 files changed, 109 insertions(+), 109 deletions(-)
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index b99e6fa..2afab20 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -740,32 +740,32 @@ Example:
void qapi_free_UserDefOne(UserDefOne *obj)
{
- QapiDeallocVisitor *md;
+ QapiDeallocVisitor *qdv;
Visitor *v;
if (!obj) {
return;
}
- md = qapi_dealloc_visitor_new();
- v = qapi_dealloc_get_visitor(md);
+ qdv = qapi_dealloc_visitor_new();
+ v = qapi_dealloc_get_visitor(qdv);
visit_type_UserDefOne(v, &obj, NULL, NULL);
- qapi_dealloc_visitor_cleanup(md);
+ qapi_dealloc_visitor_cleanup(qdv);
}
void qapi_free_UserDefOneList(UserDefOneList *obj)
{
- QapiDeallocVisitor *md;
+ QapiDeallocVisitor *qdv;
Visitor *v;
if (!obj) {
return;
}
- md = qapi_dealloc_visitor_new();
- v = qapi_dealloc_get_visitor(md);
+ qdv = qapi_dealloc_visitor_new();
+ v = qapi_dealloc_get_visitor(qdv);
visit_type_UserDefOneList(v, &obj, NULL, NULL);
- qapi_dealloc_visitor_cleanup(md);
+ qapi_dealloc_visitor_cleanup(qdv);
}
$ cat qapi-generated/example-qapi-types.h
[Uninteresting stuff omitted...]
@@ -823,15 +823,15 @@ Example:
$ cat qapi-generated/example-qapi-visit.c
[Uninteresting stuff omitted...]
- static void visit_type_UserDefOne_fields(Visitor *m, UserDefOne **obj, Error **errp)
+ static void visit_type_UserDefOne_fields(Visitor *v, UserDefOne **obj, Error **errp)
{
Error *err = NULL;
- visit_type_int(m, &(*obj)->integer, "integer", &err);
+ visit_type_int(v, &(*obj)->integer, "integer", &err);
if (err) {
goto out;
}
- visit_type_str(m, &(*obj)->string, "string", &err);
+ visit_type_str(v, &(*obj)->string, "string", &err);
if (err) {
goto out;
}
@@ -840,40 +840,40 @@ Example:
error_propagate(errp, err);
}
- void visit_type_UserDefOne(Visitor *m, UserDefOne **obj, const char *name, Error **errp)
+ void visit_type_UserDefOne(Visitor *v, UserDefOne **obj, const char *name, Error **errp)
{
Error *err = NULL;
- visit_start_struct(m, (void **)obj, "UserDefOne", name, sizeof(UserDefOne), &err);
+ visit_start_struct(v, (void **)obj, "UserDefOne", name, sizeof(UserDefOne), &err);
if (!err) {
if (*obj) {
- visit_type_UserDefOne_fields(m, obj, errp);
+ visit_type_UserDefOne_fields(v, obj, errp);
}
- visit_end_struct(m, &err);
+ visit_end_struct(v, &err);
}
error_propagate(errp, err);
}
- void visit_type_UserDefOneList(Visitor *m, UserDefOneList **obj, const char *name, Error **errp)
+ void visit_type_UserDefOneList(Visitor *v, UserDefOneList **obj, const char *name, Error **errp)
{
Error *err = NULL;
GenericList *i, **prev;
- visit_start_list(m, name, &err);
+ visit_start_list(v, name, &err);
if (err) {
goto out;
}
for (prev = (GenericList **)obj;
- !err && (i = visit_next_list(m, prev, &err)) != NULL;
+ !err && (i = visit_next_list(v, prev, &err)) != NULL;
prev = &i) {
UserDefOneList *native_i = (UserDefOneList *)i;
- visit_type_UserDefOne(m, &native_i->value, NULL, &err);
+ visit_type_UserDefOne(v, &native_i->value, NULL, &err);
}
error_propagate(errp, err);
err = NULL;
- visit_end_list(m, &err);
+ visit_end_list(v, &err);
out:
error_propagate(errp, err);
}
@@ -885,8 +885,8 @@ Example:
[Visitors for built-in types omitted...]
- void visit_type_UserDefOne(Visitor *m, UserDefOne **obj, const char *name, Error **errp);
- void visit_type_UserDefOneList(Visitor *m, UserDefOneList **obj, const char *name, Error **errp);
+ void visit_type_UserDefOne(Visitor *v, UserDefOne **obj, const char *name, Error **errp);
+ void visit_type_UserDefOneList(Visitor *v, UserDefOneList **obj, const char *name, Error **errp);
#endif
@@ -917,36 +917,36 @@ Example:
static void qmp_marshal_output_UserDefOne(UserDefOne *ret_in, QObject **ret_out, Error **errp)
{
Error *err = NULL;
- QmpOutputVisitor *mo = qmp_output_visitor_new();
- QapiDeallocVisitor *md;
+ QmpOutputVisitor *qov = qmp_output_visitor_new();
+ QapiDeallocVisitor *qdv;
Visitor *v;
- v = qmp_output_get_visitor(mo);
+ v = qmp_output_get_visitor(qov);
visit_type_UserDefOne(v, &ret_in, "unused", &err);
if (err) {
goto out;
}
- *ret_out = qmp_output_get_qobject(mo);
+ *ret_out = qmp_output_get_qobject(qov);
out:
error_propagate(errp, err);
- qmp_output_visitor_cleanup(mo);
- md = qapi_dealloc_visitor_new();
- v = qapi_dealloc_get_visitor(md);
+ qmp_output_visitor_cleanup(qov);
+ qdv = qapi_dealloc_visitor_new();
+ v = qapi_dealloc_get_visitor(qdv);
visit_type_UserDefOne(v, &ret_in, "unused", NULL);
- qapi_dealloc_visitor_cleanup(md);
+ qapi_dealloc_visitor_cleanup(qdv);
}
static void qmp_marshal_my_command(QDict *args, QObject **ret, Error **errp)
{
Error *err = NULL;
UserDefOne *retval;
- QmpInputVisitor *mi = qmp_input_visitor_new_strict(QOBJECT(args));
- QapiDeallocVisitor *md;
+ QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args));
+ QapiDeallocVisitor *qdv;
Visitor *v;
UserDefOne *arg1 = NULL;
- v = qmp_input_get_visitor(mi);
+ v = qmp_input_get_visitor(qiv);
visit_type_UserDefOne(v, &arg1, "arg1", &err);
if (err) {
goto out;
@@ -961,11 +961,11 @@ Example:
out:
error_propagate(errp, err);
- qmp_input_visitor_cleanup(mi);
- md = qapi_dealloc_visitor_new();
- v = qapi_dealloc_get_visitor(md);
+ qmp_input_visitor_cleanup(qiv);
+ qdv = qapi_dealloc_visitor_new();
+ v = qapi_dealloc_get_visitor(qdv);
visit_type_UserDefOne(v, &arg1, "arg1", NULL);
- qapi_dealloc_visitor_cleanup(md);
+ qapi_dealloc_visitor_cleanup(qdv);
}
static void qmp_init_marshal(void)
diff --git a/qom/object.c b/qom/object.c
index 4805328..11cd86b 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1167,31 +1167,31 @@ out:
void object_property_parse(Object *obj, const char *string,
const char *name, Error **errp)
{
- StringInputVisitor *mi;
- mi = string_input_visitor_new(string);
- object_property_set(obj, string_input_get_visitor(mi), name, errp);
+ StringInputVisitor *siv;
+ siv = string_input_visitor_new(string);
+ object_property_set(obj, string_input_get_visitor(siv), name, errp);
- string_input_visitor_cleanup(mi);
+ string_input_visitor_cleanup(siv);
}
char *object_property_print(Object *obj, const char *name, bool human,
Error **errp)
{
- StringOutputVisitor *mo;
+ StringOutputVisitor *sov;
char *string = NULL;
Error *local_err = NULL;
- mo = string_output_visitor_new(human);
- object_property_get(obj, string_output_get_visitor(mo), name, &local_err);
+ sov = string_output_visitor_new(human);
+ object_property_get(obj, string_output_get_visitor(sov), name, &local_err);
if (local_err) {
error_propagate(errp, local_err);
goto out;
}
- string = string_output_get_string(mo);
+ string = string_output_get_string(sov);
out:
- string_output_visitor_cleanup(mo);
+ string_output_visitor_cleanup(sov);
return string;
}
diff --git a/qom/qom-qobject.c b/qom/qom-qobject.c
index 6384b8e..9649890 100644
--- a/qom/qom-qobject.c
+++ b/qom/qom-qobject.c
@@ -19,11 +19,11 @@
void object_property_set_qobject(Object *obj, QObject *value,
const char *name, Error **errp)
{
- QmpInputVisitor *mi;
- mi = qmp_input_visitor_new(value);
- object_property_set(obj, qmp_input_get_visitor(mi), name, errp);
+ QmpInputVisitor *qiv;
+ qiv = qmp_input_visitor_new(value);
+ object_property_set(obj, qmp_input_get_visitor(qiv), name, errp);
- qmp_input_visitor_cleanup(mi);
+ qmp_input_visitor_cleanup(qiv);
}
QObject *object_property_get_qobject(Object *obj, const char *name,
@@ -31,14 +31,14 @@ QObject *object_property_get_qobject(Object *obj, const char *name,
{
QObject *ret = NULL;
Error *local_err = NULL;
- QmpOutputVisitor *mo;
+ QmpOutputVisitor *qov;
- mo = qmp_output_visitor_new();
- object_property_get(obj, qmp_output_get_visitor(mo), name, &local_err);
+ qov = qmp_output_visitor_new();
+ object_property_get(obj, qmp_output_get_visitor(qov), name, &local_err);
if (!local_err) {
- ret = qmp_output_get_qobject(mo);
+ ret = qmp_output_get_qobject(qov);
}
error_propagate(errp, local_err);
- qmp_output_visitor_cleanup(mo);
+ qmp_output_visitor_cleanup(qov);
return ret;
}
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 2151120..475735d 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -82,8 +82,8 @@ def gen_marshal_vars(arg_type, ret_type):
if arg_type:
ret += mcgen('''
-QmpInputVisitor *mi = qmp_input_visitor_new_strict(QOBJECT(args));
-QapiDeallocVisitor *md;
+QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args));
+QapiDeallocVisitor *qdv;
Visitor *v;
''')
@@ -122,15 +122,15 @@ def gen_marshal_input_visit(arg_type, dealloc=False):
errparg = 'NULL'
errarg = None
ret += mcgen('''
-qmp_input_visitor_cleanup(mi);
-md = qapi_dealloc_visitor_new();
-v = qapi_dealloc_get_visitor(md);
+qmp_input_visitor_cleanup(qiv);
+qdv = qapi_dealloc_visitor_new();
+v = qapi_dealloc_get_visitor(qdv);
''')
else:
errparg = '&err'
errarg = 'err'
ret += mcgen('''
-v = qmp_input_get_visitor(mi);
+v = qmp_input_get_visitor(qiv);
''')
for memb in arg_type.members:
@@ -160,7 +160,7 @@ visit_type_%(c_type)s(v, &%(c_name)s, "%(name)s", %(errp)s);
if dealloc:
ret += mcgen('''
-qapi_dealloc_visitor_cleanup(md);
+qapi_dealloc_visitor_cleanup(qdv);
''')
pop_indent()
return ret
@@ -172,24 +172,24 @@ def gen_marshal_output(ret_type):
static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out, Error **errp)
{
Error *err = NULL;
- QmpOutputVisitor *mo = qmp_output_visitor_new();
- QapiDeallocVisitor *md;
+ QmpOutputVisitor *qov = qmp_output_visitor_new();
+ QapiDeallocVisitor *qdv;
Visitor *v;
- v = qmp_output_get_visitor(mo);
+ v = qmp_output_get_visitor(qov);
visit_type_%(c_name)s(v, &ret_in, "unused", &err);
if (err) {
goto out;
}
- *ret_out = qmp_output_get_qobject(mo);
+ *ret_out = qmp_output_get_qobject(qov);
out:
error_propagate(errp, err);
- qmp_output_visitor_cleanup(mo);
- md = qapi_dealloc_visitor_new();
- v = qapi_dealloc_get_visitor(md);
+ qmp_output_visitor_cleanup(qov);
+ qdv = qapi_dealloc_visitor_new();
+ v = qapi_dealloc_get_visitor(qdv);
visit_type_%(c_name)s(v, &ret_in, "unused", NULL);
- qapi_dealloc_visitor_cleanup(md);
+ qapi_dealloc_visitor_cleanup(qdv);
}
''',
c_type=ret_type.c_type(), c_name=ret_type.c_name())
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index b292682..d405f8d 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -188,17 +188,17 @@ def gen_type_cleanup(name):
void qapi_free_%(c_name)s(%(c_name)s *obj)
{
- QapiDeallocVisitor *md;
+ QapiDeallocVisitor *qdv;
Visitor *v;
if (!obj) {
return;
}
- md = qapi_dealloc_visitor_new();
- v = qapi_dealloc_get_visitor(md);
+ qdv = qapi_dealloc_visitor_new();
+ v = qapi_dealloc_get_visitor(qdv);
visit_type_%(c_name)s(v, &obj, NULL, NULL);
- qapi_dealloc_visitor_cleanup(md);
+ qapi_dealloc_visitor_cleanup(qdv);
}
''',
c_name=c_name(name))
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 97343cf..348efe0 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -24,7 +24,7 @@ def gen_visit_decl(name, scalar=False):
if not scalar:
c_type += '*'
return mcgen('''
-void visit_type_%(c_name)s(Visitor *m, %(c_type)sobj, const char *name, Error **errp);
+void visit_type_%(c_name)s(Visitor *v, %(c_type)sobj, const char *name, Error **errp);
''',
c_name=c_name(name), c_type=c_type)
@@ -39,20 +39,20 @@ def gen_visit_implicit_struct(typ):
# Need a forward declaration
ret += mcgen('''
-static void visit_type_%(c_type)s_fields(Visitor *m, %(c_type)s **obj, Error **errp);
+static void visit_type_%(c_type)s_fields(Visitor *v, %(c_type)s **obj, Error **errp);
''',
c_type=typ.c_name())
ret += mcgen('''
-static void visit_type_implicit_%(c_type)s(Visitor *m, %(c_type)s **obj, Error **errp)
+static void visit_type_implicit_%(c_type)s(Visitor *v, %(c_type)s **obj, Error **errp)
{
Error *err = NULL;
- visit_start_implicit_struct(m, (void **)obj, sizeof(%(c_type)s), &err);
+ visit_start_implicit_struct(v, (void **)obj, sizeof(%(c_type)s), &err);
if (!err) {
- visit_type_%(c_type)s_fields(m, obj, errp);
- visit_end_implicit_struct(m, &err);
+ visit_type_%(c_type)s_fields(v, obj, errp);
+ visit_end_implicit_struct(v, &err);
}
error_propagate(errp, err);
}
@@ -71,7 +71,7 @@ def gen_visit_struct_fields(name, base, members):
ret += mcgen('''
-static void visit_type_%(c_name)s_fields(Visitor *m, %(c_name)s **obj, Error **errp)
+static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s **obj, Error **errp)
{
Error *err = NULL;
@@ -81,7 +81,7 @@ static void visit_type_%(c_name)s_fields(Visitor *m, %(c_name)s **obj, Error **e
if base:
ret += mcgen('''
-visit_type_implicit_%(c_type)s(m, &(*obj)->%(c_name)s, &err);
+visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err);
if (err) {
goto out;
}
@@ -91,14 +91,14 @@ if (err) {
for memb in members:
if memb.optional:
ret += mcgen('''
-visit_optional(m, &(*obj)->has_%(c_name)s, "%(name)s", &err);
+visit_optional(v, &(*obj)->has_%(c_name)s, "%(name)s", &err);
if (!err && (*obj)->has_%(c_name)s) {
''',
c_name=c_name(memb.name), name=memb.name)
push_indent()
ret += mcgen('''
-visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "%(name)s", &err);
+visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err);
''',
c_type=memb.type.c_name(), c_name=c_name(memb.name),
name=memb.name)
@@ -136,16 +136,16 @@ def gen_visit_struct(name, base, members):
# call qapi_free_FOO() to avoid a memory leak of the partial FOO.
ret += mcgen('''
-void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error **errp)
+void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error **errp)
{
Error *err = NULL;
- visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), &err);
+ visit_start_struct(v, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), &err);
if (!err) {
if (*obj) {
- visit_type_%(c_name)s_fields(m, obj, errp);
+ visit_type_%(c_name)s_fields(v, obj, errp);
}
- visit_end_struct(m, &err);
+ visit_end_struct(v, &err);
}
error_propagate(errp, err);
}
@@ -158,26 +158,26 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error
def gen_visit_list(name, element_type):
return mcgen('''
-void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error **errp)
+void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error **errp)
{
Error *err = NULL;
GenericList *i, **prev;
- visit_start_list(m, name, &err);
+ visit_start_list(v, name, &err);
if (err) {
goto out;
}
for (prev = (GenericList **)obj;
- !err && (i = visit_next_list(m, prev, &err)) != NULL;
+ !err && (i = visit_next_list(v, prev, &err)) != NULL;
prev = &i) {
%(c_name)s *native_i = (%(c_name)s *)i;
- visit_type_%(c_elt_type)s(m, &native_i->value, NULL, &err);
+ visit_type_%(c_elt_type)s(v, &native_i->value, NULL, &err);
}
error_propagate(errp, err);
err = NULL;
- visit_end_list(m, &err);
+ visit_end_list(v, &err);
out:
error_propagate(errp, err);
}
@@ -188,9 +188,9 @@ out:
def gen_visit_enum(name):
return mcgen('''
-void visit_type_%(c_name)s(Visitor *m, %(c_name)s *obj, const char *name, Error **errp)
+void visit_type_%(c_name)s(Visitor *v, %(c_name)s *obj, const char *name, Error **errp)
{
- visit_type_enum(m, (int *)obj, %(c_name)s_lookup, "%(name)s", name, errp);
+ visit_type_enum(v, (int *)obj, %(c_name)s_lookup, "%(name)s", name, errp);
}
''',
c_name=c_name(name), name=name)
@@ -199,15 +199,15 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s *obj, const char *name, Error
def gen_visit_alternate(name, variants):
ret = mcgen('''
-void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error **errp)
+void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error **errp)
{
Error *err = NULL;
- visit_start_implicit_struct(m, (void**) obj, sizeof(%(c_name)s), &err);
+ visit_start_implicit_struct(v, (void**) obj, sizeof(%(c_name)s), &err);
if (err) {
goto out;
}
- visit_get_next_type(m, (int*) &(*obj)->kind, %(c_name)s_qtypes, name, &err);
+ visit_get_next_type(v, (int*) &(*obj)->kind, %(c_name)s_qtypes, name, &err);
if (err) {
goto out_end;
}
@@ -218,7 +218,7 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error
for var in variants.variants:
ret += mcgen('''
case %(case)s:
- visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, name, &err);
+ visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, name, &err);
break;
''',
case=c_enum_const(variants.tag_member.type.name,
@@ -233,7 +233,7 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error
out_end:
error_propagate(errp, err);
err = NULL;
- visit_end_implicit_struct(m, &err);
+ visit_end_implicit_struct(v, &err);
out:
error_propagate(errp, err);
}
@@ -256,11 +256,11 @@ def gen_visit_union(name, base, variants):
ret += mcgen('''
-void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error **errp)
+void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error **errp)
{
Error *err = NULL;
- visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), &err);
+ visit_start_struct(v, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), &err);
if (err) {
goto out;
}
@@ -270,7 +270,7 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error
if base:
ret += mcgen('''
- visit_type_%(c_name)s_fields(m, obj, &err);
+ visit_type_%(c_name)s_fields(v, obj, &err);
if (err) {
goto out_obj;
}
@@ -282,11 +282,11 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error
# we pointlessly use a different key for simple unions
tag_key = 'type'
ret += mcgen('''
- visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "%(name)s", &err);
+ visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err);
if (err) {
goto out_obj;
}
- if (!visit_start_union(m, !!(*obj)->data, &err) || err) {
+ if (!visit_start_union(v, !!(*obj)->data, &err) || err) {
goto out_obj;
}
switch ((*obj)->%(c_name)s) {
@@ -308,13 +308,13 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error
var.name))
if simple_union_type:
ret += mcgen('''
- visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "data", &err);
+ visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "data", &err);
''',
c_type=simple_union_type.c_name(),
c_name=c_name(var.name))
else:
ret += mcgen('''
- visit_type_implicit_%(c_type)s(m, &(*obj)->%(c_name)s, &err);
+ visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err);
''',
c_type=var.type.c_name(),
c_name=c_name(var.name))
@@ -329,11 +329,11 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error
out_obj:
error_propagate(errp, err);
err = NULL;
- visit_end_union(m, !!(*obj)->data, &err);
+ visit_end_union(v, !!(*obj)->data, &err);
error_propagate(errp, err);
err = NULL;
}
- visit_end_struct(m, &err);
+ visit_end_struct(v, &err);
out:
error_propagate(errp, err);
}
--
2.4.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PULL v2 15/20] qapi: Consistent generated code: prefer common labels
2015-10-12 18:19 [Qemu-devel] [PULL v2 00/20] QAPI patches Markus Armbruster
` (13 preceding siblings ...)
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 14/20] qapi: Consistent generated code: prefer visitor 'v' Markus Armbruster
@ 2015-10-12 18:19 ` Markus Armbruster
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 16/20] qapi: Consistent generated code: prefer common indentation Markus Armbruster
` (5 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2015-10-12 18:19 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
We had some pointless differences in the generated code for visit,
command marshalling, and events; unifying them makes it easier for
future patches to consolidate to common helper functions.
This is one patch of a series to clean up these differences.
This patch names the goto labels 'out' (not 'clean') and 'out_obj'
(not 'out_end'). Additionally, the generator was inconsistent on
whether labels had a leading space [our HACKING is silent; while
emacs 'gnu' style adds the space to avoid littering column 1].
For minimal churn, prefer no leading space; this also matches
the style that is more prevalent in current qemu.git.
No change in semantics to the generated code.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-13-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/qapi-event.py | 8 ++++----
scripts/qapi-visit.py | 4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index d41af40..b5a9d4f 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -69,7 +69,7 @@ def gen_event_send(name, arg_type):
/* Fake visit, as if all members are under a structure */
visit_start_struct(v, NULL, "", "%(name)s", 0, &err);
if (err) {
- goto clean;
+ goto out;
}
''',
@@ -92,7 +92,7 @@ def gen_event_send(name, arg_type):
ret += mcgen('''
visit_type_%(c_type)s(v, %(cast)s&%(c_name)s, "%(name)s", &err);
if (err) {
- goto clean;
+ goto out;
}
''',
cast=cast,
@@ -110,7 +110,7 @@ def gen_event_send(name, arg_type):
visit_end_struct(v, &err);
if (err) {
- goto clean;
+ goto out;
}
obj = qmp_output_get_qobject(qov);
@@ -127,7 +127,7 @@ def gen_event_send(name, arg_type):
if arg_type and arg_type.members:
ret += mcgen('''
- clean:
+out:
qmp_output_visitor_cleanup(qov);
''')
ret += mcgen('''
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 348efe0..5a453ea 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -209,7 +209,7 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
}
visit_get_next_type(v, (int*) &(*obj)->kind, %(c_name)s_qtypes, name, &err);
if (err) {
- goto out_end;
+ goto out_obj;
}
switch ((*obj)->kind) {
''',
@@ -230,7 +230,7 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
default:
abort();
}
-out_end:
+out_obj:
error_propagate(errp, err);
err = NULL;
visit_end_implicit_struct(v, &err);
--
2.4.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PULL v2 16/20] qapi: Consistent generated code: prefer common indentation
2015-10-12 18:19 [Qemu-devel] [PULL v2 00/20] QAPI patches Markus Armbruster
` (14 preceding siblings ...)
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 15/20] qapi: Consistent generated code: prefer common labels Markus Armbruster
@ 2015-10-12 18:19 ` Markus Armbruster
2015-10-14 16:01 ` Marc-André Lureau
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 17/20] qapi: Consistent generated code: minimize push_indent() usage Markus Armbruster
` (4 subsequent siblings)
20 siblings, 1 reply; 24+ messages in thread
From: Markus Armbruster @ 2015-10-12 18:19 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
We had some pointless differences in the generated code for visit,
command marshalling, and events; unifying them makes it easier for
future patches to consolidate to common helper functions.
This is one patch of a series to clean up these differences.
This patch adjusts gen_visit_union() to use the same indentation
as other functions, namely, by jumping early to the error label
if the object was not set rather than placing the rest of the
body inside an if for when it is set.
No change in semantics to the generated code.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-14-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/qapi-visit.py | 53 ++++++++++++++++++++++++++-------------------------
1 file changed, 27 insertions(+), 26 deletions(-)
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 5a453ea..fe9780e 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -264,16 +264,18 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
if (err) {
goto out;
}
- if (*obj) {
+ if (!*obj) {
+ goto out_obj;
+ }
''',
c_name=c_name(name), name=name)
if base:
ret += mcgen('''
- visit_type_%(c_name)s_fields(v, obj, &err);
- if (err) {
- goto out_obj;
- }
+ visit_type_%(c_name)s_fields(v, obj, &err);
+ if (err) {
+ goto out_obj;
+ }
''',
c_name=c_name(name))
@@ -282,14 +284,14 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
# we pointlessly use a different key for simple unions
tag_key = 'type'
ret += mcgen('''
- visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err);
- if (err) {
- goto out_obj;
- }
- if (!visit_start_union(v, !!(*obj)->data, &err) || err) {
- goto out_obj;
- }
- switch ((*obj)->%(c_name)s) {
+ visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err);
+ if (err) {
+ goto out_obj;
+ }
+ if (!visit_start_union(v, !!(*obj)->data, &err) || err) {
+ goto out_obj;
+ }
+ switch ((*obj)->%(c_name)s) {
''',
c_type=variants.tag_member.type.c_name(),
# TODO ugly special case for simple union
@@ -302,37 +304,36 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
# TODO ugly special case for simple union
simple_union_type = var.simple_union_type()
ret += mcgen('''
- case %(case)s:
+ case %(case)s:
''',
case=c_enum_const(variants.tag_member.type.name,
var.name))
if simple_union_type:
ret += mcgen('''
- visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "data", &err);
+ visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "data", &err);
''',
c_type=simple_union_type.c_name(),
c_name=c_name(var.name))
else:
ret += mcgen('''
- visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err);
+ visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err);
''',
c_type=var.type.c_name(),
c_name=c_name(var.name))
ret += mcgen('''
- break;
+ break;
''')
ret += mcgen('''
- default:
- abort();
- }
-out_obj:
- error_propagate(errp, err);
- err = NULL;
- visit_end_union(v, !!(*obj)->data, &err);
- error_propagate(errp, err);
- err = NULL;
+ default:
+ abort();
}
+out_obj:
+ error_propagate(errp, err);
+ err = NULL;
+ visit_end_union(v, !!(*obj)->data, &err);
+ error_propagate(errp, err);
+ err = NULL;
visit_end_struct(v, &err);
out:
error_propagate(errp, err);
--
2.4.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PULL v2 16/20] qapi: Consistent generated code: prefer common indentation
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 16/20] qapi: Consistent generated code: prefer common indentation Markus Armbruster
@ 2015-10-14 16:01 ` Marc-André Lureau
2015-10-14 16:24 ` Eric Blake
0 siblings, 1 reply; 24+ messages in thread
From: Marc-André Lureau @ 2015-10-14 16:01 UTC (permalink / raw)
To: Markus Armbruster, Eric Blake; +Cc: QEMU
Hi,
This patch introduces a regression:
$ x86_64-softmmu/qemu-system-x86_64 -netdev ?
Program received signal SIGSEGV, Segmentation fault.
0x0000555555a0bc0f in visit_type_NetClientOptions (v=0x5555564e19e0,
obj=0x555556510438, name=0x555555af7041 "opts", errp=0x7fffffffd450)
at qapi-visit.c:6906
6906 visit_end_union(v, !!(*obj)->data, &err);
(gdb) print *obj
$1 = (NetClientOptions *) 0x0
On Mon, Oct 12, 2015 at 8:19 PM, Markus Armbruster <armbru@redhat.com> wrote:
> From: Eric Blake <eblake@redhat.com>
>
> We had some pointless differences in the generated code for visit,
> command marshalling, and events; unifying them makes it easier for
> future patches to consolidate to common helper functions.
> This is one patch of a series to clean up these differences.
>
> This patch adjusts gen_visit_union() to use the same indentation
> as other functions, namely, by jumping early to the error label
> if the object was not set rather than placing the rest of the
> body inside an if for when it is set.
>
> No change in semantics to the generated code.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> Message-Id: <1443565276-4535-14-git-send-email-eblake@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> scripts/qapi-visit.py | 53 ++++++++++++++++++++++++++-------------------------
> 1 file changed, 27 insertions(+), 26 deletions(-)
>
> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
> index 5a453ea..fe9780e 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -264,16 +264,18 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
> if (err) {
> goto out;
> }
> - if (*obj) {
> + if (!*obj) {
> + goto out_obj;
> + }
> ''',
> c_name=c_name(name), name=name)
>
> if base:
> ret += mcgen('''
> - visit_type_%(c_name)s_fields(v, obj, &err);
> - if (err) {
> - goto out_obj;
> - }
> + visit_type_%(c_name)s_fields(v, obj, &err);
> + if (err) {
> + goto out_obj;
> + }
> ''',
> c_name=c_name(name))
>
> @@ -282,14 +284,14 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
> # we pointlessly use a different key for simple unions
> tag_key = 'type'
> ret += mcgen('''
> - visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err);
> - if (err) {
> - goto out_obj;
> - }
> - if (!visit_start_union(v, !!(*obj)->data, &err) || err) {
> - goto out_obj;
> - }
> - switch ((*obj)->%(c_name)s) {
> + visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err);
> + if (err) {
> + goto out_obj;
> + }
> + if (!visit_start_union(v, !!(*obj)->data, &err) || err) {
> + goto out_obj;
> + }
> + switch ((*obj)->%(c_name)s) {
> ''',
> c_type=variants.tag_member.type.c_name(),
> # TODO ugly special case for simple union
> @@ -302,37 +304,36 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
> # TODO ugly special case for simple union
> simple_union_type = var.simple_union_type()
> ret += mcgen('''
> - case %(case)s:
> + case %(case)s:
> ''',
> case=c_enum_const(variants.tag_member.type.name,
> var.name))
> if simple_union_type:
> ret += mcgen('''
> - visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "data", &err);
> + visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "data", &err);
> ''',
> c_type=simple_union_type.c_name(),
> c_name=c_name(var.name))
> else:
> ret += mcgen('''
> - visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err);
> + visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err);
> ''',
> c_type=var.type.c_name(),
> c_name=c_name(var.name))
> ret += mcgen('''
> - break;
> + break;
> ''')
>
> ret += mcgen('''
> - default:
> - abort();
> - }
> -out_obj:
> - error_propagate(errp, err);
> - err = NULL;
> - visit_end_union(v, !!(*obj)->data, &err);
> - error_propagate(errp, err);
> - err = NULL;
> + default:
> + abort();
> }
> +out_obj:
> + error_propagate(errp, err);
> + err = NULL;
> + visit_end_union(v, !!(*obj)->data, &err);
> + error_propagate(errp, err);
> + err = NULL;
> visit_end_struct(v, &err);
> out:
> error_propagate(errp, err);
> --
> 2.4.3
>
>
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PULL v2 16/20] qapi: Consistent generated code: prefer common indentation
2015-10-14 16:01 ` Marc-André Lureau
@ 2015-10-14 16:24 ` Eric Blake
0 siblings, 0 replies; 24+ messages in thread
From: Eric Blake @ 2015-10-14 16:24 UTC (permalink / raw)
To: Marc-André Lureau, Markus Armbruster; +Cc: QEMU
[-- Attachment #1: Type: text/plain, Size: 1696 bytes --]
On 10/14/2015 10:01 AM, Marc-André Lureau wrote:
> Hi,
>
> This patch introduces a regression:
>
> $ x86_64-softmmu/qemu-system-x86_64 -netdev ?
>
> Program received signal SIGSEGV, Segmentation fault.
> 0x0000555555a0bc0f in visit_type_NetClientOptions (v=0x5555564e19e0,
> obj=0x555556510438, name=0x555555af7041 "opts", errp=0x7fffffffd450)
> at qapi-visit.c:6906
> 6906 visit_end_union(v, !!(*obj)->data, &err);
> (gdb) print *obj
> $1 = (NetClientOptions *) 0x0
visit_end_union() is pointless (it is a no-op). I plan on properly
deleting visit_begin_union/visit_end_union later in my series.
But in the meantime, I see the source of the problem:
>> @@ -264,16 +264,18 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
>> if (err) {
>> goto out;
>> }
>> - if (*obj) {
>> + if (!*obj) {
>> + goto out_obj;
>> + }
>> ''',
>> c_name=c_name(name), name=name)
>> ret += mcgen('''
>> - default:
>> - abort();
>> - }
>> -out_obj:
>> - error_propagate(errp, err);
>> - err = NULL;
>> - visit_end_union(v, !!(*obj)->data, &err);
>> - error_propagate(errp, err);
>> - err = NULL;
>> + default:
>> + abort();
>> }
>> +out_obj:
>> + error_propagate(errp, err);
>> + err = NULL;
>> + visit_end_union(v, !!(*obj)->data, &err);
The call to visit_end_union() used to be conditional on whether *obj was
non-NULL. So I'll post the quick and obvious fix.
--
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: 604 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* [Qemu-devel] [PULL v2 17/20] qapi: Consistent generated code: minimize push_indent() usage
2015-10-12 18:19 [Qemu-devel] [PULL v2 00/20] QAPI patches Markus Armbruster
` (15 preceding siblings ...)
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 16/20] qapi: Consistent generated code: prefer common indentation Markus Armbruster
@ 2015-10-12 18:19 ` Markus Armbruster
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 18/20] qapi: Share gen_err_check() Markus Armbruster
` (3 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2015-10-12 18:19 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
We had some pointless differences in the generated code for visit,
command marshalling, and events; unifying them makes it easier for
future patches to consolidate to common helper functions.
This is one patch of a series to clean up these differences.
This patch reduces the number of push_indent()/pop_indent() pairs
so that generated code is typically already at its natural output
indentation in the python files. It is easier to reason about
generated code if the reader does not have to track how much
spacing will be inserted alongside the code, and moreso when all
of the generators use the same patterns (qapi-type and qapi-event
were already using in-place indentation).
Arguably, the resulting python may be a bit harder to read with C
code at the same indentation as python; on the other hand, not
having to think about push_indent() is a win, and most decent
editors provide syntax highlighting that makes it easier to
visually distinguish python code from string literals that will
become C code.
There is no change to the generated output.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-15-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/qapi-commands.py | 54 ++++++++++++++++++++----------------------------
scripts/qapi-visit.py | 24 ++++++++++-----------
2 files changed, 33 insertions(+), 45 deletions(-)
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 475735d..267991e 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -29,9 +29,9 @@ def gen_err_check(err):
if not err:
return ''
return mcgen('''
-if (%(err)s) {
- goto out;
-}
+ if (%(err)s) {
+ goto out;
+ }
''',
err=err)
@@ -50,20 +50,18 @@ def gen_call(name, arg_type, ret_type):
if ret_type:
lhs = 'retval = '
- push_indent()
ret = mcgen('''
-%(lhs)sqmp_%(c_name)s(%(args)s&err);
+ %(lhs)sqmp_%(c_name)s(%(args)s&err);
''',
c_name=c_name(name), args=argstr, lhs=lhs)
if ret_type:
ret += gen_err_check('err')
ret += mcgen('''
-qmp_marshal_output_%(c_name)s(retval, ret, &err);
+ qmp_marshal_output_%(c_name)s(retval, ret, &err);
''',
c_name=ret_type.c_name())
- pop_indent()
return ret
@@ -72,29 +70,27 @@ def gen_marshal_vars(arg_type, ret_type):
Error *err = NULL;
''')
- push_indent()
-
if ret_type:
ret += mcgen('''
-%(c_type)s retval;
+ %(c_type)s retval;
''',
c_type=ret_type.c_type())
if arg_type:
ret += mcgen('''
-QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args));
-QapiDeallocVisitor *qdv;
-Visitor *v;
+ QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args));
+ QapiDeallocVisitor *qdv;
+ Visitor *v;
''')
for memb in arg_type.members:
if memb.optional:
ret += mcgen('''
-bool has_%(c_name)s = false;
+ bool has_%(c_name)s = false;
''',
c_name=c_name(memb.name))
ret += mcgen('''
-%(c_type)s %(c_name)s = %(c_null)s;
+ %(c_type)s %(c_name)s = %(c_null)s;
''',
c_name=c_name(memb.name),
c_type=memb.type.c_type(),
@@ -103,10 +99,9 @@ bool has_%(c_name)s = false;
else:
ret += mcgen('''
-(void)args;
+ (void)args;
''')
- pop_indent()
return ret
@@ -116,38 +111,36 @@ def gen_marshal_input_visit(arg_type, dealloc=False):
if not arg_type:
return ret
- push_indent()
-
if dealloc:
errparg = 'NULL'
errarg = None
ret += mcgen('''
-qmp_input_visitor_cleanup(qiv);
-qdv = qapi_dealloc_visitor_new();
-v = qapi_dealloc_get_visitor(qdv);
+ qmp_input_visitor_cleanup(qiv);
+ qdv = qapi_dealloc_visitor_new();
+ v = qapi_dealloc_get_visitor(qdv);
''')
else:
errparg = '&err'
errarg = 'err'
ret += mcgen('''
-v = qmp_input_get_visitor(qiv);
+ v = qmp_input_get_visitor(qiv);
''')
for memb in arg_type.members:
if memb.optional:
ret += mcgen('''
-visit_optional(v, &has_%(c_name)s, "%(name)s", %(errp)s);
+ visit_optional(v, &has_%(c_name)s, "%(name)s", %(errp)s);
''',
c_name=c_name(memb.name), name=memb.name,
errp=errparg)
ret += gen_err_check(errarg)
ret += mcgen('''
-if (has_%(c_name)s) {
+ if (has_%(c_name)s) {
''',
c_name=c_name(memb.name))
push_indent()
ret += mcgen('''
-visit_type_%(c_type)s(v, &%(c_name)s, "%(name)s", %(errp)s);
+ visit_type_%(c_type)s(v, &%(c_name)s, "%(name)s", %(errp)s);
''',
c_name=c_name(memb.name), name=memb.name,
c_type=memb.type.c_name(), errp=errparg)
@@ -155,14 +148,13 @@ visit_type_%(c_type)s(v, &%(c_name)s, "%(name)s", %(errp)s);
if memb.optional:
pop_indent()
ret += mcgen('''
-}
+ }
''')
if dealloc:
ret += mcgen('''
-qapi_dealloc_visitor_cleanup(qdv);
+ qapi_dealloc_visitor_cleanup(qdv);
''')
- pop_indent()
return ret
@@ -237,17 +229,15 @@ out:
def gen_register_command(name, success_response):
- push_indent()
options = 'QCO_NO_OPTIONS'
if not success_response:
options = 'QCO_NO_SUCCESS_RESP'
ret = mcgen('''
-qmp_register_command("%(name)s", qmp_marshal_%(c_name)s, %(opts)s);
+ qmp_register_command("%(name)s", qmp_marshal_%(c_name)s, %(opts)s);
''',
name=name, c_name=c_name(name),
opts=options)
- pop_indent()
return ret
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index fe9780e..78ad556 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -77,28 +77,27 @@ static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s **obj, Error **e
''',
c_name=c_name(name))
- push_indent()
if base:
ret += mcgen('''
-visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err);
-if (err) {
- goto out;
-}
+ visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err);
+ if (err) {
+ goto out;
+ }
''',
c_type=base.c_name(), c_name=c_name('base'))
for memb in members:
if memb.optional:
ret += mcgen('''
-visit_optional(v, &(*obj)->has_%(c_name)s, "%(name)s", &err);
-if (!err && (*obj)->has_%(c_name)s) {
+ visit_optional(v, &(*obj)->has_%(c_name)s, "%(name)s", &err);
+ if (!err && (*obj)->has_%(c_name)s) {
''',
c_name=c_name(memb.name), name=memb.name)
push_indent()
ret += mcgen('''
-visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err);
+ visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err);
''',
c_type=memb.type.c_name(), c_name=c_name(memb.name),
name=memb.name)
@@ -106,15 +105,14 @@ visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err);
if memb.optional:
pop_indent()
ret += mcgen('''
-}
+ }
''')
ret += mcgen('''
-if (err) {
- goto out;
-}
+ if (err) {
+ goto out;
+ }
''')
- pop_indent()
if re.search('^ *goto out;', ret, re.MULTILINE):
ret += mcgen('''
--
2.4.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PULL v2 18/20] qapi: Share gen_err_check()
2015-10-12 18:19 [Qemu-devel] [PULL v2 00/20] QAPI patches Markus Armbruster
` (16 preceding siblings ...)
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 17/20] qapi: Consistent generated code: minimize push_indent() usage Markus Armbruster
@ 2015-10-12 18:19 ` Markus Armbruster
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 19/20] qapi: Share gen_visit_fields() Markus Armbruster
` (2 subsequent siblings)
20 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2015-10-12 18:19 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
qapi-commands has a nice helper gen_err_check(), but did not
use it everywhere. In fact, using it in more places makes it
easier to reduce the lines of code used for generating error
checks. This in turn will make it easier for later patches
to consolidate another common pattern among the generators.
The generated code has fewer blank lines in qapi-event.c functions,
but has no semantic difference.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-16-git-send-email-eblake@redhat.com>
[Drop another blank line for symmetry]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/qapi-commands.py | 17 +++--------------
scripts/qapi-event.py | 10 ++--------
scripts/qapi-visit.py | 14 +++-----------
scripts/qapi.py | 12 ++++++++++++
4 files changed, 20 insertions(+), 33 deletions(-)
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 267991e..4e99c1d 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -25,17 +25,6 @@ def gen_command_decl(name, arg_type, ret_type):
params=gen_params(arg_type, 'Error **errp'))
-def gen_err_check(err):
- if not err:
- return ''
- return mcgen('''
- if (%(err)s) {
- goto out;
- }
-''',
- err=err)
-
-
def gen_call(name, arg_type, ret_type):
ret = ''
@@ -56,7 +45,7 @@ def gen_call(name, arg_type, ret_type):
''',
c_name=c_name(name), args=argstr, lhs=lhs)
if ret_type:
- ret += gen_err_check('err')
+ ret += gen_err_check()
ret += mcgen('''
qmp_marshal_output_%(c_name)s(retval, ret, &err);
@@ -133,7 +122,7 @@ def gen_marshal_input_visit(arg_type, dealloc=False):
''',
c_name=c_name(memb.name), name=memb.name,
errp=errparg)
- ret += gen_err_check(errarg)
+ ret += gen_err_check(err=errarg)
ret += mcgen('''
if (has_%(c_name)s) {
''',
@@ -144,7 +133,7 @@ def gen_marshal_input_visit(arg_type, dealloc=False):
''',
c_name=c_name(memb.name), name=memb.name,
c_type=memb.type.c_name(), errp=errparg)
- ret += gen_err_check(errarg)
+ ret += gen_err_check(err=errarg)
if memb.optional:
pop_indent()
ret += mcgen('''
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index b5a9d4f..eaaac05 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -68,12 +68,9 @@ def gen_event_send(name, arg_type):
/* Fake visit, as if all members are under a structure */
visit_start_struct(v, NULL, "", "%(name)s", 0, &err);
- if (err) {
- goto out;
- }
-
''',
name=name)
+ ret += gen_err_check()
for memb in arg_type.members:
if memb.optional:
@@ -91,14 +88,12 @@ def gen_event_send(name, arg_type):
ret += mcgen('''
visit_type_%(c_type)s(v, %(cast)s&%(c_name)s, "%(name)s", &err);
- if (err) {
- goto out;
- }
''',
cast=cast,
c_name=c_name(memb.name),
c_type=memb.type.c_name(),
name=memb.name)
+ ret += gen_err_check()
if memb.optional:
pop_indent()
@@ -107,7 +102,6 @@ def gen_event_send(name, arg_type):
''')
ret += mcgen('''
-
visit_end_struct(v, &err);
if (err) {
goto out;
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 78ad556..bc6911f 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -81,11 +81,9 @@ static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s **obj, Error **e
if base:
ret += mcgen('''
visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err);
- if (err) {
- goto out;
- }
''',
c_type=base.c_name(), c_name=c_name('base'))
+ ret += gen_err_check()
for memb in members:
if memb.optional:
@@ -107,11 +105,7 @@ static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s **obj, Error **e
ret += mcgen('''
}
''')
- ret += mcgen('''
- if (err) {
- goto out;
- }
-''')
+ ret += gen_err_check()
if re.search('^ *goto out;', ret, re.MULTILINE):
ret += mcgen('''
@@ -271,11 +265,9 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
if base:
ret += mcgen('''
visit_type_%(c_name)s_fields(v, obj, &err);
- if (err) {
- goto out_obj;
- }
''',
c_name=c_name(name))
+ ret += gen_err_check(label='out_obj')
tag_key = variants.tag_member.name
if not variants.tag_name:
diff --git a/scripts/qapi.py b/scripts/qapi.py
index c0728d7..62a415c 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1536,6 +1536,18 @@ def gen_params(arg_type, extra):
ret += sep + extra
return ret
+
+def gen_err_check(err='err', label='out'):
+ if not err:
+ return ''
+ return mcgen('''
+ if (%(err)s) {
+ goto %(label)s;
+ }
+''',
+ err=err, label=label)
+
+
#
# Common command line parsing
#
--
2.4.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PULL v2 19/20] qapi: Share gen_visit_fields()
2015-10-12 18:19 [Qemu-devel] [PULL v2 00/20] QAPI patches Markus Armbruster
` (17 preceding siblings ...)
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 18/20] qapi: Share gen_err_check() Markus Armbruster
@ 2015-10-12 18:19 ` Markus Armbruster
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 20/20] qapi: Simplify gen_visit_fields() error handling Markus Armbruster
2015-10-13 10:55 ` [Qemu-devel] [PULL v2 00/20] QAPI patches Peter Maydell
20 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2015-10-12 18:19 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
Consolidate the code between visit, command marshalling, and
event generation that iterates over the members of a struct.
It reduces code duplication in the generator, so that a future
patch can reduce the size of generated code while touching only
one instead of three locations.
There are no changes to the generated marshal code.
The visitor code becomes slightly more verbose, but remains
semantically equivalent, and is actually easier to read as
it follows a more common idiom:
| visit_optional(v, &(*obj)->has_device, "device", &err);
|- if (!err && (*obj)->has_device) {
|- visit_type_str(v, &(*obj)->device, "device", &err);
|- }
| if (err) {
| goto out;
| }
|+ if ((*obj)->has_device) {
|+ visit_type_str(v, &(*obj)->device, "device", &err);
|+ if (err) {
|+ goto out;
|+ }
|+ }
The event code becomes slightly more verbose, but this is
arguably a bug fix: although the visitors are not well
documented, use of an optional member should not be attempted
unless guarded by a prior call to visit_optional(). Works only
because the output qmp visitor has a no-op visit_optional():
|+ visit_optional(v, &has_offset, "offset", &err);
|+ if (err) {
|+ goto out;
|+ }
| if (has_offset) {
| visit_type_int(v, &offset, "offset", &err);
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-17-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/qapi-commands.py | 27 +--------------------------
scripts/qapi-event.py | 31 +------------------------------
scripts/qapi-visit.py | 22 +---------------------
scripts/qapi.py | 43 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 46 insertions(+), 77 deletions(-)
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 4e99c1d..9d214a6 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -101,7 +101,6 @@ def gen_marshal_input_visit(arg_type, dealloc=False):
return ret
if dealloc:
- errparg = 'NULL'
errarg = None
ret += mcgen('''
qmp_input_visitor_cleanup(qiv);
@@ -109,36 +108,12 @@ def gen_marshal_input_visit(arg_type, dealloc=False):
v = qapi_dealloc_get_visitor(qdv);
''')
else:
- errparg = '&err'
errarg = 'err'
ret += mcgen('''
v = qmp_input_get_visitor(qiv);
''')
- for memb in arg_type.members:
- if memb.optional:
- ret += mcgen('''
- visit_optional(v, &has_%(c_name)s, "%(name)s", %(errp)s);
-''',
- c_name=c_name(memb.name), name=memb.name,
- errp=errparg)
- ret += gen_err_check(err=errarg)
- ret += mcgen('''
- if (has_%(c_name)s) {
-''',
- c_name=c_name(memb.name))
- push_indent()
- ret += mcgen('''
- visit_type_%(c_type)s(v, &%(c_name)s, "%(name)s", %(errp)s);
-''',
- c_name=c_name(memb.name), name=memb.name,
- c_type=memb.type.c_name(), errp=errparg)
- ret += gen_err_check(err=errarg)
- if memb.optional:
- pop_indent()
- ret += mcgen('''
- }
-''')
+ ret += gen_visit_fields(arg_type.members, errarg=errarg)
if dealloc:
ret += mcgen('''
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index eaaac05..720486f 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -71,36 +71,7 @@ def gen_event_send(name, arg_type):
''',
name=name)
ret += gen_err_check()
-
- for memb in arg_type.members:
- if memb.optional:
- ret += mcgen('''
- if (has_%(c_name)s) {
-''',
- c_name=c_name(memb.name))
- push_indent()
-
- # Ugly: need to cast away the const
- if memb.type.name == "str":
- cast = '(char **)'
- else:
- cast = ''
-
- ret += mcgen('''
- visit_type_%(c_type)s(v, %(cast)s&%(c_name)s, "%(name)s", &err);
-''',
- cast=cast,
- c_name=c_name(memb.name),
- c_type=memb.type.c_name(),
- name=memb.name)
- ret += gen_err_check()
-
- if memb.optional:
- pop_indent()
- ret += mcgen('''
- }
-''')
-
+ ret += gen_visit_fields(arg_type.members, need_cast=True)
ret += mcgen('''
visit_end_struct(v, &err);
if (err) {
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index bc6911f..4f97781 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -85,27 +85,7 @@ static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s **obj, Error **e
c_type=base.c_name(), c_name=c_name('base'))
ret += gen_err_check()
- for memb in members:
- if memb.optional:
- ret += mcgen('''
- visit_optional(v, &(*obj)->has_%(c_name)s, "%(name)s", &err);
- if (!err && (*obj)->has_%(c_name)s) {
-''',
- c_name=c_name(memb.name), name=memb.name)
- push_indent()
-
- ret += mcgen('''
- visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err);
-''',
- c_type=memb.type.c_name(), c_name=c_name(memb.name),
- name=memb.name)
-
- if memb.optional:
- pop_indent()
- ret += mcgen('''
- }
-''')
- ret += gen_err_check()
+ ret += gen_visit_fields(members, prefix='(*obj)->')
if re.search('^ *goto out;', ret, re.MULTILINE):
ret += mcgen('''
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 62a415c..ada6380 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1548,6 +1548,49 @@ def gen_err_check(err='err', label='out'):
err=err, label=label)
+def gen_visit_fields(members, prefix='', need_cast=False, errarg='err'):
+ ret = ''
+ if errarg:
+ errparg = '&' + errarg
+ else:
+ errparg = 'NULL'
+
+ for memb in members:
+ if memb.optional:
+ ret += mcgen('''
+ visit_optional(v, &%(prefix)shas_%(c_name)s, "%(name)s", %(errp)s);
+''',
+ prefix=prefix, c_name=c_name(memb.name),
+ name=memb.name, errp=errparg)
+ ret += gen_err_check(err=errarg)
+ ret += mcgen('''
+ if (%(prefix)shas_%(c_name)s) {
+''',
+ prefix=prefix, c_name=c_name(memb.name))
+ push_indent()
+
+ # Ugly: sometimes we need to cast away const
+ if need_cast and memb.type.name == 'str':
+ cast = '(char **)'
+ else:
+ cast = ''
+
+ ret += mcgen('''
+ visit_type_%(c_type)s(v, %(cast)s&%(prefix)s%(c_name)s, "%(name)s", %(errp)s);
+''',
+ c_type=memb.type.c_name(), prefix=prefix, cast=cast,
+ c_name=c_name(memb.name), name=memb.name,
+ errp=errparg)
+ ret += gen_err_check(err=errarg)
+
+ if memb.optional:
+ pop_indent()
+ ret += mcgen('''
+ }
+''')
+ return ret
+
+
#
# Common command line parsing
#
--
2.4.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PULL v2 20/20] qapi: Simplify gen_visit_fields() error handling
2015-10-12 18:19 [Qemu-devel] [PULL v2 00/20] QAPI patches Markus Armbruster
` (18 preceding siblings ...)
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 19/20] qapi: Share gen_visit_fields() Markus Armbruster
@ 2015-10-12 18:19 ` Markus Armbruster
2015-10-13 10:55 ` [Qemu-devel] [PULL v2 00/20] QAPI patches Peter Maydell
20 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2015-10-12 18:19 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
Since we have consolidated all generated code to use 'err' as
the name of the local variable for error detection, we can
simplify the decision on whether to skip error detection (useful
for deallocation paths) to be a boolean.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-18-git-send-email-eblake@redhat.com>
[Change to gen_visit_fields() simplified]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/qapi-commands.py | 4 +---
scripts/qapi.py | 20 ++++++++++----------
2 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 9d214a6..43a893b 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -101,19 +101,17 @@ def gen_marshal_input_visit(arg_type, dealloc=False):
return ret
if dealloc:
- errarg = None
ret += mcgen('''
qmp_input_visitor_cleanup(qiv);
qdv = qapi_dealloc_visitor_new();
v = qapi_dealloc_get_visitor(qdv);
''')
else:
- errarg = 'err'
ret += mcgen('''
v = qmp_input_get_visitor(qiv);
''')
- ret += gen_visit_fields(arg_type.members, errarg=errarg)
+ ret += gen_visit_fields(arg_type.members, skiperr=dealloc)
if dealloc:
ret += mcgen('''
diff --git a/scripts/qapi.py b/scripts/qapi.py
index ada6380..26cff3f 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1537,23 +1537,23 @@ def gen_params(arg_type, extra):
return ret
-def gen_err_check(err='err', label='out'):
- if not err:
+def gen_err_check(label='out', skiperr=False):
+ if skiperr:
return ''
return mcgen('''
- if (%(err)s) {
+ if (err) {
goto %(label)s;
}
''',
- err=err, label=label)
+ label=label)
-def gen_visit_fields(members, prefix='', need_cast=False, errarg='err'):
+def gen_visit_fields(members, prefix='', need_cast=False, skiperr=False):
ret = ''
- if errarg:
- errparg = '&' + errarg
- else:
+ if skiperr:
errparg = 'NULL'
+ else:
+ errparg = '&err'
for memb in members:
if memb.optional:
@@ -1562,7 +1562,7 @@ def gen_visit_fields(members, prefix='', need_cast=False, errarg='err'):
''',
prefix=prefix, c_name=c_name(memb.name),
name=memb.name, errp=errparg)
- ret += gen_err_check(err=errarg)
+ ret += gen_err_check(skiperr=skiperr)
ret += mcgen('''
if (%(prefix)shas_%(c_name)s) {
''',
@@ -1581,7 +1581,7 @@ def gen_visit_fields(members, prefix='', need_cast=False, errarg='err'):
c_type=memb.type.c_name(), prefix=prefix, cast=cast,
c_name=c_name(memb.name), name=memb.name,
errp=errparg)
- ret += gen_err_check(err=errarg)
+ ret += gen_err_check(skiperr=skiperr)
if memb.optional:
pop_indent()
--
2.4.3
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PULL v2 00/20] QAPI patches
2015-10-12 18:19 [Qemu-devel] [PULL v2 00/20] QAPI patches Markus Armbruster
` (19 preceding siblings ...)
2015-10-12 18:19 ` [Qemu-devel] [PULL v2 20/20] qapi: Simplify gen_visit_fields() error handling Markus Armbruster
@ 2015-10-13 10:55 ` Peter Maydell
20 siblings, 0 replies; 24+ messages in thread
From: Peter Maydell @ 2015-10-13 10:55 UTC (permalink / raw)
To: Markus Armbruster; +Cc: QEMU Developers
On 12 October 2015 at 19:19, Markus Armbruster <armbru@redhat.com> wrote:
> The following changes since commit 5451316ed07b758a187dedf21047bed8f843f7f1:
>
> Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2015-10-12 15:52:54 +0100)
>
> are available in the git repository at:
>
> git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2015-10-12
>
> for you to fetch changes up to 18bdbc3ac8b477e160d56aa6ecd6942495ce44d0:
>
> qapi: Simplify gen_visit_fields() error handling (2015-10-12 18:46:50 +0200)
>
> ----------------------------------------------------------------
> QAPI patches
>
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 24+ messages in thread