[Qemu-devel] [PATCH v4 13/19] qapi: More rigourous checking of types

[Eric Blake]

Now that we know every expression is valid with regards to
its keys, we can add further tests that those keys refer to
valid types.  With this patch, all references to a type (the
'data': of command, type, union, and event, and the 'returns':
of command) must resolve to a builtin or another type declared
by the current qapi parse; this includes recursing into each
member of a data dictionary.  Dealing with '**' and nested
sub-structs will be done in later patches.

Update the testsuite to match improved output.

Signed-off-by: Eric Blake <address@hidden>
---
 scripts/qapi.py                              | 73 ++++++++++++++++++++++++++--
 tests/qapi-schema/data-array-empty.err       |  1 +
 tests/qapi-schema/data-array-empty.exit      |  2 +-
 tests/qapi-schema/data-array-empty.json      |  2 +-
 tests/qapi-schema/data-array-empty.out       |  3 --
 tests/qapi-schema/data-array-unknown.err     |  1 +
 tests/qapi-schema/data-array-unknown.exit    |  2 +-
 tests/qapi-schema/data-array-unknown.json    |  2 +-
 tests/qapi-schema/data-array-unknown.out     |  3 --
 tests/qapi-schema/data-int.err               |  1 +
 tests/qapi-schema/data-int.exit              |  2 +-
 tests/qapi-schema/data-int.json              |  2 +-
 tests/qapi-schema/data-int.out               |  3 --
 tests/qapi-schema/data-member-array-bad.err  |  1 +
 tests/qapi-schema/data-member-array-bad.exit |  2 +-
 tests/qapi-schema/data-member-array-bad.json |  2 +-
 tests/qapi-schema/data-member-array-bad.out  |  3 --
 tests/qapi-schema/data-member-unknown.err    |  1 +
 tests/qapi-schema/data-member-unknown.exit   |  2 +-
 tests/qapi-schema/data-member-unknown.json   |  2 +-
 tests/qapi-schema/data-member-unknown.out    |  3 --
 tests/qapi-schema/data-unknown.err           |  1 +
 tests/qapi-schema/data-unknown.exit          |  2 +-
 tests/qapi-schema/data-unknown.json          |  2 +-
 tests/qapi-schema/data-unknown.out           |  3 --
 tests/qapi-schema/returns-array-bad.err      |  1 +
 tests/qapi-schema/returns-array-bad.exit     |  2 +-
 tests/qapi-schema/returns-array-bad.json     |  2 +-
 tests/qapi-schema/returns-array-bad.out      |  3 --
 tests/qapi-schema/returns-unknown.err        |  1 +
 tests/qapi-schema/returns-unknown.exit       |  2 +-
 tests/qapi-schema/returns-unknown.json       |  2 +-
 tests/qapi-schema/returns-unknown.out        |  3 --
 33 files changed, 93 insertions(+), 44 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index bf243fa..20c0ce9 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -255,6 +255,52 @@ def discriminator_find_enum_define(expr):

     return find_enum(discriminator_type)

+def check_type(expr_info, source, data, allow_array = False,
+               allowed_names = [], allow_dict = True):
+    global all_types
+
+    if data is None:
+        return
+
+    if data == '**':
+        return
+
+    # Check if array type for data is okay
+    if isinstance(data, list):
+        if not allow_array:
+            raise QAPIExprError(expr_info,
+                                "%s cannot be an array" % source)
+        if len(data) != 1 or not isinstance(data[0], basestring):
+            raise QAPIExprError(expr_info,
+                                "%s: array type must contain single type name"
+                                % source)
+        data = data[0]
+
+    # Check if type name for data is okay
+    if isinstance(data, basestring):
+        if not data in all_types:
+            raise QAPIExprError(expr_info,
+                                "%s references unknown type '%s'"
+                                % (source, data))
+        if not all_types[data] in allowed_names:
+            raise QAPIExprError(expr_info,
+                                "%s cannot reference %s type '%s'"
+                                % (source, all_types[data], data))
+        return
+
+    # data is a dictionary, check that each member is okay
+    if not isinstance(data, OrderedDict):
+        raise QAPIExprError(expr_info,
+                            "%s should be a dictionary" % source)
+    if not allow_dict:
+        raise QAPIExprError(expr_info,
+                            "%s should be a type name" % source)
+    for (key, value) in data.items():
+        check_type(expr_info, "member '%s' of %s" % (key, source), value,
+                   allow_array=True,
+                   allowed_names=['built-in', 'union', 'struct', 'enum'],
+                   allow_dict=True)
+
 def check_command(expr, expr_info):
     global commands
     name = expr['command']
