From dad1fcab91bf101a02151069036d416367b59c5c Mon Sep 17 00:00:00 2001 From: Wenchao Xia Date: Tue, 4 Mar 2014 18:44:31 -0800 Subject: [PATCH 01/11] qapi script: remember explicitly defined enum values Later other scripts will need to check the enum values. Signed-off-by: Wenchao Xia Reviewed-by: Eric Blake Reviewed-by: Markus Armbruster Signed-off-by: Luiz Capitulino --- scripts/qapi.py | 16 +++++++++++----- tests/qapi-schema/comments.out | 2 +- tests/qapi-schema/qapi-schema-test.out | 10 +++++----- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/scripts/qapi.py b/scripts/qapi.py index f3c2a2037a..023930ea0c 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -169,7 +169,7 @@ def parse_schema(fp): for expr in schema.exprs: if expr.has_key('enum'): - add_enum(expr['enum']) + add_enum(expr['enum'], expr['data']) elif expr.has_key('union'): add_union(expr) add_enum('%sKind' % expr['union']) @@ -289,13 +289,19 @@ def find_union(name): return union return None -def add_enum(name): +def add_enum(name, enum_values = None): global enum_types - enum_types.append(name) + enum_types.append({"enum_name": name, "enum_values": enum_values}) + +def find_enum(name): + global enum_types + for enum in enum_types: + if enum['enum_name'] == name: + return enum + return None def is_enum(name): - global enum_types - return (name in enum_types) + return find_enum(name) != None def c_type(name): if name == 'str': diff --git a/tests/qapi-schema/comments.out b/tests/qapi-schema/comments.out index e3bd904453..4ce3dcf12f 100644 --- a/tests/qapi-schema/comments.out +++ b/tests/qapi-schema/comments.out @@ -1,3 +1,3 @@ [OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])] -['Status'] +[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}] [] diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index 89b53d4d4d..01685d4a46 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -15,11 +15,11 @@ OrderedDict([('command', 'user_def_cmd2'), ('data', OrderedDict([('ud1a', 'UserDefOne'), ('*ud1b', 'UserDefOne')])), ('returns', 'UserDefTwo')]), OrderedDict([('command', 'user_def_cmd3'), ('data', OrderedDict([('a', 'int'), ('*b', 'int')])), ('returns', 'int')]), OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))])] -['EnumOne', - 'UserDefUnionKind', - 'UserDefFlatUnionKind', - 'UserDefAnonUnionKind', - 'UserDefNativeListUnionKind'] +[{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']}, + {'enum_name': 'UserDefUnionKind', 'enum_values': None}, + {'enum_name': 'UserDefFlatUnionKind', 'enum_values': None}, + {'enum_name': 'UserDefAnonUnionKind', 'enum_values': None}, + {'enum_name': 'UserDefNativeListUnionKind', 'enum_values': None}] [OrderedDict([('type', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 'EnumOne')]))]), OrderedDict([('type', 'UserDefZero'), ('data', OrderedDict([('integer', 'int')]))]), OrderedDict([('type', 'UserDefOne'), ('base', 'UserDefZero'), ('data', OrderedDict([('string', 'str'), ('*enum1', 'EnumOne')]))]), From 4b35991a3bd5f9e03333d5b1bd4a7bcf9941aac5 Mon Sep 17 00:00:00 2001 From: Wenchao Xia Date: Tue, 4 Mar 2014 18:44:32 -0800 Subject: [PATCH 02/11] qapi script: add check for duplicated key It is bad that same key was specified twice, especially when a union has two branches with same condition. This patch can prevent it. Signed-off-by: Wenchao Xia Reviewed-by: Eric Blake Reviewed-by: Markus Armbruster Signed-off-by: Luiz Capitulino --- scripts/qapi.py | 2 ++ tests/Makefile | 3 ++- tests/qapi-schema/duplicate-key.err | 1 + tests/qapi-schema/duplicate-key.exit | 1 + tests/qapi-schema/duplicate-key.json | 2 ++ tests/qapi-schema/duplicate-key.out | 0 6 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 tests/qapi-schema/duplicate-key.err create mode 100644 tests/qapi-schema/duplicate-key.exit create mode 100644 tests/qapi-schema/duplicate-key.json create mode 100644 tests/qapi-schema/duplicate-key.out diff --git a/scripts/qapi.py b/scripts/qapi.py index 023930ea0c..d0e793452a 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -116,6 +116,8 @@ def get_members(self): if self.tok != ':': raise QAPISchemaError(self, 'Expected ":"') self.accept() + if key in expr: + raise QAPISchemaError(self, 'Duplicate key "%s"' % key) expr[key] = self.get_expr(True) if self.tok == '}': self.accept() diff --git a/tests/Makefile b/tests/Makefile index b17d41e4e2..dfe06eb1f4 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -142,7 +142,8 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \ missing-comma-object.json non-objects.json \ qapi-schema-test.json quoted-structural-chars.json \ trailing-comma-list.json trailing-comma-object.json \ - unclosed-list.json unclosed-object.json unclosed-string.json) + unclosed-list.json unclosed-object.json unclosed-string.json \ + duplicate-key.json) GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h diff --git a/tests/qapi-schema/duplicate-key.err b/tests/qapi-schema/duplicate-key.err new file mode 100644 index 0000000000..0801c6a9bb --- /dev/null +++ b/tests/qapi-schema/duplicate-key.err @@ -0,0 +1 @@ +:2:10: Duplicate key "key" diff --git a/tests/qapi-schema/duplicate-key.exit b/tests/qapi-schema/duplicate-key.exit new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/tests/qapi-schema/duplicate-key.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/duplicate-key.json b/tests/qapi-schema/duplicate-key.json new file mode 100644 index 0000000000..1b55d88107 --- /dev/null +++ b/tests/qapi-schema/duplicate-key.json @@ -0,0 +1,2 @@ +{ 'key': 'value', + 'key': 'value' } diff --git a/tests/qapi-schema/duplicate-key.out b/tests/qapi-schema/duplicate-key.out new file mode 100644 index 0000000000..e69de29bb2 From 515b943a91db6c9faf9e35377c18db9ca32ecb40 Mon Sep 17 00:00:00 2001 From: Wenchao Xia Date: Tue, 4 Mar 2014 18:44:33 -0800 Subject: [PATCH 03/11] qapi script: remember line number in schema parsing Before this patch, 'QAPISchemaError' scans whole input until 'pos' to get error line number. After this patch, the scan is avoided since line number is remembered in schema parsing. This patch also benefits other error report functions, which would be introduced later. Signed-off-by: Wenchao Xia Reviewed-by: Eric Blake Reviewed-by: Markus Armbruster Signed-off-by: Luiz Capitulino --- scripts/qapi.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/scripts/qapi.py b/scripts/qapi.py index d0e793452a..19542920ee 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -39,12 +39,10 @@ class QAPISchemaError(Exception): def __init__(self, schema, msg): self.fp = schema.fp self.msg = msg - self.line = self.col = 1 - for ch in schema.src[0:schema.pos]: - if ch == '\n': - self.line += 1 - self.col = 1 - elif ch == '\t': + self.col = 1 + self.line = schema.line + for ch in schema.src[schema.line_pos:schema.pos]: + if ch == '\t': self.col = (self.col + 7) % 8 + 1 else: self.col += 1 @@ -60,6 +58,8 @@ def __init__(self, fp): if self.src == '' or self.src[-1] != '\n': self.src += '\n' self.cursor = 0 + self.line = 1 + self.line_pos = 0 self.exprs = [] self.accept() @@ -100,6 +100,8 @@ def accept(self): if self.cursor == len(self.src): self.tok = None return + self.line += 1 + self.line_pos = self.cursor elif not self.tok.isspace(): raise QAPISchemaError(self, 'Stray "%s"' % self.tok) From b86b05ed60d8d49c5770851860d4e6b89c133e7e Mon Sep 17 00:00:00 2001 From: Wenchao Xia Date: Tue, 4 Mar 2014 18:44:34 -0800 Subject: [PATCH 04/11] qapi script: check correctness of union Since line info is remembered as QAPISchema.line now, this patch uses it as additional info for every expr in QAPISchema inside qapi.py, then improves error message with it in checking of exprs. For common union the patch will check whether base is a valid complex type if specified. For flat union it will check whether base presents, whether discriminator is found in base, whether the key of every branch is correct when discriminator is an enum type. Signed-off-by: Wenchao Xia Reviewed-by: Markus Armbruster Signed-off-by: Luiz Capitulino --- scripts/qapi.py | 88 ++++++++++++++++++- tests/Makefile | 4 +- .../flat-union-invalid-branch-key.err | 1 + .../flat-union-invalid-branch-key.exit | 1 + .../flat-union-invalid-branch-key.json | 17 ++++ .../flat-union-invalid-branch-key.out | 0 .../flat-union-invalid-discriminator.err | 1 + .../flat-union-invalid-discriminator.exit | 1 + .../flat-union-invalid-discriminator.json | 17 ++++ .../flat-union-invalid-discriminator.out | 0 tests/qapi-schema/flat-union-no-base.err | 1 + tests/qapi-schema/flat-union-no-base.exit | 1 + tests/qapi-schema/flat-union-no-base.json | 10 +++ tests/qapi-schema/flat-union-no-base.out | 0 tests/qapi-schema/union-invalid-base.err | 1 + tests/qapi-schema/union-invalid-base.exit | 1 + tests/qapi-schema/union-invalid-base.json | 10 +++ tests/qapi-schema/union-invalid-base.out | 0 18 files changed, 151 insertions(+), 3 deletions(-) create mode 100644 tests/qapi-schema/flat-union-invalid-branch-key.err create mode 100644 tests/qapi-schema/flat-union-invalid-branch-key.exit create mode 100644 tests/qapi-schema/flat-union-invalid-branch-key.json create mode 100644 tests/qapi-schema/flat-union-invalid-branch-key.out create mode 100644 tests/qapi-schema/flat-union-invalid-discriminator.err create mode 100644 tests/qapi-schema/flat-union-invalid-discriminator.exit create mode 100644 tests/qapi-schema/flat-union-invalid-discriminator.json create mode 100644 tests/qapi-schema/flat-union-invalid-discriminator.out create mode 100644 tests/qapi-schema/flat-union-no-base.err create mode 100644 tests/qapi-schema/flat-union-no-base.exit create mode 100644 tests/qapi-schema/flat-union-no-base.json create mode 100644 tests/qapi-schema/flat-union-no-base.out create mode 100644 tests/qapi-schema/union-invalid-base.err create mode 100644 tests/qapi-schema/union-invalid-base.exit create mode 100644 tests/qapi-schema/union-invalid-base.json create mode 100644 tests/qapi-schema/union-invalid-base.out diff --git a/scripts/qapi.py b/scripts/qapi.py index 19542920ee..f1ca5b6d1c 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -50,6 +50,15 @@ def __init__(self, schema, msg): def __str__(self): return "%s:%s:%s: %s" % (self.fp.name, self.line, self.col, self.msg) +class QAPIExprError(Exception): + def __init__(self, expr_info, msg): + self.fp = expr_info['fp'] + self.line = expr_info['line'] + self.msg = msg + + def __str__(self): + return "%s:%s: %s" % (self.fp.name, self.line, self.msg) + class QAPISchema: def __init__(self, fp): @@ -64,7 +73,10 @@ def __init__(self, fp): self.accept() while self.tok != None: - self.exprs.append(self.get_expr(False)) + expr_info = {'fp': fp, 'line': self.line} + expr_elem = {'expr': self.get_expr(False), + 'info': expr_info} + self.exprs.append(expr_elem) def accept(self): while True: @@ -162,6 +174,71 @@ def get_expr(self, nested): raise QAPISchemaError(self, 'Expected "{", "[" or string') return expr +def find_base_fields(base): + base_struct_define = find_struct(base) + if not base_struct_define: + return None + return base_struct_define['data'] + +def check_union(expr, expr_info): + name = expr['union'] + base = expr.get('base') + discriminator = expr.get('discriminator') + members = expr['data'] + + # If the object has a member 'base', its value must name a complex type. + if base: + base_fields = find_base_fields(base) + if not base_fields: + raise QAPIExprError(expr_info, + "Base '%s' is not a valid type" + % base) + + # If the union object has no member 'discriminator', it's an + # ordinary union. + if not discriminator: + enum_define = None + + # Else if the value of member 'discriminator' is {}, it's an + # anonymous union. + elif discriminator == {}: + enum_define = None + + # Else, it's a flat union. + else: + # The object must have a member 'base'. + if not base: + raise QAPIExprError(expr_info, + "Flat union '%s' must have a base field" + % name) + # The value of member 'discriminator' must name a member of the + # base type. + discriminator_type = base_fields.get(discriminator) + if not discriminator_type: + raise QAPIExprError(expr_info, + "Discriminator '%s' is not a member of base " + "type '%s'" + % (discriminator, base)) + enum_define = find_enum(discriminator_type) + + # Check every branch + for (key, value) in members.items(): + # If this named member's value names an enum type, then all members + # of 'data' must also be members of the enum type. + if enum_define and not key in enum_define['enum_values']: + raise QAPIExprError(expr_info, + "Discriminator value '%s' is not found in " + "enum '%s'" % + (key, enum_define["enum_name"])) + # 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_exprs(schema): + for expr_elem in schema.exprs: + expr = expr_elem['expr'] + if expr.has_key('union'): + check_union(expr, expr_elem['info']) + def parse_schema(fp): try: schema = QAPISchema(fp) @@ -171,7 +248,8 @@ def parse_schema(fp): exprs = [] - for expr in schema.exprs: + for expr_elem in schema.exprs: + expr = expr_elem['expr'] if expr.has_key('enum'): add_enum(expr['enum'], expr['data']) elif expr.has_key('union'): @@ -181,6 +259,12 @@ def parse_schema(fp): add_struct(expr) exprs.append(expr) + try: + check_exprs(schema) + except QAPIExprError, e: + print >>sys.stderr, e + exit(1) + return exprs def parse_args(typeinfo): diff --git a/tests/Makefile b/tests/Makefile index dfe06eb1f4..6ac988932b 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -143,7 +143,9 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \ qapi-schema-test.json quoted-structural-chars.json \ trailing-comma-list.json trailing-comma-object.json \ unclosed-list.json unclosed-object.json unclosed-string.json \ - duplicate-key.json) + duplicate-key.json union-invalid-base.json flat-union-no-base.json \ + flat-union-invalid-discriminator.json \ + flat-union-invalid-branch-key.json) GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h diff --git a/tests/qapi-schema/flat-union-invalid-branch-key.err b/tests/qapi-schema/flat-union-invalid-branch-key.err new file mode 100644 index 0000000000..1125caf5db --- /dev/null +++ b/tests/qapi-schema/flat-union-invalid-branch-key.err @@ -0,0 +1 @@ +:13: Discriminator value 'value_wrong' is not found in enum 'TestEnum' diff --git a/tests/qapi-schema/flat-union-invalid-branch-key.exit b/tests/qapi-schema/flat-union-invalid-branch-key.exit new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/tests/qapi-schema/flat-union-invalid-branch-key.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/flat-union-invalid-branch-key.json b/tests/qapi-schema/flat-union-invalid-branch-key.json new file mode 100644 index 0000000000..a6242823ed --- /dev/null +++ b/tests/qapi-schema/flat-union-invalid-branch-key.json @@ -0,0 +1,17 @@ +{ 'enum': 'TestEnum', + 'data': [ 'value1', 'value2' ] } + +{ 'type': 'TestBase', + 'data': { 'enum1': 'TestEnum' } } + +{ 'type': 'TestTypeA', + 'data': { 'string': 'str' } } + +{ 'type': 'TestTypeB', + 'data': { 'integer': 'int' } } + +{ 'union': 'TestUnion', + 'base': 'TestBase', + 'discriminator': 'enum1', + 'data': { 'value_wrong': 'TestTypeA', + 'value2': 'TestTypeB' } } diff --git a/tests/qapi-schema/flat-union-invalid-branch-key.out b/tests/qapi-schema/flat-union-invalid-branch-key.out new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.err b/tests/qapi-schema/flat-union-invalid-discriminator.err new file mode 100644 index 0000000000..cad9dbf225 --- /dev/null +++ b/tests/qapi-schema/flat-union-invalid-discriminator.err @@ -0,0 +1 @@ +:13: Discriminator 'enum_wrong' is not a member of base type 'TestBase' diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.exit b/tests/qapi-schema/flat-union-invalid-discriminator.exit new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/tests/qapi-schema/flat-union-invalid-discriminator.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.json b/tests/qapi-schema/flat-union-invalid-discriminator.json new file mode 100644 index 0000000000..887157e173 --- /dev/null +++ b/tests/qapi-schema/flat-union-invalid-discriminator.json @@ -0,0 +1,17 @@ +{ 'enum': 'TestEnum', + 'data': [ 'value1', 'value2' ] } + +{ 'type': 'TestBase', + 'data': { 'enum1': 'TestEnum' } } + +{ 'type': 'TestTypeA', + 'data': { 'string': 'str' } } + +{ 'type': 'TestTypeB', + 'data': { 'integer': 'int' } } + +{ 'union': 'TestUnion', + 'base': 'TestBase', + 'discriminator': 'enum_wrong', + 'data': { 'value1': 'TestTypeA', + 'value2': 'TestTypeB' } } diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.out b/tests/qapi-schema/flat-union-invalid-discriminator.out new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/qapi-schema/flat-union-no-base.err b/tests/qapi-schema/flat-union-no-base.err new file mode 100644 index 0000000000..e2d7443a3b --- /dev/null +++ b/tests/qapi-schema/flat-union-no-base.err @@ -0,0 +1 @@ +:7: Flat union 'TestUnion' must have a base field diff --git a/tests/qapi-schema/flat-union-no-base.exit b/tests/qapi-schema/flat-union-no-base.exit new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/tests/qapi-schema/flat-union-no-base.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/flat-union-no-base.json b/tests/qapi-schema/flat-union-no-base.json new file mode 100644 index 0000000000..50f267323b --- /dev/null +++ b/tests/qapi-schema/flat-union-no-base.json @@ -0,0 +1,10 @@ +{ 'type': 'TestTypeA', + 'data': { 'string': 'str' } } + +{ 'type': 'TestTypeB', + 'data': { 'integer': 'int' } } + +{ 'union': 'TestUnion', + 'discriminator': 'enum1', + 'data': { 'value1': 'TestTypeA', + 'value2': 'TestTypeB' } } diff --git a/tests/qapi-schema/flat-union-no-base.out b/tests/qapi-schema/flat-union-no-base.out new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/qapi-schema/union-invalid-base.err b/tests/qapi-schema/union-invalid-base.err new file mode 100644 index 0000000000..dd8e3d1b3b --- /dev/null +++ b/tests/qapi-schema/union-invalid-base.err @@ -0,0 +1 @@ +:7: Base 'TestBaseWrong' is not a valid type diff --git a/tests/qapi-schema/union-invalid-base.exit b/tests/qapi-schema/union-invalid-base.exit new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/tests/qapi-schema/union-invalid-base.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/union-invalid-base.json b/tests/qapi-schema/union-invalid-base.json new file mode 100644 index 0000000000..1fa4930010 --- /dev/null +++ b/tests/qapi-schema/union-invalid-base.json @@ -0,0 +1,10 @@ +{ 'type': 'TestTypeA', + 'data': { 'string': 'str' } } + +{ 'type': 'TestTypeB', + 'data': { 'integer': 'int' } } + +{ 'union': 'TestUnion', + 'base': 'TestBaseWrong', + 'data': { 'value1': 'TestTypeA', + 'value2': 'TestTypeB' } } diff --git a/tests/qapi-schema/union-invalid-base.out b/tests/qapi-schema/union-invalid-base.out new file mode 100644 index 0000000000..e69de29bb2 From 6299659f54420955419c4995283f7dd770367939 Mon Sep 17 00:00:00 2001 From: Wenchao Xia Date: Tue, 4 Mar 2014 18:44:35 -0800 Subject: [PATCH 05/11] qapi script: code move for generate_enum_name() Later both qapi-types.py and qapi-visit.py need a common function for enum name generation. Signed-off-by: Wenchao Xia Reviewed-by: Eric Blake Reviewed-by: Markus Armbruster Signed-off-by: Luiz Capitulino --- scripts/qapi-types.py | 10 ---------- scripts/qapi.py | 10 ++++++++++ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index 2c6e0dcd5c..35ad9931b6 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -127,16 +127,6 @@ def generate_enum_lookup(name, values): ''') return ret -def generate_enum_name(name): - if name.isupper(): - return c_fun(name, False) - new_name = '' - for c in c_fun(name, False): - if c.isupper(): - new_name += '_' - new_name += c - return new_name.lstrip('_').upper() - def generate_enum(name, values): lookup_decl = mcgen(''' extern const char *%(name)s_lookup[]; diff --git a/scripts/qapi.py b/scripts/qapi.py index f1ca5b6d1c..0de9fe2cc5 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -467,3 +467,13 @@ def guardend(name): ''', name=guardname(name)) + +def generate_enum_name(name): + if name.isupper(): + return c_fun(name, False) + new_name = '' + for c in c_fun(name, False): + if c.isupper(): + new_name += '_' + new_name += c + return new_name.lstrip('_').upper() From b0b58195e4a3039b6a473124dc27ed707db50240 Mon Sep 17 00:00:00 2001 From: Wenchao Xia Date: Tue, 4 Mar 2014 18:44:36 -0800 Subject: [PATCH 06/11] qapi script: use same function to generate enum string Prior to this patch, qapi-visit.py used custom code to generate enum names used for handling a qapi union. Fix it to instead reuse common code, with identical generated results, and allowing future updates to generation to only need to touch one place. Signed-off-by: Wenchao Xia Reviewed-by: Eric Blake Reviewed-by: Markus Armbruster Signed-off-by: Luiz Capitulino --- scripts/qapi-types.py | 6 +++--- scripts/qapi-visit.py | 19 +++++++++++++------ scripts/qapi.py | 13 +++++++++---- 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index 35ad9931b6..5885bac5ba 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -144,11 +144,11 @@ def generate_enum(name, values): i = 0 for value in enum_values: + enum_full_value = generate_enum_full_value(name, value) enum_decl += mcgen(''' - %(abbrev)s_%(value)s = %(i)d, + %(enum_full_value)s = %(i)d, ''', - abbrev=de_camel_case(name).upper(), - value=generate_enum_name(value), + enum_full_value = enum_full_value, i=i) i += 1 diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index c6de9aeed4..0baaf606dc 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -214,18 +214,22 @@ def generate_visit_anon_union(name, members): ''', name=name) + # For anon union, always use the default enum type automatically generated + # as "'%sKind' % (name)" + disc_type = '%sKind' % (name) + for key in members: assert (members[key] in builtin_types or find_struct(members[key]) or find_union(members[key])), "Invalid anonymous union member" + enum_full_value = generate_enum_full_value(disc_type, key) ret += mcgen(''' - case %(abbrev)s_KIND_%(enum)s: + case %(enum_full_value)s: visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, name, &err); break; ''', - abbrev = de_camel_case(name).upper(), - enum = c_fun(de_camel_case(key),False).upper(), + enum_full_value = enum_full_value, c_type = type_name(members[key]), c_name = c_fun(key)) @@ -255,7 +259,10 @@ def generate_visit_union(expr): assert not base return generate_visit_anon_union(name, members) + # There will always be a discriminator in the C switch code, by default it + # is an enum type generated silently as "'%sKind' % (name)" ret = generate_visit_enum('%sKind' % name, members.keys()) + disc_type = '%sKind' % (name) if base: base_fields = find_struct(base)['data'] @@ -313,13 +320,13 @@ def generate_visit_union(expr): visit_end_implicit_struct(m, &err); }''' + enum_full_value = generate_enum_full_value(disc_type, key) ret += mcgen(''' - case %(abbrev)s_KIND_%(enum)s: + case %(enum_full_value)s: ''' + fmt + ''' break; ''', - abbrev = de_camel_case(name).upper(), - enum = c_fun(de_camel_case(key),False).upper(), + enum_full_value = enum_full_value, c_type=type_name(members[key]), c_name=c_fun(key)) diff --git a/scripts/qapi.py b/scripts/qapi.py index 0de9fe2cc5..eebc8a7061 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -468,12 +468,17 @@ def guardend(name): ''', name=guardname(name)) -def generate_enum_name(name): - if name.isupper(): - return c_fun(name, False) +def _generate_enum_value_string(value): + if value.isupper(): + return c_fun(value, False) new_name = '' - for c in c_fun(name, False): + for c in c_fun(value, False): if c.isupper(): new_name += '_' new_name += c return new_name.lstrip('_').upper() + +def generate_enum_full_value(enum_name, enum_value): + abbrev_string = de_camel_case(enum_name).upper() + value_string = _generate_enum_value_string(enum_value) + return "%s_%s" % (abbrev_string, value_string) From bceae7697ff1711675c26f715b945737bc6849ae Mon Sep 17 00:00:00 2001 From: Wenchao Xia Date: Thu, 6 Mar 2014 17:08:56 -0800 Subject: [PATCH 07/11] qapi script: support enum type as discriminator in union By default, any union will automatically generate a enum type as "[UnionName]Kind" in C code, and it is duplicated when the discriminator is specified as a pre-defined enum type in schema. After this patch, the pre-defined enum type will be really used as the switch case condition in generated C code, if discriminator is an enum field. Signed-off-by: Wenchao Xia Reviewed-by: Markus Armbruster Signed-off-by: Luiz Capitulino --- docs/qapi-code-gen.txt | 8 +++-- scripts/qapi-types.py | 18 +++++++++--- scripts/qapi-visit.py | 29 +++++++++++++------ scripts/qapi.py | 27 ++++++++++++++++- tests/Makefile | 2 +- .../qapi-schema/flat-union-reverse-define.err | 0 .../flat-union-reverse-define.exit | 1 + .../flat-union-reverse-define.json | 17 +++++++++++ .../qapi-schema/flat-union-reverse-define.out | 9 ++++++ 9 files changed, 94 insertions(+), 17 deletions(-) create mode 100644 tests/qapi-schema/flat-union-reverse-define.err create mode 100644 tests/qapi-schema/flat-union-reverse-define.exit create mode 100644 tests/qapi-schema/flat-union-reverse-define.json create mode 100644 tests/qapi-schema/flat-union-reverse-define.out diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index 0728f36c65..a2e7921601 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -123,11 +123,15 @@ And it looks like this on the wire: Flat union types avoid the nesting on the wire. They are used whenever a specific field of the base type is declared as the discriminator ('type' is -then no longer generated). The discriminator must always be a string field. +then no longer generated). The discriminator can be a string field or a +predefined enum field. If it is a string field, a hidden enum type will be +generated as "[UNION_NAME]Kind". If it is an enum field, a compile time check +will be done to verify the correctness. It is recommended to use an enum field. The above example can then be modified as follows: + { 'enum': 'BlockdevDriver', 'data': [ 'raw', 'qcow2' ] } { 'type': 'BlockdevCommonOptions', - 'data': { 'driver': 'str', 'readonly': 'bool' } } + 'data': { 'driver': 'BlockdevDriver', 'readonly': 'bool' } } { 'union': 'BlockdevOptions', 'base': 'BlockdevCommonOptions', 'discriminator': 'driver', diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index 5885bac5ba..10864efc58 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -201,14 +201,21 @@ def generate_union(expr): base = expr.get('base') discriminator = expr.get('discriminator') + enum_define = discriminator_find_enum_define(expr) + if enum_define: + discriminator_type_name = enum_define['enum_name'] + else: + discriminator_type_name = '%sKind' % (name) + ret = mcgen(''' struct %(name)s { - %(name)sKind kind; + %(discriminator_type_name)s kind; union { void *data; ''', - name=name) + name=name, + discriminator_type_name=discriminator_type_name) for key in typeinfo: ret += mcgen(''' @@ -389,8 +396,11 @@ def maybe_open(really, name, opt): fdef.write(generate_enum_lookup(expr['enum'], expr['data'])) elif expr.has_key('union'): ret += generate_fwd_struct(expr['union'], expr['data']) + "\n" - ret += generate_enum('%sKind' % expr['union'], expr['data'].keys()) - fdef.write(generate_enum_lookup('%sKind' % expr['union'], expr['data'].keys())) + enum_define = discriminator_find_enum_define(expr) + if not enum_define: + ret += generate_enum('%sKind' % expr['union'], expr['data'].keys()) + fdef.write(generate_enum_lookup('%sKind' % expr['union'], + expr['data'].keys())) if expr.get('discriminator') == {}: fdef.write(generate_anon_union_qtypes(expr)) else: diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 0baaf606dc..45ce3a957a 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -259,10 +259,16 @@ def generate_visit_union(expr): assert not base return generate_visit_anon_union(name, members) - # There will always be a discriminator in the C switch code, by default it - # is an enum type generated silently as "'%sKind' % (name)" - ret = generate_visit_enum('%sKind' % name, members.keys()) - disc_type = '%sKind' % (name) + enum_define = discriminator_find_enum_define(expr) + if enum_define: + # Use the enum type as discriminator + ret = "" + disc_type = enum_define['enum_name'] + else: + # There will always be a discriminator in the C switch code, by default it + # is an enum type generated silently as "'%sKind' % (name)" + ret = generate_visit_enum('%sKind' % name, members.keys()) + disc_type = '%sKind' % (name) if base: base_fields = find_struct(base)['data'] @@ -298,15 +304,16 @@ def generate_visit_union(expr): pop_indent() if not discriminator: - desc_type = "type" + disc_key = "type" else: - desc_type = discriminator + disc_key = discriminator ret += mcgen(''' - visit_type_%(name)sKind(m, &(*obj)->kind, "%(type)s", &err); + visit_type_%(disc_type)s(m, &(*obj)->kind, "%(disc_key)s", &err); if (!err) { switch ((*obj)->kind) { ''', - name=name, type=desc_type) + disc_type = disc_type, + disc_key = disc_key) for key in members: if not discriminator: @@ -517,7 +524,11 @@ def maybe_open(really, name, opt): ret += generate_visit_list(expr['union'], expr['data']) fdef.write(ret) - ret = generate_decl_enum('%sKind' % expr['union'], expr['data'].keys()) + enum_define = discriminator_find_enum_define(expr) + ret = "" + if not enum_define: + ret = generate_decl_enum('%sKind' % expr['union'], + expr['data'].keys()) ret += generate_declaration(expr['union'], expr['data']) fdecl.write(ret) elif expr.has_key('enum'): diff --git a/scripts/qapi.py b/scripts/qapi.py index eebc8a7061..2b43ad2f1b 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -180,6 +180,25 @@ def find_base_fields(base): return None return base_struct_define['data'] +# Return the discriminator enum define if discriminator is specified as an +# enum type, otherwise return None. +def discriminator_find_enum_define(expr): + base = expr.get('base') + discriminator = expr.get('discriminator') + + if not (discriminator and base): + return None + + base_fields = find_base_fields(base) + if not base_fields: + return None + + discriminator_type = base_fields.get(discriminator) + if not discriminator_type: + return None + + return find_enum(discriminator_type) + def check_union(expr, expr_info): name = expr['union'] base = expr.get('base') @@ -254,11 +273,17 @@ def parse_schema(fp): add_enum(expr['enum'], expr['data']) elif expr.has_key('union'): add_union(expr) - add_enum('%sKind' % expr['union']) elif expr.has_key('type'): add_struct(expr) exprs.append(expr) + # Try again for hidden UnionKind enum + for expr_elem in schema.exprs: + expr = expr_elem['expr'] + if expr.has_key('union'): + if not discriminator_find_enum_define(expr): + add_enum('%sKind' % expr['union']) + try: check_exprs(schema) except QAPIExprError, e: diff --git a/tests/Makefile b/tests/Makefile index 6ac988932b..88fca31276 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -145,7 +145,7 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \ unclosed-list.json unclosed-object.json unclosed-string.json \ duplicate-key.json union-invalid-base.json flat-union-no-base.json \ flat-union-invalid-discriminator.json \ - flat-union-invalid-branch-key.json) + flat-union-invalid-branch-key.json flat-union-reverse-define.json) GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h diff --git a/tests/qapi-schema/flat-union-reverse-define.err b/tests/qapi-schema/flat-union-reverse-define.err new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/qapi-schema/flat-union-reverse-define.exit b/tests/qapi-schema/flat-union-reverse-define.exit new file mode 100644 index 0000000000..573541ac97 --- /dev/null +++ b/tests/qapi-schema/flat-union-reverse-define.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/flat-union-reverse-define.json b/tests/qapi-schema/flat-union-reverse-define.json new file mode 100644 index 0000000000..9ea7e72201 --- /dev/null +++ b/tests/qapi-schema/flat-union-reverse-define.json @@ -0,0 +1,17 @@ +{ 'union': 'TestUnion', + 'base': 'TestBase', + 'discriminator': 'enum1', + 'data': { 'value1': 'TestTypeA', + 'value2': 'TestTypeB' } } + +{ 'type': 'TestBase', + 'data': { 'enum1': 'TestEnum' } } + +{ 'enum': 'TestEnum', + 'data': [ 'value1', 'value2' ] } + +{ 'type': 'TestTypeA', + 'data': { 'string': 'str' } } + +{ 'type': 'TestTypeB', + 'data': { 'integer': 'int' } } diff --git a/tests/qapi-schema/flat-union-reverse-define.out b/tests/qapi-schema/flat-union-reverse-define.out new file mode 100644 index 0000000000..03c952e28a --- /dev/null +++ b/tests/qapi-schema/flat-union-reverse-define.out @@ -0,0 +1,9 @@ +[OrderedDict([('union', 'TestUnion'), ('base', 'TestBase'), ('discriminator', 'enum1'), ('data', OrderedDict([('value1', 'TestTypeA'), ('value2', 'TestTypeB')]))]), + OrderedDict([('type', 'TestBase'), ('data', OrderedDict([('enum1', 'TestEnum')]))]), + OrderedDict([('enum', 'TestEnum'), ('data', ['value1', 'value2'])]), + OrderedDict([('type', 'TestTypeA'), ('data', OrderedDict([('string', 'str')]))]), + OrderedDict([('type', 'TestTypeB'), ('data', OrderedDict([('integer', 'int')]))])] +[{'enum_name': 'TestEnum', 'enum_values': ['value1', 'value2']}] +[OrderedDict([('type', 'TestBase'), ('data', OrderedDict([('enum1', 'TestEnum')]))]), + OrderedDict([('type', 'TestTypeA'), ('data', OrderedDict([('string', 'str')]))]), + OrderedDict([('type', 'TestTypeB'), ('data', OrderedDict([('integer', 'int')]))])] From 59ca664ef86c8a9078157336b566e832fc5466e2 Mon Sep 17 00:00:00 2001 From: Wenchao Xia Date: Tue, 4 Mar 2014 18:44:38 -0800 Subject: [PATCH 08/11] qapi: convert BlockdevOptions to use enum discriminator After this patch, hidden enum type BlockdevOptionsKind will not be generated, and other API can use enum BlockdevDriver. Signed-off-by: Wenchao Xia Reviewed-by: Eric Blake Reviewed-by: Markus Armbruster Signed-off-by: Luiz Capitulino --- qapi-schema.json | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/qapi-schema.json b/qapi-schema.json index 6c381b7306..f4f9439fe6 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -4248,6 +4248,18 @@ '*direct': 'bool', '*no-flush': 'bool' } } +## +# @BlockdevDriver +# +# Drivers that are supported in block device operations. +# +# Since: 2.0 +## +{ 'enum': 'BlockdevDriver', + 'data': [ 'file', 'http', 'https', 'ftp', 'ftps', 'tftp', 'vvfat', 'blkdebug', + 'blkverify', 'bochs', 'cloop', 'cow', 'dmg', 'parallels', 'qcow', + 'qcow2', 'qed', 'raw', 'vdi', 'vhdx', 'vmdk', 'vpc', 'quorum' ] } + ## # @BlockdevOptionsBase # @@ -4272,7 +4284,7 @@ # Since: 1.7 ## { 'type': 'BlockdevOptionsBase', - 'data': { 'driver': 'str', + 'data': { 'driver': 'BlockdevDriver', '*id': 'str', '*node-name': 'str', '*discard': 'BlockdevDiscardOptions', From 5223070c47c6fc35ee000b2392ae76d9fab54f16 Mon Sep 17 00:00:00 2001 From: Wenchao Xia Date: Tue, 4 Mar 2014 18:44:39 -0800 Subject: [PATCH 09/11] qapi script: do not allow string discriminator Since enum based discriminators provide better type-safety and ensure that future qapi additions do not forget to adjust dependent unions, forbid using string as discriminator from now on. Signed-off-by: Wenchao Xia Reviewed-by: Eric Blake Reviewed-by: Markus Armbruster Signed-off-by: Luiz Capitulino --- docs/qapi-code-gen.txt | 5 +---- scripts/qapi.py | 5 +++++ tests/Makefile | 3 ++- .../flat-union-string-discriminator.err | 1 + .../flat-union-string-discriminator.exit | 1 + .../flat-union-string-discriminator.json | 17 +++++++++++++++++ .../flat-union-string-discriminator.out | 0 tests/qapi-schema/qapi-schema-test.json | 9 ++++++--- tests/qapi-schema/qapi-schema-test.out | 5 +++-- tests/test-qmp-input-strict.c | 5 ++++- tests/test-qmp-input-visitor.c | 10 +++++++--- tests/test-qmp-output-visitor.c | 10 ++++++---- 12 files changed, 53 insertions(+), 18 deletions(-) create mode 100644 tests/qapi-schema/flat-union-string-discriminator.err create mode 100644 tests/qapi-schema/flat-union-string-discriminator.exit create mode 100644 tests/qapi-schema/flat-union-string-discriminator.json create mode 100644 tests/qapi-schema/flat-union-string-discriminator.out diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index a2e7921601..d78921f875 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -123,10 +123,7 @@ And it looks like this on the wire: Flat union types avoid the nesting on the wire. They are used whenever a specific field of the base type is declared as the discriminator ('type' is -then no longer generated). The discriminator can be a string field or a -predefined enum field. If it is a string field, a hidden enum type will be -generated as "[UNION_NAME]Kind". If it is an enum field, a compile time check -will be done to verify the correctness. It is recommended to use an enum field. +then no longer generated). The discriminator must be of enumeration type. The above example can then be modified as follows: { 'enum': 'BlockdevDriver', 'data': [ 'raw', 'qcow2' ] } diff --git a/scripts/qapi.py b/scripts/qapi.py index 2b43ad2f1b..bd00d01c0f 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -239,6 +239,11 @@ def check_union(expr, expr_info): "type '%s'" % (discriminator, base)) enum_define = find_enum(discriminator_type) + # Do not allow string discriminator + if not enum_define: + raise QAPIExprError(expr_info, + "Discriminator '%s' must be of enumeration " + "type" % discriminator) # Check every branch for (key, value) in members.items(): diff --git a/tests/Makefile b/tests/Makefile index 88fca31276..e146f81d44 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -145,7 +145,8 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \ unclosed-list.json unclosed-object.json unclosed-string.json \ duplicate-key.json union-invalid-base.json flat-union-no-base.json \ flat-union-invalid-discriminator.json \ - flat-union-invalid-branch-key.json flat-union-reverse-define.json) + flat-union-invalid-branch-key.json flat-union-reverse-define.json \ + flat-union-string-discriminator.json) GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h diff --git a/tests/qapi-schema/flat-union-string-discriminator.err b/tests/qapi-schema/flat-union-string-discriminator.err new file mode 100644 index 0000000000..87482704ec --- /dev/null +++ b/tests/qapi-schema/flat-union-string-discriminator.err @@ -0,0 +1 @@ +:13: Discriminator 'kind' must be of enumeration type diff --git a/tests/qapi-schema/flat-union-string-discriminator.exit b/tests/qapi-schema/flat-union-string-discriminator.exit new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/tests/qapi-schema/flat-union-string-discriminator.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/flat-union-string-discriminator.json b/tests/qapi-schema/flat-union-string-discriminator.json new file mode 100644 index 0000000000..e966aeb395 --- /dev/null +++ b/tests/qapi-schema/flat-union-string-discriminator.json @@ -0,0 +1,17 @@ +{ 'enum': 'TestEnum', + 'data': [ 'value1', 'value2' ] } + +{ 'type': 'TestBase', + 'data': { 'enum1': 'TestEnum', 'kind': 'str' } } + +{ 'type': 'TestTypeA', + 'data': { 'string': 'str' } } + +{ 'type': 'TestTypeB', + 'data': { 'integer': 'int' } } + +{ 'union': 'TestUnion', + 'base': 'TestBase', + 'discriminator': 'kind', + 'data': { 'kind1': 'TestTypeA', + 'kind2': 'TestTypeB' } } diff --git a/tests/qapi-schema/flat-union-string-discriminator.out b/tests/qapi-schema/flat-union-string-discriminator.out new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index 471ba47dde..818c06dc75 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -37,10 +37,13 @@ 'base': 'UserDefZero', 'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } } +{ 'type': 'UserDefUnionBase', + 'data': { 'string': 'str', 'enum1': 'EnumOne' } } + { 'union': 'UserDefFlatUnion', - 'base': 'UserDefOne', - 'discriminator': 'string', - 'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } } + 'base': 'UserDefUnionBase', + 'discriminator': 'enum1', + 'data': { 'value1' : 'UserDefA', 'value2' : 'UserDefB', 'value3' : 'UserDefB' } } # FIXME generated struct UserDefFlatUnion has members for direct base # UserDefOne, but lacks members for indirect base UserDefZero diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index 01685d4a46..6cd03f31c3 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -7,7 +7,8 @@ OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]), OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]), OrderedDict([('union', 'UserDefUnion'), ('base', 'UserDefZero'), ('data', OrderedDict([('a', 'UserDefA'), ('b', 'UserDefB')]))]), - OrderedDict([('union', 'UserDefFlatUnion'), ('base', 'UserDefOne'), ('discriminator', 'string'), ('data', OrderedDict([('a', 'UserDefA'), ('b', 'UserDefB')]))]), + OrderedDict([('type', 'UserDefUnionBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]), + OrderedDict([('union', 'UserDefFlatUnion'), ('base', 'UserDefUnionBase'), ('discriminator', 'enum1'), ('data', OrderedDict([('value1', 'UserDefA'), ('value2', 'UserDefB'), ('value3', 'UserDefB')]))]), OrderedDict([('union', 'UserDefAnonUnion'), ('discriminator', OrderedDict()), ('data', OrderedDict([('uda', 'UserDefA'), ('s', 'str'), ('i', 'int')]))]), OrderedDict([('union', 'UserDefNativeListUnion'), ('data', OrderedDict([('integer', ['int']), ('s8', ['int8']), ('s16', ['int16']), ('s32', ['int32']), ('s64', ['int64']), ('u8', ['uint8']), ('u16', ['uint16']), ('u32', ['uint32']), ('u64', ['uint64']), ('number', ['number']), ('boolean', ['bool']), ('string', ['str'])]))]), OrderedDict([('command', 'user_def_cmd'), ('data', OrderedDict())]), @@ -17,7 +18,6 @@ OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))])] [{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']}, {'enum_name': 'UserDefUnionKind', 'enum_values': None}, - {'enum_name': 'UserDefFlatUnionKind', 'enum_values': None}, {'enum_name': 'UserDefAnonUnionKind', 'enum_values': None}, {'enum_name': 'UserDefNativeListUnionKind', 'enum_values': None}] [OrderedDict([('type', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 'EnumOne')]))]), @@ -27,4 +27,5 @@ OrderedDict([('type', 'UserDefNested'), ('data', OrderedDict([('string0', 'str'), ('dict1', OrderedDict([('string1', 'str'), ('dict2', OrderedDict([('userdef1', 'UserDefOne'), ('string2', 'str')])), ('*dict3', OrderedDict([('userdef2', 'UserDefOne'), ('string3', 'str')]))]))]))]), OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]), OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]), + OrderedDict([('type', 'UserDefUnionBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]), OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))])] diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c index 64d72f6f05..38b5e95f68 100644 --- a/tests/test-qmp-input-strict.c +++ b/tests/test-qmp-input-strict.c @@ -146,7 +146,10 @@ static void test_validate_union_flat(TestInputVisitorData *data, Visitor *v; Error *errp = NULL; - v = validate_test_init(data, "{ 'string': 'a', 'boolean': true }"); + v = validate_test_init(data, + "{ 'enum1': 'value1', " + "'string': 'str', " + "'boolean': true }"); /* TODO when generator bug is fixed, add 'integer': 41 */ visit_type_UserDefFlatUnion(v, &tmp, NULL, &errp); diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c index 2dffafc1f4..1729667a6f 100644 --- a/tests/test-qmp-input-visitor.c +++ b/tests/test-qmp-input-visitor.c @@ -310,14 +310,18 @@ static void test_visitor_in_union_flat(TestInputVisitorData *data, Error *err = NULL; UserDefFlatUnion *tmp; - v = visitor_input_test_init(data, "{ 'string': 'a', 'boolean': true }"); + v = visitor_input_test_init(data, + "{ 'enum1': 'value1', " + "'string': 'str', " + "'boolean': true }"); /* TODO when generator bug is fixed, add 'integer': 41 */ visit_type_UserDefFlatUnion(v, &tmp, NULL, &err); g_assert(err == NULL); - g_assert_cmpint(tmp->kind, ==, USER_DEF_UNION_KIND_A); + g_assert_cmpint(tmp->kind, ==, ENUM_ONE_VALUE1); + g_assert_cmpstr(tmp->string, ==, "str"); /* TODO g_assert_cmpint(tmp->integer, ==, 41); */ - g_assert_cmpint(tmp->a->boolean, ==, true); + g_assert_cmpint(tmp->value1->boolean, ==, true); qapi_free_UserDefFlatUnion(tmp); } diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c index 105f4cf94b..da279713f2 100644 --- a/tests/test-qmp-output-visitor.c +++ b/tests/test-qmp-output-visitor.c @@ -449,10 +449,11 @@ static void test_visitor_out_union_flat(TestOutputVisitorData *data, Error *err = NULL; UserDefFlatUnion *tmp = g_malloc0(sizeof(UserDefFlatUnion)); - tmp->kind = USER_DEF_UNION_KIND_A; - tmp->a = g_malloc0(sizeof(UserDefA)); + tmp->kind = ENUM_ONE_VALUE1; + tmp->string = g_strdup("str"); + tmp->value1 = g_malloc0(sizeof(UserDefA)); /* TODO when generator bug is fixed: tmp->integer = 41; */ - tmp->a->boolean = true; + tmp->value1->boolean = true; visit_type_UserDefFlatUnion(data->ov, &tmp, NULL, &err); g_assert(err == NULL); @@ -461,7 +462,8 @@ static void test_visitor_out_union_flat(TestOutputVisitorData *data, g_assert(qobject_type(arg) == QTYPE_QDICT); qdict = qobject_to_qdict(arg); - g_assert_cmpstr(qdict_get_str(qdict, "string"), ==, "a"); + g_assert_cmpstr(qdict_get_str(qdict, "enum1"), ==, "value1"); + g_assert_cmpstr(qdict_get_str(qdict, "string"), ==, "str"); /* TODO g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 41); */ g_assert_cmpint(qdict_get_bool(qdict, "boolean"), ==, true); From 5d371f41b4db8e47c89626ecf9d9914119583e23 Mon Sep 17 00:00:00 2001 From: Wenchao Xia Date: Tue, 4 Mar 2014 18:44:40 -0800 Subject: [PATCH 10/11] qapi script: do not add "_" for every capitalized char in enum Now "enum AIOContext" will generate AIO_CONTEXT instead of A_I_O_CONTEXT, "X86CPU" will generate X86_CPU instead of X86_C_P_U. Signed-off-by: Wenchao Xia Reviewed-by: Eric Blake Reviewed-by: Markus Armbruster Signed-off-by: Luiz Capitulino --- include/qapi/qmp/qerror.h | 2 +- scripts/qapi.py | 26 +++++++++++++++++++------- target-i386/cpu.c | 2 +- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index 25193c943b..da75abf6d6 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -159,7 +159,7 @@ void qerror_report_err(Error *err); ERROR_CLASS_GENERIC_ERROR, "Invalid JSON syntax" #define QERR_KVM_MISSING_CAP \ - ERROR_CLASS_K_V_M_MISSING_CAP, "Using KVM without %s, %s unavailable" + ERROR_CLASS_KVM_MISSING_CAP, "Using KVM without %s, %s unavailable" #define QERR_MIGRATION_ACTIVE \ ERROR_CLASS_GENERIC_ERROR, "There's a migration process in progress" diff --git a/scripts/qapi.py b/scripts/qapi.py index bd00d01c0f..b474c39558 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -498,17 +498,29 @@ def guardend(name): ''', name=guardname(name)) -def _generate_enum_value_string(value): +# ENUMName -> ENUM_NAME, EnumName1 -> ENUM_NAME1 +# ENUM_NAME -> ENUM_NAME, ENUM_NAME1 -> ENUM_NAME1, ENUM_Name2 -> ENUM_NAME2 +# ENUM24_Name -> ENUM24_NAME +def _generate_enum_string(value): + c_fun_str = c_fun(value, False) if value.isupper(): - return c_fun(value, False) + return c_fun_str + new_name = '' - for c in c_fun(value, False): - if c.isupper(): - new_name += '_' + l = len(c_fun_str) + for i in range(l): + 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(): + new_name += '_' new_name += c return new_name.lstrip('_').upper() def generate_enum_full_value(enum_name, enum_value): - abbrev_string = de_camel_case(enum_name).upper() - value_string = _generate_enum_value_string(enum_value) + abbrev_string = _generate_enum_string(enum_name) + value_string = _generate_enum_string(enum_value) return "%s_%s" % (abbrev_string, value_string) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 0e8812a11d..c83ab0f8ce 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -315,7 +315,7 @@ typedef struct X86RegisterInfo32 { } X86RegisterInfo32; #define REGISTER(reg) \ - [R_##reg] = { .name = #reg, .qapi_enum = X86_C_P_U_REGISTER32_##reg } + [R_##reg] = { .name = #reg, .qapi_enum = X86_CPU_REGISTER32_##reg } X86RegisterInfo32 x86_reg_info_32[CPU_NB_REGS32] = { REGISTER(EAX), REGISTER(ECX), From 2a7a1a56d1e30de07cf7d7636a35bf7706b9500e Mon Sep 17 00:00:00 2001 From: Luiz Capitulino Date: Sat, 8 Mar 2014 12:20:06 -0500 Subject: [PATCH 11/11] tests: test-qmp-commands: Fix double free The ret variable is freed twice, but on the second time we actually want to free ret3 instead. Don't know why this didn't explode. Reported-by: Peter Maydell Tested-by: Peter Maydell Reviewed-by: Eric Blake Signed-off-by: Luiz Capitulino --- tests/test-qmp-commands.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c index 8e62c2d8ad..554e222b32 100644 --- a/tests/test-qmp-commands.c +++ b/tests/test-qmp-commands.c @@ -141,7 +141,7 @@ static void test_dispatch_cmd_io(void) ret3 = qobject_to_qint(test_qmp_dispatch(req)); assert(qint_get_int(ret3) == 66); - QDECREF(ret); + QDECREF(ret3); QDECREF(req); }