From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43286) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XV6cW-0004PL-Ph for qemu-devel@nongnu.org; Fri, 19 Sep 2014 18:25:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XV6cQ-0004ZP-W7 for qemu-devel@nongnu.org; Fri, 19 Sep 2014 18:25:40 -0400 Received: from mx1.redhat.com ([209.132.183.28]:14803) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XV6cQ-0004Wm-Gh for qemu-devel@nongnu.org; Fri, 19 Sep 2014 18:25:34 -0400 From: Eric Blake Date: Fri, 19 Sep 2014 16:24:55 -0600 Message-Id: <1411165504-18198-11-git-send-email-eblake@redhat.com> In-Reply-To: <1411165504-18198-1-git-send-email-eblake@redhat.com> References: <1411165504-18198-1-git-send-email-eblake@redhat.com> Subject: [Qemu-devel] [PATCH v4 10/19] qapi: Better error messages for duplicated expressions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Luiz Capitulino , Fam Zheng , Markus Armbruster , wenchaoqemu@gmail.com The previous commit demonstrated that the generator overlooked duplicate expressions: - a complex type reusing a built-in type name - redeclaration of a type name, whether by the same or different metatype - redeclaration of a command or event - lack of tracking of 'size' as a built-in type Add a global array to track all known types and their source, as well as enhancing check_exprs to also check for duplicate events and commands. 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 --- scripts/qapi.py | 71 +++++++++++++++++++++++++------- tests/qapi-schema/redefined-builtin.err | 1 + tests/qapi-schema/redefined-builtin.exit | 2 +- tests/qapi-schema/redefined-builtin.json | 2 +- tests/qapi-schema/redefined-builtin.out | 3 -- tests/qapi-schema/redefined-command.err | 1 + tests/qapi-schema/redefined-command.exit | 2 +- tests/qapi-schema/redefined-command.json | 2 +- tests/qapi-schema/redefined-command.out | 4 -- tests/qapi-schema/redefined-event.err | 1 + tests/qapi-schema/redefined-event.exit | 2 +- tests/qapi-schema/redefined-event.json | 2 +- tests/qapi-schema/redefined-event.out | 4 -- tests/qapi-schema/redefined-type.err | 1 + tests/qapi-schema/redefined-type.exit | 2 +- tests/qapi-schema/redefined-type.json | 2 +- tests/qapi-schema/redefined-type.out | 4 -- 17 files changed, 69 insertions(+), 37 deletions(-) diff --git a/scripts/qapi.py b/scripts/qapi.py index 8fbc45f..bf243fa 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -19,8 +19,15 @@ import sys builtin_types = [ 'str', 'int', 'number', 'bool', 'int8', 'int16', 'int32', 'int64', - 'uint8', 'uint16', 'uint32', 'uint64' + 'uint8', 'uint16', 'uint32', 'uint64', + 'size' ] +enum_types = [] +struct_types = [] +union_types = [] +all_types = {} +commands = [] +events = ['MAX'] builtin_type_qtypes = { 'str': 'QTYPE_QSTRING', @@ -248,8 +255,22 @@ def discriminator_find_enum_define(expr): return find_enum(discriminator_type) +def check_command(expr, expr_info): + global commands + name = expr['command'] + if name in commands: + raise QAPIExprError(expr_info, + "command '%s' is already defined" % name) + commands.append(name) + def check_event(expr, expr_info): + global events + name = expr['event'] params = expr.get('data') + if name in events: + raise QAPIExprError(expr_info, + "event '%s' is already defined" % name) + events.append(name) if params: for argname, argentry, optional, structured in parse_args(params): if structured: @@ -347,6 +368,8 @@ def check_exprs(schema): check_event(expr, info) if expr.has_key('enum'): check_enum(expr, info) + if expr.has_key('command'): + check_command(expr, info) def check_keys(expr_elem, meta, required, optional=[]): expr = expr_elem['expr'] @@ -370,6 +393,9 @@ def check_keys(expr_elem, meta, required, optional=[]): def parse_schema(input_file): + global all_types + exprs = [] + # First pass: read entire file into memory try: schema = QAPISchema(open(input_file, "r")) @@ -377,16 +403,17 @@ 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: + all_types[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'): # Two styles of union, based on discriminator discriminator = expr.get('discriminator') @@ -395,10 +422,10 @@ def parse_schema(input_file): else: check_keys(expr_elem, 'union', ['data'], ['base', 'discriminator']) - add_union(expr) + add_union(expr, info) 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']) @@ -414,7 +441,7 @@ 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']) # Final pass - validate that exprs make sense check_exprs(schema) @@ -508,12 +535,15 @@ def type_name(name): return c_list_type(name[0]) return name -enum_types = [] -struct_types = [] -union_types = [] - -def add_struct(definition): +def add_struct(definition, info): global struct_types + global all_types + name = definition['type'] + if name in all_types: + raise QAPIExprError(info, + "%s '%s' is already defined" + %(all_types[name], name)) + all_types[name] = 'struct' struct_types.append(definition) def find_struct(name): @@ -523,8 +553,15 @@ def find_struct(name): return struct return None -def add_union(definition): +def add_union(definition, info): global union_types + global all_types + name = definition['union'] + if name in all_types: + raise QAPIExprError(info, + "%s '%s' is already defined" + %(all_types[name], name)) + all_types[name] = 'union' union_types.append(definition) def find_union(name): @@ -534,8 +571,14 @@ def find_union(name): return union return None -def add_enum(name, enum_values = None): +def add_enum(name, info, enum_values = None): global enum_types + global all_types + if name in all_types: + raise QAPIExprError(info, + "%s '%s' is already defined" + %(all_types[name], name)) + all_types[name] = 'enum' enum_types.append({"enum_name": name, "enum_values": enum_values}) def find_enum(name): diff --git a/tests/qapi-schema/redefined-builtin.err b/tests/qapi-schema/redefined-builtin.err index e69de29..b275722 100644 --- a/tests/qapi-schema/redefined-builtin.err +++ b/tests/qapi-schema/redefined-builtin.err @@ -0,0 +1 @@ +tests/qapi-schema/redefined-builtin.json:2: built-in 'size' is already defined diff --git a/tests/qapi-schema/redefined-builtin.exit b/tests/qapi-schema/redefined-builtin.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/redefined-builtin.exit +++ b/tests/qapi-schema/redefined-builtin.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/redefined-builtin.json b/tests/qapi-schema/redefined-builtin.json index a10050d..df328cc 100644 --- a/tests/qapi-schema/redefined-builtin.json +++ b/tests/qapi-schema/redefined-builtin.json @@ -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' } } diff --git a/tests/qapi-schema/redefined-builtin.out b/tests/qapi-schema/redefined-builtin.out index b0a9aea..e69de29 100644 --- a/tests/qapi-schema/redefined-builtin.out +++ b/tests/qapi-schema/redefined-builtin.out @@ -1,3 +0,0 @@ -[OrderedDict([('type', 'size'), ('data', OrderedDict([('myint', 'size')]))])] -[] -[OrderedDict([('type', 'size'), ('data', OrderedDict([('myint', 'size')]))])] diff --git a/tests/qapi-schema/redefined-command.err b/tests/qapi-schema/redefined-command.err index e69de29..82ae256 100644 --- a/tests/qapi-schema/redefined-command.err +++ b/tests/qapi-schema/redefined-command.err @@ -0,0 +1 @@ +tests/qapi-schema/redefined-command.json:3: command 'foo' is already defined diff --git a/tests/qapi-schema/redefined-command.exit b/tests/qapi-schema/redefined-command.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/redefined-command.exit +++ b/tests/qapi-schema/redefined-command.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/redefined-command.json b/tests/qapi-schema/redefined-command.json index d8c9975..247e401 100644 --- a/tests/qapi-schema/redefined-command.json +++ b/tests/qapi-schema/redefined-command.json @@ -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' } } diff --git a/tests/qapi-schema/redefined-command.out b/tests/qapi-schema/redefined-command.out index 44ddba6..e69de29 100644 --- a/tests/qapi-schema/redefined-command.out +++ b/tests/qapi-schema/redefined-command.out @@ -1,4 +0,0 @@ -[OrderedDict([('command', 'foo'), ('data', OrderedDict([('one', 'str')]))]), - OrderedDict([('command', 'foo'), ('data', OrderedDict([('*two', 'str')]))])] -[] -[] diff --git a/tests/qapi-schema/redefined-event.err b/tests/qapi-schema/redefined-event.err index e69de29..35429cb 100644 --- a/tests/qapi-schema/redefined-event.err +++ b/tests/qapi-schema/redefined-event.err @@ -0,0 +1 @@ +tests/qapi-schema/redefined-event.json:3: event 'EVENT_A' is already defined diff --git a/tests/qapi-schema/redefined-event.exit b/tests/qapi-schema/redefined-event.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/redefined-event.exit +++ b/tests/qapi-schema/redefined-event.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/redefined-event.json b/tests/qapi-schema/redefined-event.json index 152cce7..7717e91 100644 --- a/tests/qapi-schema/redefined-event.json +++ b/tests/qapi-schema/redefined-event.json @@ -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' } } diff --git a/tests/qapi-schema/redefined-event.out b/tests/qapi-schema/redefined-event.out index 261b183..e69de29 100644 --- a/tests/qapi-schema/redefined-event.out +++ b/tests/qapi-schema/redefined-event.out @@ -1,4 +0,0 @@ -[OrderedDict([('event', 'EVENT_A'), ('data', OrderedDict([('myint', 'int')]))]), - OrderedDict([('event', 'EVENT_A'), ('data', OrderedDict([('myint', 'int')]))])] -[] -[] diff --git a/tests/qapi-schema/redefined-type.err b/tests/qapi-schema/redefined-type.err index e69de29..06ea78c 100644 --- a/tests/qapi-schema/redefined-type.err +++ b/tests/qapi-schema/redefined-type.err @@ -0,0 +1 @@ +tests/qapi-schema/redefined-type.json:3: struct 'foo' is already defined diff --git a/tests/qapi-schema/redefined-type.exit b/tests/qapi-schema/redefined-type.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/redefined-type.exit +++ b/tests/qapi-schema/redefined-type.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/redefined-type.json b/tests/qapi-schema/redefined-type.json index 7972194..e6a5f24 100644 --- a/tests/qapi-schema/redefined-type.json +++ b/tests/qapi-schema/redefined-type.json @@ -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' ] } diff --git a/tests/qapi-schema/redefined-type.out b/tests/qapi-schema/redefined-type.out index b509e57..e69de29 100644 --- a/tests/qapi-schema/redefined-type.out +++ b/tests/qapi-schema/redefined-type.out @@ -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')]))])] -- 1.9.3