]> git.proxmox.com Git - mirror_qemu.git/commitdiff
qapi: Better error messages for duplicated expressions
authorEric Blake <eblake@redhat.com>
Mon, 4 May 2015 15:05:17 +0000 (09:05 -0600)
committerMarkus Armbruster <armbru@redhat.com>
Tue, 5 May 2015 16:39:01 +0000 (18:39 +0200)
The previous commit demonstrated that the generator overlooked
duplicate expressions:
- a complex type or command reusing a built-in type name
- redeclaration of a type name, whether by the same or different
metatype
- redeclaration of a command or event
- collision of a type with implicit 'Kind' enum for a union
- collision with an implicit MAX enum constant

Since the c_type() function in the generator treats all names
as being in the same namespace, this patch adds a global array
to track all known names and their source, to prevent collisions
before it can cause further problems.  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 <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
29 files changed:
scripts/qapi.py
tests/qapi-schema/command-int.err
tests/qapi-schema/command-int.exit
tests/qapi-schema/command-int.json
tests/qapi-schema/command-int.out
tests/qapi-schema/enum-union-clash.err
tests/qapi-schema/enum-union-clash.exit
tests/qapi-schema/enum-union-clash.json
tests/qapi-schema/enum-union-clash.out
tests/qapi-schema/event-max.err
tests/qapi-schema/event-max.exit
tests/qapi-schema/event-max.json
tests/qapi-schema/event-max.out
tests/qapi-schema/redefined-builtin.err
tests/qapi-schema/redefined-builtin.exit
tests/qapi-schema/redefined-builtin.json
tests/qapi-schema/redefined-builtin.out
tests/qapi-schema/redefined-command.err
tests/qapi-schema/redefined-command.exit
tests/qapi-schema/redefined-command.json
tests/qapi-schema/redefined-command.out
tests/qapi-schema/redefined-event.err
tests/qapi-schema/redefined-event.exit
tests/qapi-schema/redefined-event.json
tests/qapi-schema/redefined-event.out
tests/qapi-schema/redefined-type.err
tests/qapi-schema/redefined-type.exit
tests/qapi-schema/redefined-type.json
tests/qapi-schema/redefined-type.out

index 868f08b59326ef23349aaa903099458b319f90ce..6a339d60b5b49614cfc3fbe415b538eb55a2905a 100644 (file)
@@ -32,6 +32,12 @@ builtin_types = {
     'size':     'QTYPE_QINT',
 }
 
+enum_types = []
+struct_types = []
+union_types = []
+events = []
+all_names = {}
+
 def error_path(parent):
     res = ""
     while parent:
@@ -256,7 +262,14 @@ def discriminator_find_enum_define(expr):
     return find_enum(discriminator_type)
 
 def check_event(expr, expr_info):
+    global events
+    name = expr['event']
     params = expr.get('data')
+
+    if name.upper() == 'MAX':
+        raise QAPIExprError(expr_info, "Event name 'MAX' cannot be created")
+    events.append(name)
+
     if params:
         for argname, argentry, optional, structured in parse_args(params):
             if structured:
@@ -430,6 +443,9 @@ def check_keys(expr_elem, meta, required, optional=[]):
 
 
 def parse_schema(input_file):
+    global all_names
+    exprs = []
+
     # First pass: read entire file into memory
     try:
         schema = QAPISchema(open(input_file, "r"))
@@ -437,30 +453,34 @@ def parse_schema(input_file):
         print >>sys.stderr, e
         exit(1)
 
-    exprs = []
-
     try:
         # Next pass: learn the types and check for valid expression keys. At
         # this point, top-level 'include' has already been flattened.
+        for builtin in builtin_types.keys():
+            all_names[builtin] = 'built-in'
         for expr_elem in schema.exprs:
             expr = expr_elem['expr']
+            info = expr_elem['info']
             if expr.has_key('enum'):
                 check_keys(expr_elem, 'enum', ['data'])
