[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v4 06/19] qapi: Better error messages for bad enums
From: |
Eric Blake |
Subject: |
[Qemu-devel] [PATCH v4 06/19] qapi: Better error messages for bad enums |
Date: |
Fri, 19 Sep 2014 16:24:51 -0600 |
The previous commit demonstrated that the generator had several
flaws with less-than-perfect enums:
- an enum that listed the same string twice (or two variant
strings that map to the same C enum) ended up generating an
invalid C enum
- because the generator adds a _MAX terminator to each enum,
the use of an enum member 'max' can also cause this clash
- if an enum omits 'data', the generator left a python stack
trace rather than a graceful message
- an enum used a non-array 'data' was silently accepted by
the parser
- an enum that used non-string members in the 'data' member
was silently accepted by the parser
Add check_enum to cover these situations, and update testcases
to match. While valid .json files won't trigger any of these
cases, we might as well be nicer to developers that make a typo
while trying to add new QAPI code.
Signed-off-by: Eric Blake <address@hidden>
---
scripts/qapi.py | 32 ++++++++++++++++++++++++++++----
tests/qapi-schema/enum-clash-member.err | 1 +
tests/qapi-schema/enum-clash-member.exit | 2 +-
tests/qapi-schema/enum-clash-member.json | 2 +-
tests/qapi-schema/enum-clash-member.out | 3 ---
tests/qapi-schema/enum-dict-member.err | 1 +
tests/qapi-schema/enum-dict-member.exit | 2 +-
tests/qapi-schema/enum-dict-member.json | 2 +-
tests/qapi-schema/enum-dict-member.out | 3 ---
tests/qapi-schema/enum-max-member.err | 1 +
tests/qapi-schema/enum-max-member.exit | 2 +-
tests/qapi-schema/enum-max-member.json | 4 ++--
tests/qapi-schema/enum-max-member.out | 3 ---
tests/qapi-schema/enum-missing-data.err | 7 +------
tests/qapi-schema/enum-missing-data.json | 2 +-
tests/qapi-schema/enum-wrong-data.err | 1 +
tests/qapi-schema/enum-wrong-data.exit | 2 +-
tests/qapi-schema/enum-wrong-data.json | 2 +-
tests/qapi-schema/enum-wrong-data.out | 3 ---
19 files changed, 43 insertions(+), 32 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 77d46aa..85aa8bf 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -2,7 +2,7 @@
# QAPI helper library
#
# Copyright IBM, Corp. 2011
-# Copyright (c) 2013 Red Hat Inc.
+# Copyright (c) 2013-2014 Red Hat Inc.
#
# Authors:
# Anthony Liguori <address@hidden>
@@ -316,13 +316,37 @@ def check_union(expr, expr_info):
# Todo: add checking for values. Key is checked as above, value can be
# also checked here, but we need more functions to handle array case.
+def check_enum(expr, expr_info):
+ name = expr['enum']
+ members = expr.get('data')
+ values = { 'MAX': '(automatic)' }
+
+ if not isinstance(members, list):
+ raise QAPIExprError(expr_info,
+ "enum '%s' requires an array for 'data'" % name)
+ for member in members:
+ if not isinstance(member, basestring):
+ raise QAPIExprError(expr_info,
+ "enum '%s' member '%s' is not a string"
+ % (name, member))
+ key = _generate_enum_string(member)
+ if key in values:
+ raise QAPIExprError(expr_info,
+ "enum '%s' member '%s' clashes with '%s'"
+ % (name, member, values[key]))
+ values[key] = member
+
def check_exprs(schema):
for expr_elem in schema.exprs:
expr = expr_elem['expr']
+ info = expr_elem['info']
+
if expr.has_key('union'):
- check_union(expr, expr_elem['info'])
+ check_union(expr, info)
if expr.has_key('event'):
- check_event(expr, expr_elem['info'])
+ check_event(expr, info)
+ if expr.has_key('enum'):
+ check_enum(expr, info)
def parse_schema(input_file):
try:
@@ -336,7 +360,7 @@ def parse_schema(input_file):
for expr_elem in schema.exprs:
expr = expr_elem['expr']
if expr.has_key('enum'):
- add_enum(expr['enum'], expr['data'])
+ add_enum(expr['enum'], expr.get('data'))
elif expr.has_key('union'):
add_union(expr)
elif expr.has_key('type'):
diff --git a/tests/qapi-schema/enum-clash-member.err
b/tests/qapi-schema/enum-clash-member.err
index e69de29..3731fc2 100644
--- a/tests/qapi-schema/enum-clash-member.err
+++ b/tests/qapi-schema/enum-clash-member.err
@@ -0,0 +1 @@
+tests/qapi-schema/enum-clash-member.json:2: enum 'MyEnum' member 'ONE' clashes
with 'one'
diff --git a/tests/qapi-schema/enum-clash-member.exit
b/tests/qapi-schema/enum-clash-member.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/enum-clash-member.exit
+++ b/tests/qapi-schema/enum-clash-member.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/enum-clash-member.json
b/tests/qapi-schema/enum-clash-member.json
index cb4b428..c668ff5 100644
--- a/tests/qapi-schema/enum-clash-member.json
+++ b/tests/qapi-schema/enum-clash-member.json
@@ -1,2 +1,2 @@
-# FIXME: we should reject enums where members will clash in C switch
+# we reject enums where members will clash in C switch
{ 'enum': 'MyEnum', 'data': [ 'one', 'ONE' ] }
diff --git a/tests/qapi-schema/enum-clash-member.out
b/tests/qapi-schema/enum-clash-member.out
index 0814459..e69de29 100644
--- a/tests/qapi-schema/enum-clash-member.out
+++ b/tests/qapi-schema/enum-clash-member.out
@@ -1,3 +0,0 @@
-[OrderedDict([('enum', 'MyEnum'), ('data', ['one', 'ONE'])])]
-[{'enum_name': 'MyEnum', 'enum_values': ['one', 'ONE']}]
-[]
diff --git a/tests/qapi-schema/enum-dict-member.err
b/tests/qapi-schema/enum-dict-member.err
index e69de29..f5a8ffe 100644
--- a/tests/qapi-schema/enum-dict-member.err
+++ b/tests/qapi-schema/enum-dict-member.err
@@ -0,0 +1 @@
+tests/qapi-schema/enum-dict-member.json:2: enum 'MyEnum' member
'OrderedDict([('value', 'str')])' is not a string
diff --git a/tests/qapi-schema/enum-dict-member.exit
b/tests/qapi-schema/enum-dict-member.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/enum-dict-member.exit
+++ b/tests/qapi-schema/enum-dict-member.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/enum-dict-member.json
b/tests/qapi-schema/enum-dict-member.json
index de4d6bf..79672e0 100644
--- a/tests/qapi-schema/enum-dict-member.json
+++ b/tests/qapi-schema/enum-dict-member.json
@@ -1,2 +1,2 @@
-# FIXME: we should reject any enum member that is not a string
+# we reject any enum member that is not a string
{ 'enum': 'MyEnum', 'data': [ { 'value': 'str' } ] }
diff --git a/tests/qapi-schema/enum-dict-member.out
b/tests/qapi-schema/enum-dict-member.out
index 8b293f8..e69de29 100644
--- a/tests/qapi-schema/enum-dict-member.out
+++ b/tests/qapi-schema/enum-dict-member.out
@@ -1,3 +0,0 @@
-[OrderedDict([('enum', 'MyEnum'), ('data', [OrderedDict([('value',
'str')])])])]
-[{'enum_name': 'MyEnum', 'enum_values': [OrderedDict([('value', 'str')])]}]
-[]
diff --git a/tests/qapi-schema/enum-max-member.err
b/tests/qapi-schema/enum-max-member.err
index e69de29..a6c5db1 100644
--- a/tests/qapi-schema/enum-max-member.err
+++ b/tests/qapi-schema/enum-max-member.err
@@ -0,0 +1 @@
+tests/qapi-schema/enum-max-member.json:3: enum 'MyEnum' member 'max' clashes
with '(automatic)'
diff --git a/tests/qapi-schema/enum-max-member.exit
b/tests/qapi-schema/enum-max-member.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/enum-max-member.exit
+++ b/tests/qapi-schema/enum-max-member.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/enum-max-member.json
b/tests/qapi-schema/enum-max-member.json
index ea854c4..6af4662 100644
--- a/tests/qapi-schema/enum-max-member.json
+++ b/tests/qapi-schema/enum-max-member.json
@@ -1,3 +1,3 @@
-# FIXME: we should either reject user-supplied 'max', or munge the implicit
-# max value we generate at the end of an array
+# we reject user-supplied 'max' for clashing with implicit enum end
+# FIXME: should we instead munge the the implicit value to avoid the clash?
{ 'enum': 'MyEnum', 'data': [ 'max' ] }
diff --git a/tests/qapi-schema/enum-max-member.out
b/tests/qapi-schema/enum-max-member.out
index c933044..e69de29 100644
--- a/tests/qapi-schema/enum-max-member.out
+++ b/tests/qapi-schema/enum-max-member.out
@@ -1,3 +0,0 @@
-[OrderedDict([('enum', 'MyEnum'), ('data', ['max'])])]
-[{'enum_name': 'MyEnum', 'enum_values': ['max']}]
-[]
diff --git a/tests/qapi-schema/enum-missing-data.err
b/tests/qapi-schema/enum-missing-data.err
index 1fec213..4b8b59e 100644
--- a/tests/qapi-schema/enum-missing-data.err
+++ b/tests/qapi-schema/enum-missing-data.err
@@ -1,6 +1 @@
-Traceback (most recent call last):
- File "tests/qapi-schema/test-qapi.py", line 19, in <module>
- exprs = parse_schema(sys.argv[1])
- File "scripts/qapi.py", line 339, in parse_schema
- add_enum(expr['enum'], expr['data'])
-KeyError: 'data'
+tests/qapi-schema/enum-missing-data.json:2: enum 'MyEnum' requires an array
for 'data'
diff --git a/tests/qapi-schema/enum-missing-data.json
b/tests/qapi-schema/enum-missing-data.json
index 01f3f32..558fd35 100644
--- a/tests/qapi-schema/enum-missing-data.json
+++ b/tests/qapi-schema/enum-missing-data.json
@@ -1,2 +1,2 @@
-# FIXME: we should require that all QAPI enums have a data array
+# we require that all QAPI enums have a data array
{ 'enum': 'MyEnum' }
diff --git a/tests/qapi-schema/enum-wrong-data.err
b/tests/qapi-schema/enum-wrong-data.err
index e69de29..0d14e80 100644
--- a/tests/qapi-schema/enum-wrong-data.err
+++ b/tests/qapi-schema/enum-wrong-data.err
@@ -0,0 +1 @@
+tests/qapi-schema/enum-wrong-data.json:2: enum 'MyEnum' requires an array for
'data'
diff --git a/tests/qapi-schema/enum-wrong-data.exit
b/tests/qapi-schema/enum-wrong-data.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/enum-wrong-data.exit
+++ b/tests/qapi-schema/enum-wrong-data.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/enum-wrong-data.json
b/tests/qapi-schema/enum-wrong-data.json
index 61d25ec..7b3e255 100644
--- a/tests/qapi-schema/enum-wrong-data.json
+++ b/tests/qapi-schema/enum-wrong-data.json
@@ -1,2 +1,2 @@
-# FIXME: we should require that all qapi enums have an array for data
+# we require that all qapi enums have an array for data
{ 'enum': 'MyEnum', 'data': { 'value': 'str' } }
diff --git a/tests/qapi-schema/enum-wrong-data.out
b/tests/qapi-schema/enum-wrong-data.out
index 28d2211..e69de29 100644
--- a/tests/qapi-schema/enum-wrong-data.out
+++ b/tests/qapi-schema/enum-wrong-data.out
@@ -1,3 +0,0 @@
-[OrderedDict([('enum', 'MyEnum'), ('data', OrderedDict([('value', 'str')]))])]
-[{'enum_name': 'MyEnum', 'enum_values': OrderedDict([('value', 'str')])}]
-[]
--
1.9.3
- [Qemu-devel] [PATCH v4 12/19] qapi: Add some type check tests, (continued)
[Qemu-devel] [PATCH v4 19/19] qapi: Drop support for inline subtypes, Eric Blake, 2014/09/19
[Qemu-devel] [PATCH v4 06/19] qapi: Better error messages for bad enums,
Eric Blake <=
[Qemu-devel] [PATCH v4 18/19] qapi: Drop inline subtype in query-pci, Eric Blake, 2014/09/19
[Qemu-devel] [PATCH v4 15/19] qapi: Merge UserDefTwo and UserDefNested in tests, Eric Blake, 2014/09/19
[Qemu-devel] [PATCH v4 14/19] qapi: More rigorous checking for type safety bypass, Eric Blake, 2014/09/19
[Qemu-devel] [PATCH v4 17/19] qapi: Drop inline subtype in query-version, Eric Blake, 2014/09/19