@@ -262,6 +308,15 @@ def check_command(expr, expr_info):
         raise QAPIExprError(expr_info,
                             "command '%s' is already defined" % name)
     commands.append(name)
+    check_type(expr_info, "'data' for command '%s'" % name,
+               expr.get('data'), allow_array=True,
+               allowed_names=['union', 'struct'])
+    check_type(expr_info, "'base' for command '%s'" % name,
+               expr.get('base'), allowed_names=['struct'],
+               allow_dict=False)
+    check_type(expr_info, "'returns' for command '%s'" % name,
+               expr.get('returns'), allow_array=True,
+               allowed_names=['built-in', 'union', 'struct', 'enum'])

 def check_event(expr, expr_info):
     global events
@@ -271,6 +326,8 @@ def check_event(expr, expr_info):
         raise QAPIExprError(expr_info,
                             "event '%s' is already defined" % name)
     events.append(name)
+    check_type(expr_info, "'data' for event '%s'" % name,
+               expr.get('data'), allowed_names=['union', 'struct'])
     if params:
         for argname, argentry, optional, structured in parse_args(params):
             if structured:
@@ -357,19 +414,27 @@ def check_enum(expr, expr_info):
                                 % (name, member, values[key]))
         values[key] = member

+def check_struct(expr, expr_info):
+    name = expr['type']
+    members = expr['data']
+
+    check_type(expr_info, "'data' for type '%s'" % name, members)
+
 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, info)
-        if expr.has_key('event'):
-            check_event(expr, info)
         if expr.has_key('enum'):
             check_enum(expr, info)
+        if expr.has_key('union'):
+            check_union(expr, info)
+        if expr.has_key('type'):
+            check_struct(expr, info)
         if expr.has_key('command'):
             check_command(expr, info)
+        if expr.has_key('event'):
+            check_event(expr, info)

 def check_keys(expr_elem, meta, required, optional=[]):
     expr = expr_elem['expr']
diff --git a/tests/qapi-schema/data-array-empty.err 
b/tests/qapi-schema/data-array-empty.err
index e69de29..94e046b 100644
--- a/tests/qapi-schema/data-array-empty.err
+++ b/tests/qapi-schema/data-array-empty.err
@@ -0,0 +1 @@
+tests/qapi-schema/data-array-empty.json:2: 'data' for command 'oops': array 
type must contain single type name
diff --git a/tests/qapi-schema/data-array-empty.exit 
b/tests/qapi-schema/data-array-empty.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/data-array-empty.exit
+++ b/tests/qapi-schema/data-array-empty.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/data-array-empty.json 
b/tests/qapi-schema/data-array-empty.json
index 41b6c1e..fdd5612 100644
--- a/tests/qapi-schema/data-array-empty.json
+++ b/tests/qapi-schema/data-array-empty.json
@@ -1,2 +1,2 @@
-# FIXME: we should reject an array for data if it does not contain a known type
+# we reject an array for data if it does not contain a known type
 { 'command': 'oops', 'data': [ ] }
diff --git a/tests/qapi-schema/data-array-empty.out 
b/tests/qapi-schema/data-array-empty.out
index 67802be..e69de29 100644
--- a/tests/qapi-schema/data-array-empty.out
+++ b/tests/qapi-schema/data-array-empty.out
@@ -1,3 +0,0 @@
-[OrderedDict([('command', 'oops'), ('data', [])])]
-[]
-[]
diff --git a/tests/qapi-schema/data-array-unknown.err 
b/tests/qapi-schema/data-array-unknown.err
index e69de29..a66c2d6 100644
--- a/tests/qapi-schema/data-array-unknown.err
+++ b/tests/qapi-schema/data-array-unknown.err
@@ -0,0 +1 @@
+tests/qapi-schema/data-array-unknown.json:2: 'data' for command 'oops' 
references unknown type 'NoSuchType'
diff --git a/tests/qapi-schema/data-array-unknown.exit 
b/tests/qapi-schema/data-array-unknown.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/data-array-unknown.exit
+++ b/tests/qapi-schema/data-array-unknown.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/data-array-unknown.json 
b/tests/qapi-schema/data-array-unknown.json
index 434fb5f..9c75b96 100644
--- a/tests/qapi-schema/data-array-unknown.json
+++ b/tests/qapi-schema/data-array-unknown.json
@@ -1,2 +1,2 @@
-# FIXME: we should reject an array for data if it does not contain a known type
+# we reject an array for data if it does not contain a known type
 { 'command': 'oops', 'data': [ 'NoSuchType' ] }