-                add_enum(expr['enum'], expr['data'])
+                add_enum(expr['enum'], info, expr['data'])
             elif expr.has_key('union'):
                 check_keys(expr_elem, 'union', ['data'],
                            ['base', 'discriminator'])
-                add_union(expr)
+                add_union(expr, info)
             elif expr.has_key('alternate'):
                 check_keys(expr_elem, 'alternate', ['data'])
+                add_name(expr['alternate'], info, 'alternate')
             elif expr.has_key('type'):
                 check_keys(expr_elem, 'type', ['data'], ['base'])
-                add_struct(expr)
+                add_struct(expr, info)
             elif expr.has_key('command'):
                 check_keys(expr_elem, 'command', [],
                            ['data', 'returns', 'gen', 'success-response'])
+                add_name(expr['command'], info, 'command')
             elif expr.has_key('event'):
                 check_keys(expr_elem, 'event', [], ['data'])
+                add_name(expr['event'], info, 'event')
             else:
                 raise QAPIExprError(expr_elem['info'],
                                     "Expression is missing metatype")
@@ -471,9 +491,11 @@ def parse_schema(input_file):
             expr = expr_elem['expr']
             if expr.has_key('union'):
                 if not discriminator_find_enum_define(expr):
-                    add_enum('%sKind' % expr['union'])
+                    add_enum('%sKind' % expr['union'], expr_elem['info'],
+                             implicit=True)
             elif expr.has_key('alternate'):
-                add_enum('%sKind' % expr['alternate'])
+                add_enum('%sKind' % expr['alternate'], expr_elem['info'],
+                         implicit=True)
 
         # Final pass - validate that exprs make sense
         check_exprs(schema)
@@ -567,12 +589,22 @@ def type_name(name):
         return c_list_type(name[0])
     return name
 
-enum_types = []
-struct_types = []
-union_types = []
+def add_name(name, info, meta, implicit = False):
+    global all_names
+    if name in all_names:
+        raise QAPIExprError(info,
+                            "%s '%s' is already defined"
+                            % (all_names[name], name))
+    if not implicit and name[-4:] == 'Kind':
+        raise QAPIExprError(info,
+                            "%s '%s' should not end in 'Kind'"
+                            % (meta, name))
+    all_names[name] = meta
 
-def add_struct(definition):
+def add_struct(definition, info):
     global struct_types
+    name = definition['type']
+    add_name(name, info, 'struct')
     struct_types.append(definition)
 
 def find_struct(name):
@@ -582,8 +614,10 @@ def find_struct(name):
             return struct
     return None
 
-def add_union(definition):
+def add_union(definition, info):
     global union_types
+    name = definition['union']
+    add_name(name, info, 'union')
     union_types.append(definition)
 
 def find_union(name):
@@ -593,8 +627,9 @@ def find_union(name):
             return union
     return None
 
-def add_enum(name, enum_values = None):
+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):
@@ -636,7 +671,7 @@ def c_type(name, is_param=False):
         return name
     elif name == None or len(name) == 0:
         return 'void'
-    elif name == name.upper():
+    elif name in events:
         return '%sEvent *%s' % (camel_case(name), eatspace)
     else:
         return '%s *%s' % (name, eatspace)
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..0f9300679b304398cf6ec9cd1fdbe51ede29d3d4 100644 (file)
@@ -0,0 +1 @@
+tests/qapi-schema/command-int.json:2: built-in 'int' is already defined
index 573541ac9702dd3969c9bc859d2b91ec1f7e6e56..d00491fd7e5bb6fa28c517a0bb32b8b506539d4d 100644 (file)
@@ -1 +1 @@
-0
+1
index fcbb643c44381ef2c138a4f5230e04db82fee783..c90d408abe6ffdbf30fa40d898860b0be0bc6a13 100644 (file)
@@ -1,3 +1,3 @@
-# FIXME: we should reject collisions between commands and types
+# we reject collisions between commands and types
 { 'command': 'int', 'data': { 'character': 'str' },
   'returns': { 'value': 'int' } }
