[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v4 16/16] qapi: Prefer '"str" + var' over '"str%s" %
|
From: |
Eric Blake |
|
Subject: |
[Qemu-devel] [PATCH v4 16/16] qapi: Prefer '"str" + var' over '"str%s" % var' |
|
Date: |
Thu, 14 May 2015 06:51:02 -0600 |
Use of python's % operator to format strings is fine if there are
multiple strings or if there is integer formatting going on, but
when it is just abused for string concatenation, it looks nicer
to just use the + operator. This is particularly true when the
value being substituted is at the front of the format string,
rather than the tail.
Update an error message (and testsuite coverage) while at it, since
concatenating a non-string object does not always produce legible
results.
Signed-off-by: Eric Blake <address@hidden>
---
v4: new patch
---
scripts/qapi-commands.py | 17 +++++----
scripts/qapi-event.py | 2 +-
scripts/qapi-types.py | 10 +++---
scripts/qapi-visit.py | 4 +--
scripts/qapi.py | 64 +++++++++++++++++-----------------
tests/qapi-schema/include-non-file.err | 2 +-
6 files changed, 51 insertions(+), 48 deletions(-)
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 0a1d636..1464a3d 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -25,7 +25,7 @@ def generate_command_decl(name, args, ret_type):
for argname, argtype, optional in parse_args(args):
argtype = c_type(argtype, is_param=True)
if optional:
- arglist += "bool has_%s, " % c_name(argname)
+ arglist += "bool has_" + c_name(argname) + ", "
arglist += "%s %s, " % (argtype, c_name(argname))
return mcgen('''
%(ret_type)s qmp_%(name)s(%(args)sError **errp);
@@ -50,8 +50,8 @@ def gen_sync_call(name, args, ret_type, indent=0):
retval = "retval = "
for argname, argtype, optional in parse_args(args):
if optional:
- arglist += "has_%s, " % c_name(argname)
- arglist += "%s, " % (c_name(argname))
+ arglist += "has_" + c_name(argname) + ", "
+ arglist += c_name(argname) + ", "
push_indent(indent)
ret = mcgen('''
%(retval)sqmp_%(name)s(%(args)s&local_err);
@@ -71,7 +71,7 @@ def gen_sync_call(name, args, ret_type, indent=0):
def gen_marshal_output_call(name, ret_type):
if not ret_type:
return ""
- return "qmp_marshal_output_%s(retval, ret, &local_err);" % c_name(name)
+ return "qmp_marshal_output_" + c_name(name) + "(retval, ret, &local_err);"
def gen_visitor_input_containers_decl(args, obj):
ret = ""
@@ -200,9 +200,11 @@ out:
def gen_marshal_input_decl(name, args, ret_type, middle_mode):
if middle_mode:
- return 'int qmp_marshal_input_%s(Monitor *mon, const QDict *qdict,
QObject **ret)' % c_name(name)
+ return ('int qmp_marshal_input_' + c_name(name)
+ + '(Monitor *mon, const QDict *qdict, QObject **ret)')
else:
- return 'static void qmp_marshal_input_%s(QDict *args, QObject **ret,
Error **errp)' % c_name(name)
+ return ('static void qmp_marshal_input_' + c_name(name)
+ + '(QDict *args, QObject **ret, Error **errp)')
@@ -458,7 +460,8 @@ if dispatch_type == "sync":
fdef.write(ret)
if middle_mode:
- fdecl.write('%s;\n' % gen_marshal_input_decl(cmd['command'],
arglist, ret_type, middle_mode))
+ fdecl.write(gen_marshal_input_decl(cmd['command'], arglist,
+ ret_type, middle_mode) + ';\n')
ret = gen_marshal_input(cmd['command'], arglist, ret_type,
middle_mode) + "\n"
fdef.write(ret)
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index a7e0033..957dba5 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -17,7 +17,7 @@ import getopt
import errno
def _generate_event_api_name(event_name, params):
- api_name = "void qapi_event_send_%s(" % c_name(event_name).lower();
+ api_name = "void qapi_event_send_" + c_name(event_name).lower() + "(";
l = len(api_name)
if params:
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 5665145..1c41743 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -204,7 +204,7 @@ def generate_union(expr, meta):
if enum_define:
discriminator_type_name = enum_define['enum_name']
else:
- discriminator_type_name = '%sKind' % (name)
+ discriminator_type_name = name + 'Kind'
ret = mcgen('''
struct %(name)s
@@ -399,13 +399,13 @@ for expr in exprs:
ret += generate_fwd_struct(expr['union'], expr['data']) + "\n"
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'],
+ ret += generate_enum(expr['union'] + 'Kind', expr['data'].keys())
+ fdef.write(generate_enum_lookup(expr['union'] + 'Kind',
expr['data'].keys()))
elif expr.has_key('alternate'):
ret += generate_fwd_struct(expr['alternate'], expr['data']) + "\n"
- ret += generate_enum('%sKind' % expr['alternate'], expr['data'].keys())
- fdef.write(generate_enum_lookup('%sKind' % expr['alternate'],
+ ret += generate_enum(expr['alternate'] + 'Kind', expr['data'].keys())
+ fdef.write(generate_enum_lookup(expr['alternate'] + 'Kind',
expr['data'].keys()))
fdef.write(generate_alternate_qtypes(expr))
else:
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index e511be3..4287251 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -509,7 +509,7 @@ for expr in exprs:
enum_define = discriminator_find_enum_define(expr)
ret = ""
if not enum_define:
- ret = generate_decl_enum('%sKind' % expr['union'],
+ ret = generate_decl_enum(expr['union'] + 'Kind',
expr['data'].keys())
ret += generate_declaration(expr['union'], expr['data'])
fdecl.write(ret)
@@ -518,7 +518,7 @@ for expr in exprs:
ret += generate_visit_list(expr['alternate'], expr['data'])
fdef.write(ret)
- ret = generate_decl_enum('%sKind' % expr['alternate'],
+ ret = generate_decl_enum(expr['alternate'] + 'Kind',
expr['data'].keys())
ret += generate_declaration(expr['alternate'], expr['data'])
fdecl.write(ret)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 309dfec..1a11f61 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -122,21 +122,22 @@ class QAPISchema:
self.accept()
while self.tok != None:
- expr_info = {'file': input_relname, 'line': self.line, 'parent':
self.parent_info}
+ expr_info = {'file': input_relname, 'line': self.line,
+ 'parent': self.parent_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,
- 'Expected a file name (string), got:
%s'
- % include)
+ "Expected a string for 'include'
value")
include_path = os.path.join(self.input_dir, include)
for elem in self.include_hist:
if include_path == elem[1]:
- raise QAPIExprError(expr_info, "Inclusion loop for %s"
- % include)
+ raise QAPIExprError(expr_info,
+ "Inclusion loop for " + include)
# skip multiple include of the same file
if include_path in previously_included:
continue
@@ -208,7 +209,7 @@ class QAPISchema:
string += ch
else:
raise QAPISchemaError(self,
- "Unknown escape \\%s" %ch)
+ "Unknown escape \\" + ch)
esc = False
elif ch == "\\":
esc = True
@@ -254,7 +255,7 @@ class QAPISchema:
raise QAPISchemaError(self, 'Expected ":"')
self.accept()
if key in expr:
- raise QAPISchemaError(self, 'Duplicate key "%s"' % key)
+ raise QAPISchemaError(self, 'Duplicate key "' + key + '"')
expr[key] = self.get_expr(True)
if self.tok == '}':
self.accept()
@@ -343,7 +344,7 @@ def check_name(expr_info, source, name, allow_optional =
False,
if not isinstance(name, str):
raise QAPIExprError(expr_info,
- "%s requires a string name" % source)
+ source + " requires a string name")
if name.startswith('*'):
membername = name[1:]
if not allow_optional:
@@ -374,13 +375,12 @@ def check_type(expr_info, source, value, allow_array =
False,
if isinstance(value, list):
if not allow_array:
raise QAPIExprError(expr_info,
- "%s cannot be an array" % source)
+ source + " cannot be an array")
if len(value) != 1 or not isinstance(value[0], str):
- raise QAPIExprError(expr_info,
- "%s: array type must contain single type name"
- % source)
+ raise QAPIExprError(expr_info, source +
+ ": array type must contain single type name")
value = value[0]
- orig_value = "array of %s" %value
+ orig_value = "array of " + value
# Check if type name for value is okay
if isinstance(value, str):
@@ -401,12 +401,12 @@ def check_type(expr_info, source, value, allow_array =
False,
# value is a dictionary, check that each member is okay
if not isinstance(value, OrderedDict):
raise QAPIExprError(expr_info,
- "%s should be a dictionary" % source)
+ source + " should be a dictionary")
if not allow_dict:
raise QAPIExprError(expr_info,
- "%s should be a type name" % source)
+ source + " should be a type name")
for (key, arg) in value.items():
- check_name(expr_info, "Member of %s" % source, key,
+ check_name(expr_info, "Member of " + source, key,
allow_optional=allow_optional)
# Todo: allow dictionaries to represent default values of
# an optional argument.
@@ -433,13 +433,13 @@ def check_command(expr, expr_info):
name = expr['command']
allow_star = expr.has_key('gen')
- check_type(expr_info, "'data' for command '%s'" % name,
+ check_type(expr_info, "'data' for command '" + name + "'",
expr.get('data'), allow_dict=True, allow_optional=True,
allow_metas=['union', 'struct'], allow_star=allow_star)
returns_meta = ['union', 'struct']
if name in returns_whitelist:
returns_meta += ['built-in', 'alternate', 'enum']
- check_type(expr_info, "'returns' for command '%s'" % name,
+ check_type(expr_info, "'returns' for command '" + name + "'",
expr.get('returns'), allow_array=True, allow_dict=True,
allow_optional=True, allow_metas=returns_meta,
allow_star=allow_star)
@@ -452,7 +452,7 @@ def check_event(expr, expr_info):
if name.upper() == 'MAX':
raise QAPIExprError(expr_info, "Event name 'MAX' cannot be created")
events.append(name)
- check_type(expr_info, "'data' for event '%s'" % name,
+ check_type(expr_info, "'data' for event '" + name + "'",
expr.get('data'), allow_dict=True, allow_optional=True,
allow_metas=['union', 'struct'])
@@ -469,7 +469,7 @@ def check_union(expr, expr_info):
if discriminator is None:
raise QAPIExprError(expr_info,
"Union '%s' requires a discriminator to go "
- "along with base" %name)
+ "along with base" % name)
# Two types of unions, determined by discriminator.
@@ -497,7 +497,7 @@ def check_union(expr, expr_info):
# The value of member 'discriminator' must name a non-optional
# member of the base struct.
- check_name(expr_info, "Discriminator of flat union '%s'" % name,
+ check_name(expr_info, "Discriminator of flat union '" + name + "'",
discriminator)
discriminator_type = base_fields.get(discriminator)
if not discriminator_type:
@@ -515,7 +515,7 @@ def check_union(expr, expr_info):
# Check every branch
for (key, value) in members.items():
- check_name(expr_info, "Member of union '%s'" % name, key)
+ check_name(expr_info, "Member of union '" + name + "'", key)
# Each value must name a known type; furthermore, in flat unions,
# branches must be a struct with no overlapping member names
@@ -525,7 +525,7 @@ def check_union(expr, expr_info):
branch_struct = find_struct(value)
assert branch_struct
check_member_clash(expr_info, base, branch_struct['data'],
- " of branch '%s'" % key)
+ " of branch '" + key + "'")
# If the discriminator names an enum type, then all members
# of 'data' must also be members of the enum type.
@@ -533,8 +533,8 @@ def check_union(expr, expr_info):
if 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"]))
+ "enum '%s'"
+ % (key, enum_define["enum_name"]))
# Otherwise, check for conflicts in the generated enum
else:
@@ -585,7 +585,7 @@ def check_enum(expr, expr_info):
raise QAPIExprError(expr_info,
"Enum '%s' requires an array for 'data'" % name)
for member in members:
- check_name(expr_info, "Member of enum '%s'" %name, member,
+ check_name(expr_info, "Member of enum '" + name + "'", member,
enum_member=True)
key = camel_to_upper(member)
if key in values:
@@ -598,9 +598,9 @@ def check_struct(expr, expr_info):
name = expr['struct']
members = expr['data']
- check_type(expr_info, "'data' for struct '%s'" % name, members,
+ check_type(expr_info, "'data' for struct '" + name + "'", members,
allow_dict=True, allow_optional=True)
- check_type(expr_info, "'base' for struct '%s'" % name, expr.get('base'),
+ check_type(expr_info, "'base' for struct '" + name + "'", expr.get('base'),
allow_metas=['struct'])
if expr.get('base'):
check_member_clash(expr_info, expr['base'], expr['data'])
@@ -698,10 +698,10 @@ def parse_schema(input_file):
expr = expr_elem['expr']
if expr.has_key('union'):
if not discriminator_find_enum_define(expr):
- add_enum('%sKind' % expr['union'], expr_elem['info'],
+ add_enum(expr['union'] + 'Kind', expr_elem['info'],
implicit=True)
elif expr.has_key('alternate'):
- add_enum('%sKind' % expr['alternate'], expr_elem['info'],
+ add_enum(expr['alternate'] + 'Kind', expr_elem['info'],
implicit=True)
# Final pass - validate that exprs make sense
@@ -831,7 +831,7 @@ def type_name(value):
def add_name(name, info, meta, implicit = False):
global all_names
- check_name(info, "'%s'" % meta, name)
+ check_name(info, "'" + meta + "'", name)
if name in all_names:
raise QAPIExprError(info,
"%s '%s' is already defined"
diff --git a/tests/qapi-schema/include-non-file.err
b/tests/qapi-schema/include-non-file.err
index 9658c78..da4e257 100644
--- a/tests/qapi-schema/include-non-file.err
+++ b/tests/qapi-schema/include-non-file.err
@@ -1 +1 @@
-tests/qapi-schema/include-non-file.json:1: Expected a file name (string), got:
['foo', 'bar']
+tests/qapi-schema/include-non-file.json:1: Expected a string for 'include'
value
--
2.1.0
- [Qemu-devel] [PATCH v4 00/16] Fix qapi mangling of downstream names, Eric Blake, 2015/05/14
- [Qemu-devel] [PATCH v4 02/16] qapi: Rename identical c_fun()/c_var() into c_name(), Eric Blake, 2015/05/14
- [Qemu-devel] [PATCH v4 05/16] qapi: Simplify c_enum_const(), Eric Blake, 2015/05/14
- [Qemu-devel] [PATCH v4 10/16] qapi: Support downstream enums, Eric Blake, 2015/05/14
- [Qemu-devel] [PATCH v4 12/16] qapi: Support downstream simple unions, Eric Blake, 2015/05/14
- [Qemu-devel] [PATCH v4 14/16] qapi: Support downstream alternates, Eric Blake, 2015/05/14
- [Qemu-devel] [PATCH v4 11/16] qapi: Support downstream structs, Eric Blake, 2015/05/14
- [Qemu-devel] [PATCH v4 15/16] qapi: Support downstream events and commands, Eric Blake, 2015/05/14
- [Qemu-devel] [PATCH v4 16/16] qapi: Prefer '"str" + var' over '"str%s" % var',
Eric Blake <=
- [Qemu-devel] [PATCH v4 13/16] qapi: Support downstream flat unions, Eric Blake, 2015/05/14
- [Qemu-devel] [PATCH v4 03/16] qapi: Rename _generate_enum_string() to camel_to_upper(), Eric Blake, 2015/05/14
- [Qemu-devel] [PATCH v4 04/16] qapi: Rename generate_enum_full_value() to c_enum_const(), Eric Blake, 2015/05/14
- [Qemu-devel] [PATCH v4 01/16] qapi: Fix C identifiers generated for names containing '.', Eric Blake, 2015/05/14
- [Qemu-devel] [PATCH v4 09/16] qapi: Make c_type() consistently convert qapi names, Eric Blake, 2015/05/14
- [Qemu-devel] [PATCH v4 08/16] qapi: Tidy c_type logic, Eric Blake, 2015/05/14