diff --git a/tests/qapi-schema/data-array-unknown.out 
b/tests/qapi-schema/data-array-unknown.out
index c3bc05e..e69de29 100644
--- a/tests/qapi-schema/data-array-unknown.out
+++ b/tests/qapi-schema/data-array-unknown.out
@@ -1,3 +0,0 @@
-[OrderedDict([('command', 'oops'), ('data', ['NoSuchType'])])]
-[]
-[]
diff --git a/tests/qapi-schema/data-int.err b/tests/qapi-schema/data-int.err
index e69de29..e1d3ed5 100644
--- a/tests/qapi-schema/data-int.err
+++ b/tests/qapi-schema/data-int.err
@@ -0,0 +1 @@
+tests/qapi-schema/data-int.json:2: 'data' for command 'oops' cannot reference 
built-in type 'int'
diff --git a/tests/qapi-schema/data-int.exit b/tests/qapi-schema/data-int.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/data-int.exit
+++ b/tests/qapi-schema/data-int.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/data-int.json b/tests/qapi-schema/data-int.json
index 37916e0..a334d92 100644
--- a/tests/qapi-schema/data-int.json
+++ b/tests/qapi-schema/data-int.json
@@ -1,2 +1,2 @@
-# FIXME: we should reject commands where data is not an array or complex type
+# we reject commands where data is not an array or complex type
 { 'command': 'oops', 'data': 'int' }
diff --git a/tests/qapi-schema/data-int.out b/tests/qapi-schema/data-int.out
index e589a4f..e69de29 100644
--- a/tests/qapi-schema/data-int.out
+++ b/tests/qapi-schema/data-int.out
@@ -1,3 +0,0 @@
-[OrderedDict([('command', 'oops'), ('data', 'int')])]
-[]
-[]
diff --git a/tests/qapi-schema/data-member-array-bad.err 
b/tests/qapi-schema/data-member-array-bad.err
index e69de29..ac45442 100644
--- a/tests/qapi-schema/data-member-array-bad.err
+++ b/tests/qapi-schema/data-member-array-bad.err
@@ -0,0 +1 @@
+tests/qapi-schema/data-member-array-bad.json:2: member 'member' of 'data' for 
command 'oops': array type must contain single type name
diff --git a/tests/qapi-schema/data-member-array-bad.exit 
b/tests/qapi-schema/data-member-array-bad.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/data-member-array-bad.exit
+++ b/tests/qapi-schema/data-member-array-bad.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/data-member-array-bad.json 
b/tests/qapi-schema/data-member-array-bad.json
index c954af1..b2ff144 100644
--- a/tests/qapi-schema/data-member-array-bad.json
+++ b/tests/qapi-schema/data-member-array-bad.json
@@ -1,2 +1,2 @@
-# FIXME: we should reject data if it does not contain a valid array type
+# we reject data if it does not contain a valid array type
 { 'command': 'oops', 'data': { 'member': [ { 'nested': 'str' } ] } }
diff --git a/tests/qapi-schema/data-member-array-bad.out 
b/tests/qapi-schema/data-member-array-bad.out
index 0e00c41..e69de29 100644
--- a/tests/qapi-schema/data-member-array-bad.out
+++ b/tests/qapi-schema/data-member-array-bad.out
@@ -1,3 +0,0 @@
-[OrderedDict([('command', 'oops'), ('data', OrderedDict([('member', 
[OrderedDict([('nested', 'str')])])]))])]
-[]
-[]
diff --git a/tests/qapi-schema/data-member-unknown.err 
b/tests/qapi-schema/data-member-unknown.err
index e69de29..89dce36 100644
--- a/tests/qapi-schema/data-member-unknown.err
+++ b/tests/qapi-schema/data-member-unknown.err
@@ -0,0 +1 @@
+tests/qapi-schema/data-member-unknown.json:2: member 'member' of 'data' for 
command 'oops' references unknown type 'NoSuchType'
diff --git a/tests/qapi-schema/data-member-unknown.exit 
b/tests/qapi-schema/data-member-unknown.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/data-member-unknown.exit
+++ b/tests/qapi-schema/data-member-unknown.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/data-member-unknown.json 
b/tests/qapi-schema/data-member-unknown.json
index 40e6252..342a41e 100644
--- a/tests/qapi-schema/data-member-unknown.json
+++ b/tests/qapi-schema/data-member-unknown.json
@@ -1,2 +1,2 @@
-# FIXME: we should reject data if it does not contain a known type
+# we reject data if it does not contain a known type
 { 'command': 'oops', 'data': { 'member': 'NoSuchType' } }