index d8e1854a0466acc4e212570d8b9a1bbf8bda11d8..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 100644 (file)
@@ -1,3 +0,0 @@
-[OrderedDict([('command', 'int'), ('data', OrderedDict([('character', 'str')])), ('returns', OrderedDict([('value', 'int')]))])]
-[]
-[]
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..c04e1a8064b433d3fdc9604eb8da2b5b8a7488a1 100644 (file)
@@ -0,0 +1 @@
+tests/qapi-schema/enum-union-clash.json:2: enum 'UnionKind' should not end in 'Kind'
index 573541ac9702dd3969c9bc859d2b91ec1f7e6e56..d00491fd7e5bb6fa28c517a0bb32b8b506539d4d 100644 (file)
@@ -1 +1 @@
-0
+1
index 714ff6d556015d49f64beb395c642b3042c78162..593282b6cff69dc02e4c923af52fc8aa3325daa8 100644 (file)
@@ -1,4 +1,4 @@
-# FIXME: we should reject types that would conflict with implicit union enum
+# we reject types that would conflict with implicit union enum
 { 'enum': 'UnionKind', 'data': [ 'oops' ] }
 { 'union': 'Union',
   'data': { 'a': 'int' } }
index d45f5e8a6946253783588a9414f0f7b224f8d24b..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 100644 (file)
@@ -1,5 +0,0 @@
-[OrderedDict([('enum', 'UnionKind'), ('data', ['oops'])]),
- OrderedDict([('union', 'Union'), ('data', OrderedDict([('a', 'int')]))])]
-[{'enum_name': 'UnionKind', 'enum_values': ['oops']},
- {'enum_name': 'UnionKind', 'enum_values': None}]
-[]
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..c85653437908158f030131390fb960f89f72c649 100644 (file)
@@ -0,0 +1 @@
+tests/qapi-schema/event-max.json:2: Event name 'MAX' cannot be created
index 573541ac9702dd3969c9bc859d2b91ec1f7e6e56..d00491fd7e5bb6fa28c517a0bb32b8b506539d4d 100644 (file)
@@ -1 +1 @@
-0
+1
index 997c61c5115b73c46fd19408b69b99796f1afdbe..f3d7de2a30552640d2f9378c52970e03c270d0dc 100644 (file)
@@ -1,2 +1,2 @@
-# FIXME: an event named 'MAX' would conflict with implicit C enum
+# an event named 'MAX' would conflict with implicit C enum
 { 'event': 'MAX' }
index 010c42b3a903532380fc2b1fbd6eb90b47253527..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 100644 (file)
@@ -1,3 +0,0 @@
-[OrderedDict([('event', 'MAX')])]
-[]
-[]
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..b2757225c4ea5aecaa2ecc02ae2acca023e57659 100644 (file)
@@ -0,0 +1 @@
+tests/qapi-schema/redefined-builtin.json:2: built-in 'size' is already defined
index 573541ac9702dd3969c9bc859d2b91ec1f7e6e56..d00491fd7e5bb6fa28c517a0bb32b8b506539d4d 100644 (file)
@@ -1 +1 @@
-0
+1
index a10050dc73d08adecd8bd7aeb451d2a464d550db..df328ccc6622fc3b6e0b1042eef9230f99eddcd4 100644 (file)
@@ -1,2 +1,2 @@
-# FIXME: we should reject types that duplicate builtin names
+# we reject types that duplicate builtin names
 { 'type': 'size', 'data': { 'myint': 'size' } }
