From 7408fb67c0f9403f6e40aecf97cf798fc14e2cd8 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 29 Sep 2015 16:21:00 -0600 Subject: qapi: Improve 'include' error message Use of '"...%s" % include' to print non-strings can lead to ugly messages, such as this (if the .json change is applied without the qapi.py change): Expected a file name (string), got: OrderedDict() Better is to just omit the actual non-string value in the message. Signed-off-by: Eric Blake Message-Id: <1443565276-4535-3-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- scripts/qapi.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'scripts') diff --git a/scripts/qapi.py b/scripts/qapi.py index 06478bb..362e007 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -132,8 +132,7 @@ class QAPISchemaParser(object): include = expr["include"] if not isinstance(include, str): raise QAPIExprError(expr_info, - 'Expected a file name (string), got: %s' - % include) + "Value of 'include' must be a string") incl_abs_fname = os.path.join(os.path.dirname(abs_fname), include) # catch inclusion cycle -- cgit v1.1 From 59b00542659c8947f9d4e8c28d2d528ab3ab61a5 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 29 Sep 2015 16:21:01 -0600 Subject: qapi: Invoke exception superclass initializer pylint recommends that every exception class should explicitly invoke the superclass __init__, even though things seem to work fine without it. Signed-off-by: Eric Blake Message-Id: <1443565276-4535-4-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- scripts/qapi.py | 2 ++ 1 file changed, 2 insertions(+) (limited to 'scripts') diff --git a/scripts/qapi.py b/scripts/qapi.py index 362e007..a2d869e 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -81,6 +81,7 @@ def error_path(parent): class QAPISchemaError(Exception): def __init__(self, schema, msg): + Exception.__init__(self) self.fname = schema.fname self.msg = msg self.col = 1 @@ -98,6 +99,7 @@ class QAPISchemaError(Exception): class QAPIExprError(Exception): def __init__(self, expr_info, msg): + Exception.__init__(self) self.info = expr_info self.msg = msg -- cgit v1.1 From 437db2549be383e52acad6cd4bf2862e98fdfc93 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 29 Sep 2015 16:21:02 -0600 Subject: qapi: Clean up qapi.py per pep8 Silence pep8, and make pylint a bit happier. Just style cleanups, plus killing a useless comment in camel_to_upper(); no semantic changes. Signed-off-by: Eric Blake Message-Id: <1443565276-4535-5-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- scripts/ordereddict.py | 3 +- scripts/qapi.py | 161 ++++++++++++++++++++++++++++++++----------------- 2 files changed, 106 insertions(+), 58 deletions(-) (limited to 'scripts') diff --git a/scripts/ordereddict.py b/scripts/ordereddict.py index 7242b50..2d1d813 100644 --- a/scripts/ordereddict.py +++ b/scripts/ordereddict.py @@ -22,6 +22,7 @@ from UserDict import DictMixin + class OrderedDict(dict, DictMixin): def __init__(self, *args, **kwds): @@ -117,7 +118,7 @@ class OrderedDict(dict, DictMixin): if isinstance(other, OrderedDict): if len(self) != len(other): return False - for p, q in zip(self.items(), other.items()): + for p, q in zip(self.items(), other.items()): if p != q: return False return True diff --git a/scripts/qapi.py b/scripts/qapi.py index a2d869e..4b5d574 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -71,6 +71,7 @@ all_names = {} # Parsing the schema into expressions # + def error_path(parent): res = "" while parent: @@ -79,6 +80,7 @@ def error_path(parent): parent = parent['parent'] return res + class QAPISchemaError(Exception): def __init__(self, schema, msg): Exception.__init__(self) @@ -97,6 +99,7 @@ class QAPISchemaError(Exception): return error_path(self.info) + \ "%s:%d:%d: %s" % (self.fname, self.line, self.col, self.msg) + class QAPIExprError(Exception): def __init__(self, expr_info, msg): Exception.__init__(self) @@ -107,9 +110,10 @@ class QAPIExprError(Exception): return error_path(self.info['parent']) + \ "%s:%d: %s" % (self.info['file'], self.info['line'], self.msg) + class QAPISchemaParser(object): - def __init__(self, fp, previously_included = [], incl_info = None): + def __init__(self, fp, previously_included=[], incl_info=None): abs_fname = os.path.abspath(fp.name) fname = fp.name self.fname = fname @@ -124,13 +128,14 @@ class QAPISchemaParser(object): self.exprs = [] self.accept() - while self.tok != None: + while self.tok is not None: expr_info = {'file': fname, 'line': self.line, 'parent': self.incl_info} expr = self.get_expr(False) if isinstance(expr, dict) and "include" in expr: if len(expr) != 1: - raise QAPIExprError(expr_info, "Invalid 'include' directive") + raise QAPIExprError(expr_info, + "Invalid 'include' directive") include = expr["include"] if not isinstance(include, str): raise QAPIExprError(expr_info, @@ -193,7 +198,7 @@ class QAPISchemaParser(object): string += '\t' elif ch == 'u': value = 0 - for x in range(0, 4): + for _ in range(0, 4): ch = self.src[self.cursor] self.cursor += 1 if ch not in "0123456789abcdefABCDEF": @@ -215,7 +220,7 @@ class QAPISchemaParser(object): string += ch else: raise QAPISchemaError(self, - "Unknown escape \\%s" %ch) + "Unknown escape \\%s" % ch) esc = False elif ch == "\\": esc = True @@ -275,7 +280,7 @@ class QAPISchemaParser(object): if self.tok == ']': self.accept() return expr - if not self.tok in "{['tfn": + if self.tok not in "{['tfn": raise QAPISchemaError(self, 'Expected "{", "[", "]", string, ' 'boolean or "null"') while True: @@ -309,15 +314,17 @@ class QAPISchemaParser(object): # TODO catching name collisions in generated code would be nice # + def find_base_fields(base): base_struct_define = find_struct(base) if not base_struct_define: return None return base_struct_define['data'] + # Return the qtype of an alternate branch, or None on error. def find_alternate_member_qtype(qapi_type): - if builtin_types.has_key(qapi_type): + if qapi_type in builtin_types: return builtin_types[qapi_type] elif find_struct(qapi_type): return "QTYPE_QDICT" @@ -327,6 +334,7 @@ def find_alternate_member_qtype(qapi_type): return "QTYPE_QDICT" return None + # Return the discriminator enum define if discriminator is specified as an # enum type, otherwise return None. def discriminator_find_enum_define(expr): @@ -346,11 +354,14 @@ def discriminator_find_enum_define(expr): return find_enum(discriminator_type) + # FIXME should enforce "other than downstream extensions [...], all # names should begin with a letter". valid_name = re.compile('^[a-zA-Z_][a-zA-Z0-9_.-]*$') -def check_name(expr_info, source, name, allow_optional = False, - enum_member = False): + + +def check_name(expr_info, source, name, allow_optional=False, + enum_member=False): global valid_name membername = name @@ -371,7 +382,8 @@ def check_name(expr_info, source, name, allow_optional = False, raise QAPIExprError(expr_info, "%s uses invalid name '%s'" % (source, name)) -def add_name(name, info, meta, implicit = False): + +def add_name(name, info, meta, implicit=False): global all_names check_name(info, "'%s'" % meta, name) # FIXME should reject names that differ only in '_' vs. '.' @@ -386,12 +398,14 @@ def add_name(name, info, meta, implicit = False): % (meta, name)) all_names[name] = meta + def add_struct(definition, info): global struct_types name = definition['struct'] add_name(name, info, 'struct') struct_types.append(definition) + def find_struct(name): global struct_types for struct in struct_types: @@ -399,12 +413,14 @@ def find_struct(name): return struct return None + def add_union(definition, info): global union_types name = definition['union'] add_name(name, info, 'union') union_types.append(definition) + def find_union(name): global union_types for union in union_types: @@ -412,11 +428,13 @@ def find_union(name): return union return None -def add_enum(name, info, enum_values = None, implicit = False): + +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): global enum_types for enum in enum_types: @@ -424,12 +442,14 @@ def find_enum(name): return enum return None + def is_enum(name): - return find_enum(name) != None + return find_enum(name) is not None + -def check_type(expr_info, source, value, allow_array = False, - allow_dict = False, allow_optional = False, - allow_metas = []): +def check_type(expr_info, source, value, allow_array=False, + allow_dict=False, allow_optional=False, + allow_metas=[]): global all_names if value is None: @@ -448,7 +468,7 @@ def check_type(expr_info, source, value, allow_array = False, # Check if type name for value is okay if isinstance(value, str): - if not value in all_names: + if value not in all_names: raise QAPIExprError(expr_info, "%s uses unknown type '%s'" % (source, value)) @@ -477,7 +497,8 @@ def check_type(expr_info, source, value, allow_array = False, allow_metas=['built-in', 'union', 'alternate', 'struct', 'enum']) -def check_member_clash(expr_info, base_name, data, source = ""): + +def check_member_clash(expr_info, base_name, data, source=""): base = find_struct(base_name) assert base base_members = base['data'] @@ -491,6 +512,7 @@ def check_member_clash(expr_info, base_name, data, source = ""): if base.get('base'): check_member_clash(expr_info, base['base'], data, source) + def check_command(expr, expr_info): name = expr['command'] @@ -504,6 +526,7 @@ def check_command(expr, expr_info): expr.get('returns'), allow_array=True, allow_optional=True, allow_metas=returns_meta) + def check_event(expr, expr_info): global events name = expr['event'] @@ -515,19 +538,20 @@ def check_event(expr, expr_info): expr.get('data'), allow_dict=True, allow_optional=True, allow_metas=['struct']) + def check_union(expr, expr_info): name = expr['union'] base = expr.get('base') discriminator = expr.get('discriminator') members = expr['data'] - values = { 'MAX': '(automatic)' } + values = {'MAX': '(automatic)'} # Two types of unions, determined by discriminator. # With no discriminator it is a simple union. if discriminator is None: enum_define = None - allow_metas=['built-in', 'union', 'alternate', 'struct', 'enum'] + allow_metas = ['built-in', 'union', 'alternate', 'struct', 'enum'] if base is not None: raise QAPIExprError(expr_info, "Simple union '%s' must not have a base" @@ -557,7 +581,7 @@ def check_union(expr, expr_info): "struct '%s'" % (discriminator, base)) enum_define = find_enum(discriminator_type) - allow_metas=['struct'] + allow_metas = ['struct'] # Do not allow string discriminator if not enum_define: raise QAPIExprError(expr_info, @@ -581,7 +605,7 @@ def check_union(expr, expr_info): # If the discriminator names an enum type, then all members # of 'data' must also be members of the enum type. if enum_define: - if not key in enum_define['enum_values']: + if key not in enum_define['enum_values']: raise QAPIExprError(expr_info, "Discriminator value '%s' is not found in " "enum '%s'" % @@ -596,10 +620,11 @@ def check_union(expr, expr_info): % (name, key, values[c_key])) values[c_key] = key + def check_alternate(expr, expr_info): name = expr['alternate'] members = expr['data'] - values = { 'MAX': '(automatic)' } + values = {'MAX': '(automatic)'} types_seen = {} # Check every branch @@ -627,11 +652,12 @@ def check_alternate(expr, expr_info): % (name, key, types_seen[qtype])) types_seen[qtype] = key + def check_enum(expr, expr_info): name = expr['enum'] members = expr.get('data') prefix = expr.get('prefix') - values = { 'MAX': '(automatic)' } + values = {'MAX': '(automatic)'} if not isinstance(members, list): raise QAPIExprError(expr_info, @@ -640,7 +666,7 @@ def check_enum(expr, expr_info): raise QAPIExprError(expr_info, "Enum '%s' requires a string for 'prefix'" % name) for member in members: - check_name(expr_info, "Member of enum '%s'" %name, member, + check_name(expr_info, "Member of enum '%s'" % name, member, enum_member=True) key = camel_to_upper(member) if key in values: @@ -649,6 +675,7 @@ def check_enum(expr, expr_info): % (name, member, values[key])) values[key] = member + def check_struct(expr, expr_info): name = expr['struct'] members = expr['data'] @@ -660,6 +687,7 @@ def check_struct(expr, expr_info): if expr.get('base'): check_member_clash(expr_info, expr['base'], expr['data']) + def check_keys(expr_elem, meta, required, optional=[]): expr = expr_elem['expr'] info = expr_elem['info'] @@ -667,22 +695,23 @@ def check_keys(expr_elem, meta, required, optional=[]): if not isinstance(name, str): raise QAPIExprError(info, "'%s' key must have a string value" % meta) - required = required + [ meta ] + required = required + [meta] for (key, value) in expr.items(): - if not key in required and not key in optional: + if key not in required and key not in optional: raise QAPIExprError(info, "Unknown key '%s' in %s '%s'" % (key, meta, name)) - if (key == 'gen' or key == 'success-response') and value != False: + if (key == 'gen' or key == 'success-response') and value is not False: raise QAPIExprError(info, "'%s' of %s '%s' should only use false value" % (key, meta, name)) for key in required: - if not expr.has_key(key): + if key not in expr: raise QAPIExprError(info, "Key '%s' is missing from %s '%s'" % (key, meta, name)) + def check_exprs(exprs): global all_names @@ -692,24 +721,24 @@ def check_exprs(exprs): for expr_elem in exprs: expr = expr_elem['expr'] info = expr_elem['info'] - if expr.has_key('enum'): + if 'enum' in expr: check_keys(expr_elem, 'enum', ['data'], ['prefix']) add_enum(expr['enum'], info, expr['data']) - elif expr.has_key('union'): + elif 'union' in expr: check_keys(expr_elem, 'union', ['data'], ['base', 'discriminator']) add_union(expr, info) - elif expr.has_key('alternate'): + elif 'alternate' in expr: check_keys(expr_elem, 'alternate', ['data']) add_name(expr['alternate'], info, 'alternate') - elif expr.has_key('struct'): + elif 'struct' in expr: check_keys(expr_elem, 'struct', ['data'], ['base']) add_struct(expr, info) - elif expr.has_key('command'): + elif 'command' in expr: check_keys(expr_elem, 'command', [], ['data', 'returns', 'gen', 'success-response']) add_name(expr['command'], info, 'command') - elif expr.has_key('event'): + elif 'event' in expr: check_keys(expr_elem, 'event', [], ['data']) add_name(expr['event'], info, 'event') else: @@ -719,11 +748,11 @@ def check_exprs(exprs): # Try again for hidden UnionKind enum for expr_elem in exprs: expr = expr_elem['expr'] - if expr.has_key('union'): + if 'union' in expr: if not discriminator_find_enum_define(expr): add_enum('%sKind' % expr['union'], expr_elem['info'], implicit=True) - elif expr.has_key('alternate'): + elif 'alternate' in expr: add_enum('%sKind' % expr['alternate'], expr_elem['info'], implicit=True) @@ -732,17 +761,17 @@ def check_exprs(exprs): expr = expr_elem['expr'] info = expr_elem['info'] - if expr.has_key('enum'): + if 'enum' in expr: check_enum(expr, info) - elif expr.has_key('union'): + elif 'union' in expr: check_union(expr, info) - elif expr.has_key('alternate'): + elif 'alternate' in expr: check_alternate(expr, info) - elif expr.has_key('struct'): + elif 'struct' in expr: check_struct(expr, info) - elif expr.has_key('command'): + elif 'command' in expr: check_command(expr, info) - elif expr.has_key('event'): + elif 'event' in expr: check_event(expr, info) else: assert False, 'unexpected meta type' @@ -994,6 +1023,7 @@ class QAPISchemaObjectTypeVariants(object): vseen = dict(seen) v.check(schema, self.tag_member.type, vseen) + class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember): def __init__(self, name, typ): QAPISchemaObjectTypeMember.__init__(self, name, typ, False) @@ -1293,6 +1323,7 @@ def camel_case(name): new_name += ch.lower() return new_name + # ENUMName -> ENUM_NAME, EnumName1 -> ENUM_NAME1 # ENUM_NAME -> ENUM_NAME, ENUM_NAME1 -> ENUM_NAME1, ENUM_Name2 -> ENUM_NAME2 # ENUM24_Name -> ENUM24_NAME @@ -1307,14 +1338,14 @@ def camel_to_upper(value): 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(): + if i < l - 1 and c_fun_str[i + 1].islower(): + new_name += '_' + elif c_fun_str[i - 1].isdigit(): new_name += '_' new_name += c return new_name.lstrip('_').upper() + def c_enum_const(type_name, const_name, prefix=None): if prefix is not None: type_name = prefix @@ -1322,6 +1353,7 @@ def c_enum_const(type_name, const_name, prefix=None): c_name_trans = string.maketrans('.-', '__') + # Map @name to a valid C identifier. # If @protect, avoid returning certain ticklish identifiers (like # C keywords) by prepending "q_". @@ -1334,15 +1366,16 @@ c_name_trans = string.maketrans('.-', '__') def c_name(name, protect=True): # ANSI X3J11/88-090, 3.1.1 c89_words = set(['auto', 'break', 'case', 'char', 'const', 'continue', - 'default', 'do', 'double', 'else', 'enum', 'extern', 'float', - 'for', 'goto', 'if', 'int', 'long', 'register', 'return', - 'short', 'signed', 'sizeof', 'static', 'struct', 'switch', - 'typedef', 'union', 'unsigned', 'void', 'volatile', 'while']) + 'default', 'do', 'double', 'else', 'enum', 'extern', + 'float', 'for', 'goto', 'if', 'int', 'long', 'register', + 'return', 'short', 'signed', 'sizeof', 'static', + 'struct', 'switch', 'typedef', 'union', 'unsigned', + 'void', 'volatile', 'while']) # ISO/IEC 9899:1999, 6.4.1 c99_words = set(['inline', 'restrict', '_Bool', '_Complex', '_Imaginary']) # ISO/IEC 9899:2011, 6.4.1 - c11_words = set(['_Alignas', '_Alignof', '_Atomic', '_Generic', '_Noreturn', - '_Static_assert', '_Thread_local']) + c11_words = set(['_Alignas', '_Alignof', '_Atomic', '_Generic', + '_Noreturn', '_Static_assert', '_Thread_local']) # GCC http://gcc.gnu.org/onlinedocs/gcc-4.7.1/gcc/C-Extensions.html # excluding _.* gcc_words = set(['asm', 'typeof']) @@ -1358,29 +1391,34 @@ def c_name(name, protect=True): 'not_eq', 'or', 'or_eq', 'xor', 'xor_eq']) # namespace pollution: polluted_words = set(['unix', 'errno']) - if protect and (name in c89_words | c99_words | c11_words | gcc_words | cpp_words | polluted_words): + if protect and (name in c89_words | c99_words | c11_words | gcc_words + | cpp_words | polluted_words): return "q_" + name return name.translate(c_name_trans) eatspace = '\033EATSPACE.' pointer_suffix = ' *' + eatspace + def genindent(count): ret = "" - for i in range(count): + for _ in range(count): ret += " " return ret indent_level = 0 + def push_indent(indent_amount=4): global indent_level indent_level += indent_amount + def pop_indent(indent_amount=4): global indent_level indent_level -= indent_amount + # Generate @code with @kwds interpolated. # Obey indent_level, and strip eatspace. def cgen(code, **kwds): @@ -1393,6 +1431,7 @@ def cgen(code, **kwds): raw = raw[0] return re.sub(re.escape(eatspace) + ' *', '', raw) + def mcgen(code, **kwds): if code[0] == '\n': code = code[1:] @@ -1402,6 +1441,7 @@ def mcgen(code, **kwds): def guardname(filename): return c_name(filename, protect=False).upper() + def guardstart(name): return mcgen(''' @@ -1411,6 +1451,7 @@ def guardstart(name): ''', name=guardname(name)) + def guardend(name): return mcgen(''' @@ -1419,6 +1460,7 @@ def guardend(name): ''', name=guardname(name)) + def gen_enum_lookup(name, values, prefix=None): ret = mcgen(''' @@ -1440,6 +1482,7 @@ const char *const %(c_name)s_lookup[] = { max_index=max_index) return ret + def gen_enum(name, values, prefix=None): # append automatically generated _MAX value enum_values = values + ['MAX'] @@ -1471,6 +1514,7 @@ extern const char *const %(c_name)s_lookup[]; c_name=c_name(name)) return ret + def gen_params(arg_type, extra): if not arg_type: return extra @@ -1491,7 +1535,8 @@ def gen_params(arg_type, extra): # Common command line parsing # -def parse_command_line(extra_options = "", extra_long_options = []): + +def parse_command_line(extra_options="", extra_long_options=[]): try: opts, args = getopt.gnu_getopt(sys.argv[1:], @@ -1542,6 +1587,7 @@ def parse_command_line(extra_options = "", extra_long_options = []): # Generate output files with boilerplate # + def open_output(output_dir, do_c, do_h, prefix, c_file, h_file, c_comment, h_comment): guard = guardname(prefix + h_file) @@ -1569,7 +1615,7 @@ def open_output(output_dir, do_c, do_h, prefix, c_file, h_file, /* AUTOMATICALLY GENERATED, DO NOT MODIFY */ %(comment)s ''', - comment = c_comment)) + comment=c_comment)) fdecl.write(mcgen(''' /* AUTOMATICALLY GENERATED, DO NOT MODIFY */ @@ -1578,10 +1624,11 @@ def open_output(output_dir, do_c, do_h, prefix, c_file, h_file, #define %(guard)s ''', - comment = h_comment, guard = guard)) + comment=h_comment, guard=guard)) return (fdef, fdecl) + def close_output(fdef, fdecl): fdecl.write(''' #endif -- cgit v1.1 From 7b2a5c2f9a52c4a08630fa741052f03fe5d3cc8a Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 29 Sep 2015 16:21:04 -0600 Subject: qapi: Avoid assertion failure on union 'type' collision The previous commit added two tests that triggered an assertion failure. It's fairly straightforward to avoid the failure by just outright forbidding the collision between a union's tag values and its discriminator name (including the implicit name 'kind' supplied for simple unions [*]). Ultimately, we'd like to move the collision detection into QAPISchema*.check(), but for now it is easier just to enhance the existing checks. [*] Of course, down the road, we have plans to rename the simple union tag name to 'type' to match the QMP wire name, but the idea of the collision will still be present even then. Technically, we could avoid the collision by naming the C union members representing each enum value as '_case_value' rather than 'value'; but until we have an actual qapi client (and not just our testsuite) that has a legitimate reason to match a case label to the name of a QMP key and needs the name munging to satisfy the compiler, it's easier to just reject the qapi as invalid. Signed-off-by: Eric Blake Message-Id: <1443565276-4535-7-git-send-email-eblake@redhat.com> [Polished a few comments] Signed-off-by: Markus Armbruster --- scripts/qapi.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'scripts') diff --git a/scripts/qapi.py b/scripts/qapi.py index 4b5d574..8d2681b 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -544,7 +544,7 @@ def check_union(expr, expr_info): base = expr.get('base') discriminator = expr.get('discriminator') members = expr['data'] - values = {'MAX': '(automatic)'} + values = {'MAX': '(automatic)', 'KIND': '(automatic)'} # Two types of unions, determined by discriminator. @@ -603,13 +603,19 @@ def check_union(expr, expr_info): " of branch '%s'" % key) # If the discriminator names an enum type, then all members - # of 'data' must also be members of the enum type. + # of 'data' must also be members of the enum type, which in turn + # must not collide with the discriminator name. if enum_define: if key not in enum_define['enum_values']: raise QAPIExprError(expr_info, "Discriminator value '%s' is not found in " "enum '%s'" % (key, enum_define["enum_name"])) + if discriminator in enum_define['enum_values']: + raise QAPIExprError(expr_info, + "Discriminator name '%s' collides with " + "enum value in '%s'" % + (discriminator, enum_define["enum_name"])) # Otherwise, check for conflicts in the generated enum else: -- cgit v1.1 From 376863ef4895ae709aadb6f26365a5973310ef09 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 29 Sep 2015 16:21:07 -0600 Subject: qapi: Reuse code for flat union base validation Rather than open-code the check for a valid base type, we should reuse the common functionality. This allows for consistent error messages, and also makes it easier for a later patch to turn on support for inline anonymous base structures. Test flat-union-inline is updated to test only one feature (anonymous branch dictionaries), which can be implemented independently (test flat-union-bad-base already covers the idea of an anonymous base dictionary). Signed-off-by: Eric Blake Message-Id: <1443565276-4535-10-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- scripts/qapi.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'scripts') diff --git a/scripts/qapi.py b/scripts/qapi.py index 8d2681b..c0728d7 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -560,15 +560,14 @@ def check_union(expr, expr_info): # Else, it's a flat union. else: # The object must have a string member 'base'. - if not isinstance(base, str): + check_type(expr_info, "'base' for union '%s'" % name, + base, allow_metas=['struct']) + if not base: raise QAPIExprError(expr_info, - "Flat union '%s' must have a string base field" + "Flat union '%s' must have a base" % name) base_fields = find_base_fields(base) - if not base_fields: - raise QAPIExprError(expr_info, - "Base '%s' is not a valid struct" - % base) + assert base_fields # The value of member 'discriminator' must name a non-optional # member of the base struct. -- cgit v1.1 From 2a0f50e8d973b01eda4c63bac4a5c79ea0f584ef Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 29 Sep 2015 16:21:08 -0600 Subject: qapi: Consistent generated code: prefer error 'err' We had some pointless differences in the generated code for visit, command marshalling, and events; unifying them makes it easier for future patches to consolidate to common helper functions. This is one patch of a series to clean up these differences. This patch consistently names the local error variable 'err' rather than 'local_err'. No change in semantics to the generated code. Signed-off-by: Eric Blake Message-Id: <1443565276-4535-11-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- scripts/qapi-commands.py | 22 +++++++++++----------- scripts/qapi-event.py | 18 +++++++++--------- 2 files changed, 20 insertions(+), 20 deletions(-) (limited to 'scripts') diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 810a897..2151120 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -53,14 +53,14 @@ def gen_call(name, arg_type, ret_type): push_indent() ret = mcgen(''' -%(lhs)sqmp_%(c_name)s(%(args)s&local_err); +%(lhs)sqmp_%(c_name)s(%(args)s&err); ''', c_name=c_name(name), args=argstr, lhs=lhs) if ret_type: - ret += gen_err_check('local_err') + ret += gen_err_check('err') ret += mcgen(''' -qmp_marshal_output_%(c_name)s(retval, ret, &local_err); +qmp_marshal_output_%(c_name)s(retval, ret, &err); ''', c_name=ret_type.c_name()) pop_indent() @@ -69,7 +69,7 @@ qmp_marshal_output_%(c_name)s(retval, ret, &local_err); def gen_marshal_vars(arg_type, ret_type): ret = mcgen(''' - Error *local_err = NULL; + Error *err = NULL; ''') push_indent() @@ -127,8 +127,8 @@ md = qapi_dealloc_visitor_new(); v = qapi_dealloc_get_visitor(md); ''') else: - errparg = '&local_err' - errarg = 'local_err' + errparg = '&err' + errarg = 'err' ret += mcgen(''' v = qmp_input_get_visitor(mi); ''') @@ -171,20 +171,20 @@ def gen_marshal_output(ret_type): static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out, Error **errp) { - Error *local_err = NULL; + Error *err = NULL; QmpOutputVisitor *mo = qmp_output_visitor_new(); QapiDeallocVisitor *md; Visitor *v; v = qmp_output_get_visitor(mo); - visit_type_%(c_name)s(v, &ret_in, "unused", &local_err); - if (local_err) { + visit_type_%(c_name)s(v, &ret_in, "unused", &err); + if (err) { goto out; } *ret_out = qmp_output_get_qobject(mo); out: - error_propagate(errp, local_err); + error_propagate(errp, err); qmp_output_visitor_cleanup(mo); md = qapi_dealloc_visitor_new(); v = qapi_dealloc_get_visitor(md); @@ -227,7 +227,7 @@ def gen_marshal(name, arg_type, ret_type): out: ''') ret += mcgen(''' - error_propagate(errp, local_err); + error_propagate(errp, err); ''') ret += gen_marshal_input_visit(arg_type, dealloc=True) ret += mcgen(''' diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py index d15fad9..d41af40 100644 --- a/scripts/qapi-event.py +++ b/scripts/qapi-event.py @@ -34,7 +34,7 @@ def gen_event_send(name, arg_type): %(proto)s { QDict *qmp; - Error *local_err = NULL; + Error *err = NULL; QMPEventFuncEmit emit; ''', proto=gen_event_send_proto(name, arg_type)) @@ -67,8 +67,8 @@ def gen_event_send(name, arg_type): g_assert(v); /* Fake visit, as if all members are under a structure */ - visit_start_struct(v, NULL, "", "%(name)s", 0, &local_err); - if (local_err) { + visit_start_struct(v, NULL, "", "%(name)s", 0, &err); + if (err) { goto clean; } @@ -90,8 +90,8 @@ def gen_event_send(name, arg_type): cast = '' ret += mcgen(''' - visit_type_%(c_type)s(v, %(cast)s&%(c_name)s, "%(name)s", &local_err); - if (local_err) { + visit_type_%(c_type)s(v, %(cast)s&%(c_name)s, "%(name)s", &err); + if (err) { goto clean; } ''', @@ -108,8 +108,8 @@ def gen_event_send(name, arg_type): ret += mcgen(''' - visit_end_struct(v, &local_err); - if (local_err) { + visit_end_struct(v, &err); + if (err) { goto clean; } @@ -120,7 +120,7 @@ def gen_event_send(name, arg_type): ''') ret += mcgen(''' - emit(%(c_enum)s, qmp, &local_err); + emit(%(c_enum)s, qmp, &err); ''', c_enum=c_enum_const(event_enum_name, name)) @@ -131,7 +131,7 @@ def gen_event_send(name, arg_type): qmp_output_visitor_cleanup(qov); ''') ret += mcgen(''' - error_propagate(errp, local_err); + error_propagate(errp, err); QDECREF(qmp); } ''') -- cgit v1.1 From f8b7f1a8eafa9f565ebecfe409e8741d38cd786b Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 29 Sep 2015 16:21:09 -0600 Subject: qapi: Consistent generated code: prefer visitor 'v' We had some pointless differences in the generated code for visit, command marshalling, and events; unifying them makes it easier for future patches to consolidate to common helper functions. This is one patch of a series to clean up these differences. This patch names the local visitor variable 'v' rather than 'm'. Related objects, such as 'QapiDeallocVisitor', are also named by their initials instead of an unrelated leading m. No change in semantics to the generated code. Signed-off-by: Eric Blake Message-Id: <1443565276-4535-12-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- scripts/qapi-commands.py | 30 ++++++++++----------- scripts/qapi-types.py | 8 +++--- scripts/qapi-visit.py | 70 ++++++++++++++++++++++++------------------------ 3 files changed, 54 insertions(+), 54 deletions(-) (limited to 'scripts') diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 2151120..475735d 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -82,8 +82,8 @@ def gen_marshal_vars(arg_type, ret_type): if arg_type: ret += mcgen(''' -QmpInputVisitor *mi = qmp_input_visitor_new_strict(QOBJECT(args)); -QapiDeallocVisitor *md; +QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args)); +QapiDeallocVisitor *qdv; Visitor *v; ''') @@ -122,15 +122,15 @@ def gen_marshal_input_visit(arg_type, dealloc=False): errparg = 'NULL' errarg = None ret += mcgen(''' -qmp_input_visitor_cleanup(mi); -md = qapi_dealloc_visitor_new(); -v = qapi_dealloc_get_visitor(md); +qmp_input_visitor_cleanup(qiv); +qdv = qapi_dealloc_visitor_new(); +v = qapi_dealloc_get_visitor(qdv); ''') else: errparg = '&err' errarg = 'err' ret += mcgen(''' -v = qmp_input_get_visitor(mi); +v = qmp_input_get_visitor(qiv); ''') for memb in arg_type.members: @@ -160,7 +160,7 @@ visit_type_%(c_type)s(v, &%(c_name)s, "%(name)s", %(errp)s); if dealloc: ret += mcgen(''' -qapi_dealloc_visitor_cleanup(md); +qapi_dealloc_visitor_cleanup(qdv); ''') pop_indent() return ret @@ -172,24 +172,24 @@ def gen_marshal_output(ret_type): static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out, Error **errp) { Error *err = NULL; - QmpOutputVisitor *mo = qmp_output_visitor_new(); - QapiDeallocVisitor *md; + QmpOutputVisitor *qov = qmp_output_visitor_new(); + QapiDeallocVisitor *qdv; Visitor *v; - v = qmp_output_get_visitor(mo); + v = qmp_output_get_visitor(qov); visit_type_%(c_name)s(v, &ret_in, "unused", &err); if (err) { goto out; } - *ret_out = qmp_output_get_qobject(mo); + *ret_out = qmp_output_get_qobject(qov); out: error_propagate(errp, err); - qmp_output_visitor_cleanup(mo); - md = qapi_dealloc_visitor_new(); - v = qapi_dealloc_get_visitor(md); + qmp_output_visitor_cleanup(qov); + qdv = qapi_dealloc_visitor_new(); + v = qapi_dealloc_get_visitor(qdv); visit_type_%(c_name)s(v, &ret_in, "unused", NULL); - qapi_dealloc_visitor_cleanup(md); + qapi_dealloc_visitor_cleanup(qdv); } ''', c_type=ret_type.c_type(), c_name=ret_type.c_name()) diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index b292682..d405f8d 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -188,17 +188,17 @@ def gen_type_cleanup(name): void qapi_free_%(c_name)s(%(c_name)s *obj) { - QapiDeallocVisitor *md; + QapiDeallocVisitor *qdv; Visitor *v; if (!obj) { return; } - md = qapi_dealloc_visitor_new(); - v = qapi_dealloc_get_visitor(md); + qdv = qapi_dealloc_visitor_new(); + v = qapi_dealloc_get_visitor(qdv); visit_type_%(c_name)s(v, &obj, NULL, NULL); - qapi_dealloc_visitor_cleanup(md); + qapi_dealloc_visitor_cleanup(qdv); } ''', c_name=c_name(name)) diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 97343cf..348efe0 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -24,7 +24,7 @@ def gen_visit_decl(name, scalar=False): if not scalar: c_type += '*' return mcgen(''' -void visit_type_%(c_name)s(Visitor *m, %(c_type)sobj, const char *name, Error **errp); +void visit_type_%(c_name)s(Visitor *v, %(c_type)sobj, const char *name, Error **errp); ''', c_name=c_name(name), c_type=c_type) @@ -39,20 +39,20 @@ def gen_visit_implicit_struct(typ): # Need a forward declaration ret += mcgen(''' -static void visit_type_%(c_type)s_fields(Visitor *m, %(c_type)s **obj, Error **errp); +static void visit_type_%(c_type)s_fields(Visitor *v, %(c_type)s **obj, Error **errp); ''', c_type=typ.c_name()) ret += mcgen(''' -static void visit_type_implicit_%(c_type)s(Visitor *m, %(c_type)s **obj, Error **errp) +static void visit_type_implicit_%(c_type)s(Visitor *v, %(c_type)s **obj, Error **errp) { Error *err = NULL; - visit_start_implicit_struct(m, (void **)obj, sizeof(%(c_type)s), &err); + visit_start_implicit_struct(v, (void **)obj, sizeof(%(c_type)s), &err); if (!err) { - visit_type_%(c_type)s_fields(m, obj, errp); - visit_end_implicit_struct(m, &err); + visit_type_%(c_type)s_fields(v, obj, errp); + visit_end_implicit_struct(v, &err); } error_propagate(errp, err); } @@ -71,7 +71,7 @@ def gen_visit_struct_fields(name, base, members): ret += mcgen(''' -static void visit_type_%(c_name)s_fields(Visitor *m, %(c_name)s **obj, Error **errp) +static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s **obj, Error **errp) { Error *err = NULL; @@ -81,7 +81,7 @@ static void visit_type_%(c_name)s_fields(Visitor *m, %(c_name)s **obj, Error **e if base: ret += mcgen(''' -visit_type_implicit_%(c_type)s(m, &(*obj)->%(c_name)s, &err); +visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err); if (err) { goto out; } @@ -91,14 +91,14 @@ if (err) { for memb in members: if memb.optional: ret += mcgen(''' -visit_optional(m, &(*obj)->has_%(c_name)s, "%(name)s", &err); +visit_optional(v, &(*obj)->has_%(c_name)s, "%(name)s", &err); if (!err && (*obj)->has_%(c_name)s) { ''', c_name=c_name(memb.name), name=memb.name) push_indent() ret += mcgen(''' -visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "%(name)s", &err); +visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err); ''', c_type=memb.type.c_name(), c_name=c_name(memb.name), name=memb.name) @@ -136,16 +136,16 @@ def gen_visit_struct(name, base, members): # call qapi_free_FOO() to avoid a memory leak of the partial FOO. ret += mcgen(''' -void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error **errp) +void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error **errp) { Error *err = NULL; - visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), &err); + visit_start_struct(v, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), &err); if (!err) { if (*obj) { - visit_type_%(c_name)s_fields(m, obj, errp); + visit_type_%(c_name)s_fields(v, obj, errp); } - visit_end_struct(m, &err); + visit_end_struct(v, &err); } error_propagate(errp, err); } @@ -158,26 +158,26 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error def gen_visit_list(name, element_type): return mcgen(''' -void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error **errp) +void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error **errp) { Error *err = NULL; GenericList *i, **prev; - visit_start_list(m, name, &err); + visit_start_list(v, name, &err); if (err) { goto out; } for (prev = (GenericList **)obj; - !err && (i = visit_next_list(m, prev, &err)) != NULL; + !err && (i = visit_next_list(v, prev, &err)) != NULL; prev = &i) { %(c_name)s *native_i = (%(c_name)s *)i; - visit_type_%(c_elt_type)s(m, &native_i->value, NULL, &err); + visit_type_%(c_elt_type)s(v, &native_i->value, NULL, &err); } error_propagate(errp, err); err = NULL; - visit_end_list(m, &err); + visit_end_list(v, &err); out: error_propagate(errp, err); } @@ -188,9 +188,9 @@ out: def gen_visit_enum(name): return mcgen(''' -void visit_type_%(c_name)s(Visitor *m, %(c_name)s *obj, const char *name, Error **errp) +void visit_type_%(c_name)s(Visitor *v, %(c_name)s *obj, const char *name, Error **errp) { - visit_type_enum(m, (int *)obj, %(c_name)s_lookup, "%(name)s", name, errp); + visit_type_enum(v, (int *)obj, %(c_name)s_lookup, "%(name)s", name, errp); } ''', c_name=c_name(name), name=name) @@ -199,15 +199,15 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s *obj, const char *name, Error def gen_visit_alternate(name, variants): ret = mcgen(''' -void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error **errp) +void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error **errp) { Error *err = NULL; - visit_start_implicit_struct(m, (void**) obj, sizeof(%(c_name)s), &err); + visit_start_implicit_struct(v, (void**) obj, sizeof(%(c_name)s), &err); if (err) { goto out; } - visit_get_next_type(m, (int*) &(*obj)->kind, %(c_name)s_qtypes, name, &err); + visit_get_next_type(v, (int*) &(*obj)->kind, %(c_name)s_qtypes, name, &err); if (err) { goto out_end; } @@ -218,7 +218,7 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error for var in variants.variants: ret += mcgen(''' case %(case)s: - visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, name, &err); + visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, name, &err); break; ''', case=c_enum_const(variants.tag_member.type.name, @@ -233,7 +233,7 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error out_end: error_propagate(errp, err); err = NULL; - visit_end_implicit_struct(m, &err); + visit_end_implicit_struct(v, &err); out: error_propagate(errp, err); } @@ -256,11 +256,11 @@ def gen_visit_union(name, base, variants): ret += mcgen(''' -void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error **errp) +void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error **errp) { Error *err = NULL; - visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), &err); + visit_start_struct(v, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), &err); if (err) { goto out; } @@ -270,7 +270,7 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error if base: ret += mcgen(''' - visit_type_%(c_name)s_fields(m, obj, &err); + visit_type_%(c_name)s_fields(v, obj, &err); if (err) { goto out_obj; } @@ -282,11 +282,11 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error # we pointlessly use a different key for simple unions tag_key = 'type' ret += mcgen(''' - visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "%(name)s", &err); + visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err); if (err) { goto out_obj; } - if (!visit_start_union(m, !!(*obj)->data, &err) || err) { + if (!visit_start_union(v, !!(*obj)->data, &err) || err) { goto out_obj; } switch ((*obj)->%(c_name)s) { @@ -308,13 +308,13 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error var.name)) if simple_union_type: ret += mcgen(''' - visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "data", &err); + visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "data", &err); ''', c_type=simple_union_type.c_name(), c_name=c_name(var.name)) else: ret += mcgen(''' - visit_type_implicit_%(c_type)s(m, &(*obj)->%(c_name)s, &err); + visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err); ''', c_type=var.type.c_name(), c_name=c_name(var.name)) @@ -329,11 +329,11 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error out_obj: error_propagate(errp, err); err = NULL; - visit_end_union(m, !!(*obj)->data, &err); + visit_end_union(v, !!(*obj)->data, &err); error_propagate(errp, err); err = NULL; } - visit_end_struct(m, &err); + visit_end_struct(v, &err); out: error_propagate(errp, err); } -- cgit v1.1 From f782399cb4fa3fc4182cb046817f65a6db92ab07 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 29 Sep 2015 16:21:10 -0600 Subject: qapi: Consistent generated code: prefer common labels We had some pointless differences in the generated code for visit, command marshalling, and events; unifying them makes it easier for future patches to consolidate to common helper functions. This is one patch of a series to clean up these differences. This patch names the goto labels 'out' (not 'clean') and 'out_obj' (not 'out_end'). Additionally, the generator was inconsistent on whether labels had a leading space [our HACKING is silent; while emacs 'gnu' style adds the space to avoid littering column 1]. For minimal churn, prefer no leading space; this also matches the style that is more prevalent in current qemu.git. No change in semantics to the generated code. Signed-off-by: Eric Blake Message-Id: <1443565276-4535-13-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- scripts/qapi-event.py | 8 ++++---- scripts/qapi-visit.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) (limited to 'scripts') diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py index d41af40..b5a9d4f 100644 --- a/scripts/qapi-event.py +++ b/scripts/qapi-event.py @@ -69,7 +69,7 @@ def gen_event_send(name, arg_type): /* Fake visit, as if all members are under a structure */ visit_start_struct(v, NULL, "", "%(name)s", 0, &err); if (err) { - goto clean; + goto out; } ''', @@ -92,7 +92,7 @@ def gen_event_send(name, arg_type): ret += mcgen(''' visit_type_%(c_type)s(v, %(cast)s&%(c_name)s, "%(name)s", &err); if (err) { - goto clean; + goto out; } ''', cast=cast, @@ -110,7 +110,7 @@ def gen_event_send(name, arg_type): visit_end_struct(v, &err); if (err) { - goto clean; + goto out; } obj = qmp_output_get_qobject(qov); @@ -127,7 +127,7 @@ def gen_event_send(name, arg_type): if arg_type and arg_type.members: ret += mcgen(''' - clean: +out: qmp_output_visitor_cleanup(qov); ''') ret += mcgen(''' diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 348efe0..5a453ea 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -209,7 +209,7 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error } visit_get_next_type(v, (int*) &(*obj)->kind, %(c_name)s_qtypes, name, &err); if (err) { - goto out_end; + goto out_obj; } switch ((*obj)->kind) { ''', @@ -230,7 +230,7 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error default: abort(); } -out_end: +out_obj: error_propagate(errp, err); err = NULL; visit_end_implicit_struct(v, &err); -- cgit v1.1 From e36c714e6aad7c9266132350833e2f263f6d8874 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 29 Sep 2015 16:21:11 -0600 Subject: qapi: Consistent generated code: prefer common indentation We had some pointless differences in the generated code for visit, command marshalling, and events; unifying them makes it easier for future patches to consolidate to common helper functions. This is one patch of a series to clean up these differences. This patch adjusts gen_visit_union() to use the same indentation as other functions, namely, by jumping early to the error label if the object was not set rather than placing the rest of the body inside an if for when it is set. No change in semantics to the generated code. Signed-off-by: Eric Blake Message-Id: <1443565276-4535-14-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- scripts/qapi-visit.py | 53 ++++++++++++++++++++++++++------------------------- 1 file changed, 27 insertions(+), 26 deletions(-) (limited to 'scripts') diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 5a453ea..fe9780e 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -264,16 +264,18 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error if (err) { goto out; } - if (*obj) { + if (!*obj) { + goto out_obj; + } ''', c_name=c_name(name), name=name) if base: ret += mcgen(''' - visit_type_%(c_name)s_fields(v, obj, &err); - if (err) { - goto out_obj; - } + visit_type_%(c_name)s_fields(v, obj, &err); + if (err) { + goto out_obj; + } ''', c_name=c_name(name)) @@ -282,14 +284,14 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error # we pointlessly use a different key for simple unions tag_key = 'type' ret += mcgen(''' - visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err); - if (err) { - goto out_obj; - } - if (!visit_start_union(v, !!(*obj)->data, &err) || err) { - goto out_obj; - } - switch ((*obj)->%(c_name)s) { + visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err); + if (err) { + goto out_obj; + } + if (!visit_start_union(v, !!(*obj)->data, &err) || err) { + goto out_obj; + } + switch ((*obj)->%(c_name)s) { ''', c_type=variants.tag_member.type.c_name(), # TODO ugly special case for simple union @@ -302,37 +304,36 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error # TODO ugly special case for simple union simple_union_type = var.simple_union_type() ret += mcgen(''' - case %(case)s: + case %(case)s: ''', case=c_enum_const(variants.tag_member.type.name, var.name)) if simple_union_type: ret += mcgen(''' - visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "data", &err); + visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "data", &err); ''', c_type=simple_union_type.c_name(), c_name=c_name(var.name)) else: ret += mcgen(''' - visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err); + visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err); ''', c_type=var.type.c_name(), c_name=c_name(var.name)) ret += mcgen(''' - break; + break; ''') ret += mcgen(''' - default: - abort(); - } -out_obj: - error_propagate(errp, err); - err = NULL; - visit_end_union(v, !!(*obj)->data, &err); - error_propagate(errp, err); - err = NULL; + default: + abort(); } +out_obj: + error_propagate(errp, err); + err = NULL; + visit_end_union(v, !!(*obj)->data, &err); + error_propagate(errp, err); + err = NULL; visit_end_struct(v, &err); out: error_propagate(errp, err); -- cgit v1.1 From 05372f708a8cb3556e4d67458de79417dadf241f Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 29 Sep 2015 16:21:12 -0600 Subject: qapi: Consistent generated code: minimize push_indent() usage We had some pointless differences in the generated code for visit, command marshalling, and events; unifying them makes it easier for future patches to consolidate to common helper functions. This is one patch of a series to clean up these differences. This patch reduces the number of push_indent()/pop_indent() pairs so that generated code is typically already at its natural output indentation in the python files. It is easier to reason about generated code if the reader does not have to track how much spacing will be inserted alongside the code, and moreso when all of the generators use the same patterns (qapi-type and qapi-event were already using in-place indentation). Arguably, the resulting python may be a bit harder to read with C code at the same indentation as python; on the other hand, not having to think about push_indent() is a win, and most decent editors provide syntax highlighting that makes it easier to visually distinguish python code from string literals that will become C code. There is no change to the generated output. Signed-off-by: Eric Blake Message-Id: <1443565276-4535-15-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- scripts/qapi-commands.py | 54 ++++++++++++++++++++---------------------------- scripts/qapi-visit.py | 24 ++++++++++----------- 2 files changed, 33 insertions(+), 45 deletions(-) (limited to 'scripts') diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 475735d..267991e 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -29,9 +29,9 @@ def gen_err_check(err): if not err: return '' return mcgen(''' -if (%(err)s) { - goto out; -} + if (%(err)s) { + goto out; + } ''', err=err) @@ -50,20 +50,18 @@ def gen_call(name, arg_type, ret_type): if ret_type: lhs = 'retval = ' - push_indent() ret = mcgen(''' -%(lhs)sqmp_%(c_name)s(%(args)s&err); + %(lhs)sqmp_%(c_name)s(%(args)s&err); ''', c_name=c_name(name), args=argstr, lhs=lhs) if ret_type: ret += gen_err_check('err') ret += mcgen(''' -qmp_marshal_output_%(c_name)s(retval, ret, &err); + qmp_marshal_output_%(c_name)s(retval, ret, &err); ''', c_name=ret_type.c_name()) - pop_indent() return ret @@ -72,29 +70,27 @@ def gen_marshal_vars(arg_type, ret_type): Error *err = NULL; ''') - push_indent() - if ret_type: ret += mcgen(''' -%(c_type)s retval; + %(c_type)s retval; ''', c_type=ret_type.c_type()) if arg_type: ret += mcgen(''' -QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args)); -QapiDeallocVisitor *qdv; -Visitor *v; + QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args)); + QapiDeallocVisitor *qdv; + Visitor *v; ''') for memb in arg_type.members: if memb.optional: ret += mcgen(''' -bool has_%(c_name)s = false; + bool has_%(c_name)s = false; ''', c_name=c_name(memb.name)) ret += mcgen(''' -%(c_type)s %(c_name)s = %(c_null)s; + %(c_type)s %(c_name)s = %(c_null)s; ''', c_name=c_name(memb.name), c_type=memb.type.c_type(), @@ -103,10 +99,9 @@ bool has_%(c_name)s = false; else: ret += mcgen(''' -(void)args; + (void)args; ''') - pop_indent() return ret @@ -116,38 +111,36 @@ def gen_marshal_input_visit(arg_type, dealloc=False): if not arg_type: return ret - push_indent() - if dealloc: errparg = 'NULL' errarg = None ret += mcgen(''' -qmp_input_visitor_cleanup(qiv); -qdv = qapi_dealloc_visitor_new(); -v = qapi_dealloc_get_visitor(qdv); + qmp_input_visitor_cleanup(qiv); + qdv = qapi_dealloc_visitor_new(); + v = qapi_dealloc_get_visitor(qdv); ''') else: errparg = '&err' errarg = 'err' ret += mcgen(''' -v = qmp_input_get_visitor(qiv); + v = qmp_input_get_visitor(qiv); ''') for memb in arg_type.members: if memb.optional: ret += mcgen(''' -visit_optional(v, &has_%(c_name)s, "%(name)s", %(errp)s); + visit_optional(v, &has_%(c_name)s, "%(name)s", %(errp)s); ''', c_name=c_name(memb.name), name=memb.name, errp=errparg) ret += gen_err_check(errarg) ret += mcgen(''' -if (has_%(c_name)s) { + if (has_%(c_name)s) { ''', c_name=c_name(memb.name)) push_indent() ret += mcgen(''' -visit_type_%(c_type)s(v, &%(c_name)s, "%(name)s", %(errp)s); + visit_type_%(c_type)s(v, &%(c_name)s, "%(name)s", %(errp)s); ''', c_name=c_name(memb.name), name=memb.name, c_type=memb.type.c_name(), errp=errparg) @@ -155,14 +148,13 @@ visit_type_%(c_type)s(v, &%(c_name)s, "%(name)s", %(errp)s); if memb.optional: pop_indent() ret += mcgen(''' -} + } ''') if dealloc: ret += mcgen(''' -qapi_dealloc_visitor_cleanup(qdv); + qapi_dealloc_visitor_cleanup(qdv); ''') - pop_indent() return ret @@ -237,17 +229,15 @@ out: def gen_register_command(name, success_response): - push_indent() options = 'QCO_NO_OPTIONS' if not success_response: options = 'QCO_NO_SUCCESS_RESP' ret = mcgen(''' -qmp_register_command("%(name)s", qmp_marshal_%(c_name)s, %(opts)s); + qmp_register_command("%(name)s", qmp_marshal_%(c_name)s, %(opts)s); ''', name=name, c_name=c_name(name), opts=options) - pop_indent() return ret diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index fe9780e..78ad556 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -77,28 +77,27 @@ static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s **obj, Error **e ''', c_name=c_name(name)) - push_indent() if base: ret += mcgen(''' -visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err); -if (err) { - goto out; -} + visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err); + if (err) { + goto out; + } ''', c_type=base.c_name(), c_name=c_name('base')) for memb in members: if memb.optional: ret += mcgen(''' -visit_optional(v, &(*obj)->has_%(c_name)s, "%(name)s", &err); -if (!err && (*obj)->has_%(c_name)s) { + visit_optional(v, &(*obj)->has_%(c_name)s, "%(name)s", &err); + if (!err && (*obj)->has_%(c_name)s) { ''', c_name=c_name(memb.name), name=memb.name) push_indent() ret += mcgen(''' -visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err); + visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err); ''', c_type=memb.type.c_name(), c_name=c_name(memb.name), name=memb.name) @@ -106,15 +105,14 @@ visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err); if memb.optional: pop_indent() ret += mcgen(''' -} + } ''') ret += mcgen(''' -if (err) { - goto out; -} + if (err) { + goto out; + } ''') - pop_indent() if re.search('^ *goto out;', ret, re.MULTILINE): ret += mcgen(''' -- cgit v1.1 From 1f35334489a43800df4d20cd91362a87cee39a29 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 29 Sep 2015 16:21:13 -0600 Subject: qapi: Share gen_err_check() qapi-commands has a nice helper gen_err_check(), but did not use it everywhere. In fact, using it in more places makes it easier to reduce the lines of code used for generating error checks. This in turn will make it easier for later patches to consolidate another common pattern among the generators. The generated code has fewer blank lines in qapi-event.c functions, but has no semantic difference. Signed-off-by: Eric Blake Message-Id: <1443565276-4535-16-git-send-email-eblake@redhat.com> [Drop another blank line for symmetry] Signed-off-by: Markus Armbruster --- scripts/qapi-commands.py | 17 +++-------------- scripts/qapi-event.py | 10 ++-------- scripts/qapi-visit.py | 14 +++----------- scripts/qapi.py | 12 ++++++++++++ 4 files changed, 20 insertions(+), 33 deletions(-) (limited to 'scripts') diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 267991e..4e99c1d 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -25,17 +25,6 @@ def gen_command_decl(name, arg_type, ret_type): params=gen_params(arg_type, 'Error **errp')) -def gen_err_check(err): - if not err: - return '' - return mcgen(''' - if (%(err)s) { - goto out; - } -''', - err=err) - - def gen_call(name, arg_type, ret_type): ret = '' @@ -56,7 +45,7 @@ def gen_call(name, arg_type, ret_type): ''', c_name=c_name(name), args=argstr, lhs=lhs) if ret_type: - ret += gen_err_check('err') + ret += gen_err_check() ret += mcgen(''' qmp_marshal_output_%(c_name)s(retval, ret, &err); @@ -133,7 +122,7 @@ def gen_marshal_input_visit(arg_type, dealloc=False): ''', c_name=c_name(memb.name), name=memb.name, errp=errparg) - ret += gen_err_check(errarg) + ret += gen_err_check(err=errarg) ret += mcgen(''' if (has_%(c_name)s) { ''', @@ -144,7 +133,7 @@ def gen_marshal_input_visit(arg_type, dealloc=False): ''', c_name=c_name(memb.name), name=memb.name, c_type=memb.type.c_name(), errp=errparg) - ret += gen_err_check(errarg) + ret += gen_err_check(err=errarg) if memb.optional: pop_indent() ret += mcgen(''' diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py index b5a9d4f..eaaac05 100644 --- a/scripts/qapi-event.py +++ b/scripts/qapi-event.py @@ -68,12 +68,9 @@ def gen_event_send(name, arg_type): /* Fake visit, as if all members are under a structure */ visit_start_struct(v, NULL, "", "%(name)s", 0, &err); - if (err) { - goto out; - } - ''', name=name) + ret += gen_err_check() for memb in arg_type.members: if memb.optional: @@ -91,14 +88,12 @@ def gen_event_send(name, arg_type): ret += mcgen(''' visit_type_%(c_type)s(v, %(cast)s&%(c_name)s, "%(name)s", &err); - if (err) { - goto out; - } ''', cast=cast, c_name=c_name(memb.name), c_type=memb.type.c_name(), name=memb.name) + ret += gen_err_check() if memb.optional: pop_indent() @@ -107,7 +102,6 @@ def gen_event_send(name, arg_type): ''') ret += mcgen(''' - visit_end_struct(v, &err); if (err) { goto out; diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 78ad556..bc6911f 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -81,11 +81,9 @@ static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s **obj, Error **e if base: ret += mcgen(''' visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err); - if (err) { - goto out; - } ''', c_type=base.c_name(), c_name=c_name('base')) + ret += gen_err_check() for memb in members: if memb.optional: @@ -107,11 +105,7 @@ static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s **obj, Error **e ret += mcgen(''' } ''') - ret += mcgen(''' - if (err) { - goto out; - } -''') + ret += gen_err_check() if re.search('^ *goto out;', ret, re.MULTILINE): ret += mcgen(''' @@ -271,11 +265,9 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error if base: ret += mcgen(''' visit_type_%(c_name)s_fields(v, obj, &err); - if (err) { - goto out_obj; - } ''', c_name=c_name(name)) + ret += gen_err_check(label='out_obj') tag_key = variants.tag_member.name if not variants.tag_name: diff --git a/scripts/qapi.py b/scripts/qapi.py index c0728d7..62a415c 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -1536,6 +1536,18 @@ def gen_params(arg_type, extra): ret += sep + extra return ret + +def gen_err_check(err='err', label='out'): + if not err: + return '' + return mcgen(''' + if (%(err)s) { + goto %(label)s; + } +''', + err=err, label=label) + + # # Common command line parsing # -- cgit v1.1 From 82ca8e469666b169ccf818a0e36136aee97d7db0 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 29 Sep 2015 16:21:14 -0600 Subject: qapi: Share gen_visit_fields() Consolidate the code between visit, command marshalling, and event generation that iterates over the members of a struct. It reduces code duplication in the generator, so that a future patch can reduce the size of generated code while touching only one instead of three locations. There are no changes to the generated marshal code. The visitor code becomes slightly more verbose, but remains semantically equivalent, and is actually easier to read as it follows a more common idiom: | visit_optional(v, &(*obj)->has_device, "device", &err); |- if (!err && (*obj)->has_device) { |- visit_type_str(v, &(*obj)->device, "device", &err); |- } | if (err) { | goto out; | } |+ if ((*obj)->has_device) { |+ visit_type_str(v, &(*obj)->device, "device", &err); |+ if (err) { |+ goto out; |+ } |+ } The event code becomes slightly more verbose, but this is arguably a bug fix: although the visitors are not well documented, use of an optional member should not be attempted unless guarded by a prior call to visit_optional(). Works only because the output qmp visitor has a no-op visit_optional(): |+ visit_optional(v, &has_offset, "offset", &err); |+ if (err) { |+ goto out; |+ } | if (has_offset) { | visit_type_int(v, &offset, "offset", &err); Signed-off-by: Eric Blake Message-Id: <1443565276-4535-17-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- scripts/qapi-commands.py | 27 +-------------------------- scripts/qapi-event.py | 31 +------------------------------ scripts/qapi-visit.py | 22 +--------------------- scripts/qapi.py | 43 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 46 insertions(+), 77 deletions(-) (limited to 'scripts') diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 4e99c1d..9d214a6 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -101,7 +101,6 @@ def gen_marshal_input_visit(arg_type, dealloc=False): return ret if dealloc: - errparg = 'NULL' errarg = None ret += mcgen(''' qmp_input_visitor_cleanup(qiv); @@ -109,36 +108,12 @@ def gen_marshal_input_visit(arg_type, dealloc=False): v = qapi_dealloc_get_visitor(qdv); ''') else: - errparg = '&err' errarg = 'err' ret += mcgen(''' v = qmp_input_get_visitor(qiv); ''') - for memb in arg_type.members: - if memb.optional: - ret += mcgen(''' - visit_optional(v, &has_%(c_name)s, "%(name)s", %(errp)s); -''', - c_name=c_name(memb.name), name=memb.name, - errp=errparg) - ret += gen_err_check(err=errarg) - ret += mcgen(''' - if (has_%(c_name)s) { -''', - c_name=c_name(memb.name)) - push_indent() - ret += mcgen(''' - visit_type_%(c_type)s(v, &%(c_name)s, "%(name)s", %(errp)s); -''', - c_name=c_name(memb.name), name=memb.name, - c_type=memb.type.c_name(), errp=errparg) - ret += gen_err_check(err=errarg) - if memb.optional: - pop_indent() - ret += mcgen(''' - } -''') + ret += gen_visit_fields(arg_type.members, errarg=errarg) if dealloc: ret += mcgen(''' diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py index eaaac05..720486f 100644 --- a/scripts/qapi-event.py +++ b/scripts/qapi-event.py @@ -71,36 +71,7 @@ def gen_event_send(name, arg_type): ''', name=name) ret += gen_err_check() - - for memb in arg_type.members: - if memb.optional: - ret += mcgen(''' - if (has_%(c_name)s) { -''', - c_name=c_name(memb.name)) - push_indent() - - # Ugly: need to cast away the const - if memb.type.name == "str": - cast = '(char **)' - else: - cast = '' - - ret += mcgen(''' - visit_type_%(c_type)s(v, %(cast)s&%(c_name)s, "%(name)s", &err); -''', - cast=cast, - c_name=c_name(memb.name), - c_type=memb.type.c_name(), - name=memb.name) - ret += gen_err_check() - - if memb.optional: - pop_indent() - ret += mcgen(''' - } -''') - + ret += gen_visit_fields(arg_type.members, need_cast=True) ret += mcgen(''' visit_end_struct(v, &err); if (err) { diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index bc6911f..4f97781 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -85,27 +85,7 @@ static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s **obj, Error **e c_type=base.c_name(), c_name=c_name('base')) ret += gen_err_check() - for memb in members: - if memb.optional: - ret += mcgen(''' - visit_optional(v, &(*obj)->has_%(c_name)s, "%(name)s", &err); - if (!err && (*obj)->has_%(c_name)s) { -''', - c_name=c_name(memb.name), name=memb.name) - push_indent() - - ret += mcgen(''' - visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err); -''', - c_type=memb.type.c_name(), c_name=c_name(memb.name), - name=memb.name) - - if memb.optional: - pop_indent() - ret += mcgen(''' - } -''') - ret += gen_err_check() + ret += gen_visit_fields(members, prefix='(*obj)->') if re.search('^ *goto out;', ret, re.MULTILINE): ret += mcgen(''' diff --git a/scripts/qapi.py b/scripts/qapi.py index 62a415c..ada6380 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -1548,6 +1548,49 @@ def gen_err_check(err='err', label='out'): err=err, label=label) +def gen_visit_fields(members, prefix='', need_cast=False, errarg='err'): + ret = '' + if errarg: + errparg = '&' + errarg + else: + errparg = 'NULL' + + for memb in members: + if memb.optional: + ret += mcgen(''' + visit_optional(v, &%(prefix)shas_%(c_name)s, "%(name)s", %(errp)s); +''', + prefix=prefix, c_name=c_name(memb.name), + name=memb.name, errp=errparg) + ret += gen_err_check(err=errarg) + ret += mcgen(''' + if (%(prefix)shas_%(c_name)s) { +''', + prefix=prefix, c_name=c_name(memb.name)) + push_indent() + + # Ugly: sometimes we need to cast away const + if need_cast and memb.type.name == 'str': + cast = '(char **)' + else: + cast = '' + + ret += mcgen(''' + visit_type_%(c_type)s(v, %(cast)s&%(prefix)s%(c_name)s, "%(name)s", %(errp)s); +''', + c_type=memb.type.c_name(), prefix=prefix, cast=cast, + c_name=c_name(memb.name), name=memb.name, + errp=errparg) + ret += gen_err_check(err=errarg) + + if memb.optional: + pop_indent() + ret += mcgen(''' + } +''') + return ret + + # # Common command line parsing # -- cgit v1.1 From 18bdbc3ac8b477e160d56aa6ecd6942495ce44d0 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 29 Sep 2015 16:21:15 -0600 Subject: qapi: Simplify gen_visit_fields() error handling Since we have consolidated all generated code to use 'err' as the name of the local variable for error detection, we can simplify the decision on whether to skip error detection (useful for deallocation paths) to be a boolean. Signed-off-by: Eric Blake Message-Id: <1443565276-4535-18-git-send-email-eblake@redhat.com> [Change to gen_visit_fields() simplified] Signed-off-by: Markus Armbruster --- scripts/qapi-commands.py | 4 +--- scripts/qapi.py | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 13 deletions(-) (limited to 'scripts') diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 9d214a6..43a893b 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -101,19 +101,17 @@ def gen_marshal_input_visit(arg_type, dealloc=False): return ret if dealloc: - errarg = None ret += mcgen(''' qmp_input_visitor_cleanup(qiv); qdv = qapi_dealloc_visitor_new(); v = qapi_dealloc_get_visitor(qdv); ''') else: - errarg = 'err' ret += mcgen(''' v = qmp_input_get_visitor(qiv); ''') - ret += gen_visit_fields(arg_type.members, errarg=errarg) + ret += gen_visit_fields(arg_type.members, skiperr=dealloc) if dealloc: ret += mcgen(''' diff --git a/scripts/qapi.py b/scripts/qapi.py index ada6380..26cff3f 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -1537,23 +1537,23 @@ def gen_params(arg_type, extra): return ret -def gen_err_check(err='err', label='out'): - if not err: +def gen_err_check(label='out', skiperr=False): + if skiperr: return '' return mcgen(''' - if (%(err)s) { + if (err) { goto %(label)s; } ''', - err=err, label=label) + label=label) -def gen_visit_fields(members, prefix='', need_cast=False, errarg='err'): +def gen_visit_fields(members, prefix='', need_cast=False, skiperr=False): ret = '' - if errarg: - errparg = '&' + errarg - else: + if skiperr: errparg = 'NULL' + else: + errparg = '&err' for memb in members: if memb.optional: @@ -1562,7 +1562,7 @@ def gen_visit_fields(members, prefix='', need_cast=False, errarg='err'): ''', prefix=prefix, c_name=c_name(memb.name), name=memb.name, errp=errparg) - ret += gen_err_check(err=errarg) + ret += gen_err_check(skiperr=skiperr) ret += mcgen(''' if (%(prefix)shas_%(c_name)s) { ''', @@ -1581,7 +1581,7 @@ def gen_visit_fields(members, prefix='', need_cast=False, errarg='err'): c_type=memb.type.c_name(), prefix=prefix, cast=cast, c_name=c_name(memb.name), name=memb.name, errp=errparg) - ret += gen_err_check(err=errarg) + ret += gen_err_check(skiperr=skiperr) if memb.optional: pop_indent() -- cgit v1.1