From dad1fcab91bf101a02151069036d416367b59c5c Mon Sep 17 00:00:00 2001 From: Wenchao Xia Date: Tue, 4 Mar 2014 18:44:31 -0800 Subject: 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 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) (limited to 'scripts/qapi.py') diff --git a/scripts/qapi.py b/scripts/qapi.py index f3c2a20..023930e 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 is_enum(name): +def find_enum(name): global enum_types - return (name in enum_types) + for enum in enum_types: + if enum['enum_name'] == name: + return enum + return None + +def is_enum(name): + return find_enum(name) != None def c_type(name): if name == 'str': -- cgit v1.1 From 4b35991a3bd5f9e03333d5b1bd4a7bcf9941aac5 Mon Sep 17 00:00:00 2001 From: Wenchao Xia Date: Tue, 4 Mar 2014 18:44:32 -0800 Subject: 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 ++ 1 file changed, 2 insertions(+) (limited to 'scripts/qapi.py') diff --git a/scripts/qapi.py b/scripts/qapi.py index 023930e..d0e7934 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -116,6 +116,8 @@ class QAPISchema: 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() -- cgit v1.1 From 515b943a91db6c9faf9e35377c18db9ca32ecb40 Mon Sep 17 00:00:00 2001 From: Wenchao Xia Date: Tue, 4 Mar 2014 18:44:33 -0800 Subject: 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(-) (limited to 'scripts/qapi.py') diff --git a/scripts/qapi.py b/scripts/qapi.py index d0e7934..1954292 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 @@ class QAPISchema: 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 @@ class QAPISchema: 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) -- cgit v1.1 From b86b05ed60d8d49c5770851860d4e6b89c133e7e Mon Sep 17 00:00:00 2001 From: Wenchao Xia Date: Tue, 4 Mar 2014 18:44:34 -0800 Subject: 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 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 86 insertions(+), 2 deletions(-) (limited to 'scripts/qapi.py') diff --git a/scripts/qapi.py b/scripts/qapi.py index 1954292..f1ca5b6 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -50,6 +50,15 @@ class QAPISchemaError(Exception): 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 @@ class QAPISchema: 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 @@ class QAPISchema: 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): -- cgit v1.1 From 6299659f54420955419c4995283f7dd770367939 Mon Sep 17 00:00:00 2001 From: Wenchao Xia Date: Tue, 4 Mar 2014 18:44:35 -0800 Subject: 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.py | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'scripts/qapi.py') diff --git a/scripts/qapi.py b/scripts/qapi.py index f1ca5b6..0de9fe2 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() -- cgit v1.1 From b0b58195e4a3039b6a473124dc27ed707db50240 Mon Sep 17 00:00:00 2001 From: Wenchao Xia Date: Tue, 4 Mar 2014 18:44:36 -0800 Subject: 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.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'scripts/qapi.py') diff --git a/scripts/qapi.py b/scripts/qapi.py index 0de9fe2..eebc8a7 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) -- cgit v1.1 From bceae7697ff1711675c26f715b945737bc6849ae Mon Sep 17 00:00:00 2001 From: Wenchao Xia Date: Thu, 6 Mar 2014 17:08:56 -0800 Subject: 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 --- scripts/qapi.py | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) (limited to 'scripts/qapi.py') diff --git a/scripts/qapi.py b/scripts/qapi.py index eebc8a7..2b43ad2 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: -- cgit v1.1 From 5223070c47c6fc35ee000b2392ae76d9fab54f16 Mon Sep 17 00:00:00 2001 From: Wenchao Xia Date: Tue, 4 Mar 2014 18:44:39 -0800 Subject: 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 --- scripts/qapi.py | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'scripts/qapi.py') diff --git a/scripts/qapi.py b/scripts/qapi.py index 2b43ad2..bd00d01 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(): -- cgit v1.1 From 5d371f41b4db8e47c89626ecf9d9914119583e23 Mon Sep 17 00:00:00 2001 From: Wenchao Xia Date: Tue, 4 Mar 2014 18:44:40 -0800 Subject: 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 --- scripts/qapi.py | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) (limited to 'scripts/qapi.py') diff --git a/scripts/qapi.py b/scripts/qapi.py index bd00d01..b474c39 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) -- cgit v1.1