index b0a9aea5484fd30dc9abc4ea6cda13a956032047..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 100644 (file)
@@ -1,3 +0,0 @@
-[OrderedDict([('type', 'size'), ('data', OrderedDict([('myint', 'size')]))])]
-[]
-[OrderedDict([('type', 'size'), ('data', OrderedDict([('myint', 'size')]))])]
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..82ae256e639e8866926572ac7be6329c2a8738b6 100644 (file)
@@ -0,0 +1 @@
+tests/qapi-schema/redefined-command.json:3: command 'foo' is already defined
index 573541ac9702dd3969c9bc859d2b91ec1f7e6e56..d00491fd7e5bb6fa28c517a0bb32b8b506539d4d 100644 (file)
@@ -1 +1 @@
-0
+1
index d8c9975e1c7103b23b19cb94848e24be159cfe30..247e4019483eca0e15aa0576fe6300fa71deca4a 100644 (file)
@@ -1,3 +1,3 @@
-# FIXME: we should reject commands defined more than once
+# we reject commands defined more than once
 { 'command': 'foo', 'data': { 'one': 'str' } }
 { 'command': 'foo', 'data': { '*two': 'str' } }
index 44ddba60d5645bd2ff6d4818c7dcd450209d4c08..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 100644 (file)
@@ -1,4 +0,0 @@
-[OrderedDict([('command', 'foo'), ('data', OrderedDict([('one', 'str')]))]),
- OrderedDict([('command', 'foo'), ('data', OrderedDict([('*two', 'str')]))])]
-[]
-[]
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..35429cb48186a0bd2d3798c458c06c1f71e1bf45 100644 (file)
@@ -0,0 +1 @@
+tests/qapi-schema/redefined-event.json:3: event 'EVENT_A' is already defined
index 573541ac9702dd3969c9bc859d2b91ec1f7e6e56..d00491fd7e5bb6fa28c517a0bb32b8b506539d4d 100644 (file)
@@ -1 +1 @@
-0
+1
index 152cce720fc917e553204afb056f22f989861239..7717e91c18c4b44f33211e2a2696da2d921e1b1e 100644 (file)
@@ -1,3 +1,3 @@
-# FIXME: we should reject duplicate events
+# we reject duplicate events
 { 'event': 'EVENT_A', 'data': { 'myint': 'int' } }
 { 'event': 'EVENT_A', 'data': { 'myint': 'int' } }
index 261b183fc99f94ec84a8f20cac8ad259945316e2..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 100644 (file)
@@ -1,4 +0,0 @@
-[OrderedDict([('event', 'EVENT_A'), ('data', OrderedDict([('myint', 'int')]))]),
- OrderedDict([('event', 'EVENT_A'), ('data', OrderedDict([('myint', 'int')]))])]
-[]
-[]
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..06ea78c4781e479cea71a15b34609adc36ba36cd 100644 (file)
@@ -0,0 +1 @@
+tests/qapi-schema/redefined-type.json:3: struct 'foo' is already defined
index 573541ac9702dd3969c9bc859d2b91ec1f7e6e56..d00491fd7e5bb6fa28c517a0bb32b8b506539d4d 100644 (file)
@@ -1 +1 @@
-0
+1
index 7972194824859dff18e63463edfb0f95a69512a1..e6a5f24ca98b4231f50f572595efd5b1de8c96ec 100644 (file)
@@ -1,3 +1,3 @@
-# FIXME: we should reject types defined more than once
+# we reject types defined more than once
 { 'type': 'foo', 'data': { 'one': 'str' } }
 { 'enum': 'foo', 'data': [ 'two' ] }
index b509e5776cb5385cef89fb42e301ef458e48eaed..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 100644 (file)
@@ -1,4 +0,0 @@
-[OrderedDict([('type', 'foo'), ('data', OrderedDict([('one', 'str')]))]),
- OrderedDict([('enum', 'foo'), ('data', ['two'])])]
-[{'enum_name': 'foo', 'enum_values': ['two']}]
-[OrderedDict([('type', 'foo'), ('data', OrderedDict([('one', 'str')]))])]