diff --git a/tests/qapi-schema/data-member-unknown.out 
b/tests/qapi-schema/data-member-unknown.out
index 36252a5..e69de29 100644
--- a/tests/qapi-schema/data-member-unknown.out
+++ b/tests/qapi-schema/data-member-unknown.out
@@ -1,3 +0,0 @@
-[OrderedDict([('command', 'oops'), ('data', OrderedDict([('member', 
'NoSuchType')]))])]
-[]
-[]
diff --git a/tests/qapi-schema/data-unknown.err 
b/tests/qapi-schema/data-unknown.err
index e69de29..d1a6086 100644
--- a/tests/qapi-schema/data-unknown.err
+++ b/tests/qapi-schema/data-unknown.err
@@ -0,0 +1 @@
+tests/qapi-schema/data-unknown.json:2: 'data' for command 'oops' references 
unknown type 'NoSuchType'
diff --git a/tests/qapi-schema/data-unknown.exit 
b/tests/qapi-schema/data-unknown.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/data-unknown.exit
+++ b/tests/qapi-schema/data-unknown.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/data-unknown.json 
b/tests/qapi-schema/data-unknown.json
index 776bd34..32aba43 100644
--- a/tests/qapi-schema/data-unknown.json
+++ b/tests/qapi-schema/data-unknown.json
@@ -1,2 +1,2 @@
-# FIXME: we should reject data if it does not contain a known type
+# we reject data if it does not contain a known type
 { 'command': 'oops', 'data': 'NoSuchType' }
diff --git a/tests/qapi-schema/data-unknown.out 
b/tests/qapi-schema/data-unknown.out
index 2c60726..e69de29 100644
--- a/tests/qapi-schema/data-unknown.out
+++ b/tests/qapi-schema/data-unknown.out
@@ -1,3 +0,0 @@
-[OrderedDict([('command', 'oops'), ('data', 'NoSuchType')])]
-[]
-[]
diff --git a/tests/qapi-schema/returns-array-bad.err 
b/tests/qapi-schema/returns-array-bad.err
index e69de29..138095c 100644
--- a/tests/qapi-schema/returns-array-bad.err
+++ b/tests/qapi-schema/returns-array-bad.err
@@ -0,0 +1 @@
+tests/qapi-schema/returns-array-bad.json:2: 'returns' for command 'oops': 
array type must contain single type name
diff --git a/tests/qapi-schema/returns-array-bad.exit 
b/tests/qapi-schema/returns-array-bad.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/returns-array-bad.exit
+++ b/tests/qapi-schema/returns-array-bad.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/returns-array-bad.json 
b/tests/qapi-schema/returns-array-bad.json
index 14882c1..09b0b1f 100644
--- a/tests/qapi-schema/returns-array-bad.json
+++ b/tests/qapi-schema/returns-array-bad.json
@@ -1,2 +1,2 @@
-# FIXME: we should reject an array return that is not a single type
+# we reject an array return that is not a single type
 { 'command': 'oops', 'returns': [ 'str', 'str' ] }
diff --git a/tests/qapi-schema/returns-array-bad.out 
b/tests/qapi-schema/returns-array-bad.out
index eccad55..e69de29 100644
--- a/tests/qapi-schema/returns-array-bad.out
+++ b/tests/qapi-schema/returns-array-bad.out
@@ -1,3 +0,0 @@
-[OrderedDict([('command', 'oops'), ('returns', ['str', 'str'])])]
-[]
-[]
diff --git a/tests/qapi-schema/returns-unknown.err 
b/tests/qapi-schema/returns-unknown.err
index e69de29..962f27c 100644
--- a/tests/qapi-schema/returns-unknown.err
+++ b/tests/qapi-schema/returns-unknown.err
@@ -0,0 +1 @@
+tests/qapi-schema/returns-unknown.json:2: 'returns' for command 'oops' 
references unknown type 'NoSuchType'
diff --git a/tests/qapi-schema/returns-unknown.exit 
b/tests/qapi-schema/returns-unknown.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/returns-unknown.exit
+++ b/tests/qapi-schema/returns-unknown.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/returns-unknown.json 
b/tests/qapi-schema/returns-unknown.json
index 61f20eb..25bd498 100644
--- a/tests/qapi-schema/returns-unknown.json
+++ b/tests/qapi-schema/returns-unknown.json
@@ -1,2 +1,2 @@
-# FIXME: we should reject returns if it does not contain a known type
+# we reject returns if it does not contain a known type
 { 'command': 'oops', 'returns': 'NoSuchType' }
diff --git a/tests/qapi-schema/returns-unknown.out 
b/tests/qapi-schema/returns-unknown.out
index a482c83..e69de29 100644
--- a/tests/qapi-schema/returns-unknown.out
+++ b/tests/qapi-schema/returns-unknown.out
@@ -1,3 +0,0 @@
-[OrderedDict([('command', 'oops'), ('returns', 'NoSuchType')])]
-[]
-[]
-- 
1.9.3