[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v6 25/36] qapi: Require valid names
From: |
Eric Blake |
Subject: |
[Qemu-devel] [PATCH v6 25/36] qapi: Require valid names |
Date: |
Sat, 4 Apr 2015 22:07:56 -0600 |
Previous commits demonstrated that the generator overlooked various
bad naming situations:
- types, commands, and events need a valid name
- enum members must be valid names, when combined with prefix
- union and alternate branches cannot be marked optional
The set of valid names includes [a-zA-Z_][a-zA-Z0-9._-]* (where
'.' is useful only in downstream extensions). Since we have
existing enum values beginning with a digit (see QKeyCode), a
small hack is used to pass the same regex.
Signed-off-by: Eric Blake <address@hidden>
---
v6: rebase onto earlier changes; use regex instead of sets, and
ensure leading letter or _; don't force event case; drop dead
code for exempting '**'; extend coverage to enum members
---
scripts/qapi.py | 63 ++++++++++++++++------
tests/qapi-schema/bad-ident.err | 1 +
tests/qapi-schema/bad-ident.exit | 2 +-
tests/qapi-schema/bad-ident.json | 2 +-
tests/qapi-schema/bad-ident.out | 3 --
tests/qapi-schema/enum-bad-name.err | 1 +
tests/qapi-schema/enum-bad-name.exit | 2 +-
tests/qapi-schema/enum-bad-name.json | 2 +-
tests/qapi-schema/enum-bad-name.out | 3 --
tests/qapi-schema/enum-dict-member.err | 2 +-
tests/qapi-schema/flat-union-bad-discriminator.err | 2 +-
.../flat-union-optional-discriminator.err | 1 +
.../flat-union-optional-discriminator.exit | 2 +-
.../flat-union-optional-discriminator.json | 2 +-
.../flat-union-optional-discriminator.out | 7 ---
tests/qapi-schema/union-optional-branch.err | 1 +
tests/qapi-schema/union-optional-branch.exit | 2 +-
tests/qapi-schema/union-optional-branch.json | 2 +-
tests/qapi-schema/union-optional-branch.out | 3 --
19 files changed, 60 insertions(+), 43 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 9f64a0d..9b97683 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -276,8 +276,31 @@ def discriminator_find_enum_define(expr):
return find_enum(discriminator_type)
+valid_name = re.compile('^[a-zA-Z_][a-zA-Z0-9_.-]*$')
+def check_name(expr_info, source, name, allow_optional = False,
+ enum_member = False):
+ global valid_name
+ membername = name
+
+ if not isinstance(name, str):
+ raise QAPIExprError(expr_info,
+ "%s requires a string name" % source)
+ if name.startswith('*'):
+ membername = name[1:]
+ if not allow_optional:
+ raise QAPIExprError(expr_info,
+ "%s does not allow optional name '%s'"
+ % (source, name))
+ # Enum members can start with a digit, because the generated C
+ # code always prefixes it with the enum name
+ if enum_member:
+ membername = "_%s" %membername
+ if not valid_name.match(membername):
+ raise QAPIExprError(expr_info,
+ "%s uses invalid name '%s'" % (source, name))
+
def check_type(expr_info, source, value, allow_array = False,
- allow_dict = False, allow_metas = []):
+ allow_dict = False, allow_optional = False, allow_metas = []):
global all_names
orig_value = value
@@ -319,18 +342,21 @@ def check_type(expr_info, source, value, allow_array =
False,
raise QAPIExprError(expr_info,
"%s should be a type name" % source)
for (key, arg) in value.items():
+ check_name(expr_info, "Member of %s" % source, key,
+ allow_optional=allow_optional)
check_type(expr_info, "Member '%s' of %s" % (key, source), arg,
- allow_array=True, allow_dict=True,
+ allow_array=True, allow_dict=True, allow_optional=True,
allow_metas=['built-in', 'union', 'alternate', 'struct',
'enum'])
def check_command(expr, expr_info):
name = expr['command']
check_type(expr_info, "'data' for command '%s'" % name,
- expr.get('data'), allow_dict=True,
+ expr.get('data'), allow_dict=True, allow_optional=True,
allow_metas=['union', 'struct'])
check_type(expr_info, "'returns' for command '%s'" % name,
expr.get('returns'), allow_array=True, allow_dict=True,
+ allow_optional=True,
allow_metas=['built-in', 'union', 'alternate', 'struct',
'enum'])
@@ -343,7 +369,7 @@ def check_event(expr, expr_info):
raise QAPIExprError(expr_info, "Event name 'MAX' cannot be created")
events.append(name)
check_type(expr_info, "'data' for event '%s'" % name,
- expr.get('data'), allow_dict=True,
+ expr.get('data'), allow_dict=True, allow_optional=True,
allow_metas=['union', 'struct'])
if params:
for argname, argentry, optional, structured in parse_args(params):
@@ -392,12 +418,10 @@ def check_union(expr, expr_info):
"Base '%s' is not a valid type"
% base)
- # The value of member 'discriminator' must name a member of the
- # base type.
- if not isinstance(discriminator, str):
- raise QAPIExprError(expr_info,
- "Flat union '%s' discriminator must be a
string"
- % name)
+ # The value of member 'discriminator' must name a non-optional
+ # member of the base type.
+ check_name(expr_info, "Discriminator of flat union '%s'" % name,
+ discriminator)
discriminator_type = base_fields.get(discriminator)
if not discriminator_type:
raise QAPIExprError(expr_info,
@@ -414,6 +438,8 @@ 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)
+
# Each value must name a known type; futhermore, in flat unions,
# branches must be a struct
check_type(expr_info, "Member '%s' of union '%s'" % (key, name),
@@ -445,6 +471,8 @@ def check_alternate(expr, expr_info):
# Check every branch
for (key, value) in members.items():
+ check_name(expr_info, "Member of alternate '%s'" % name, key)
+
# Check for conflicts in the generated enum
c_key = _generate_enum_string(key)
if c_key in values:
@@ -475,10 +503,8 @@ def check_enum(expr, expr_info):
raise QAPIExprError(expr_info,
"Enum '%s' requires an array for 'data'" % name)
for member in members:
- if not isinstance(member, str):
- raise QAPIExprError(expr_info,
- "Enum '%s' member '%s' is not a string"
- % (name, member))
+ check_name(expr_info, "Member of enum '%s'" %name, member,
+ enum_member=True)
key = _generate_enum_string(member)
if key in values:
raise QAPIExprError(expr_info,
@@ -491,7 +517,7 @@ def check_struct(expr, expr_info):
members = expr['data']
check_type(expr_info, "'data' for type '%s'" % name, members,
- allow_dict=True)
+ allow_dict=True, allow_optional=True)
check_type(expr_info, "'base' for type '%s'" % name, expr.get('base'),
allow_metas=['struct'])
@@ -682,8 +708,11 @@ def type_name(name):
return c_list_type(name[0])
return name
-def add_name(name, info, meta, implicit = False):
+def add_name(name, info, meta, implicit = False, source = None):
global all_names
+ if not source:
+ source = "'%s'" % meta
+ check_name(info, source, name)
if name in all_names:
raise QAPIExprError(info,
"%s '%s' is already defined"
@@ -697,7 +726,7 @@ def add_name(name, info, meta, implicit = False):
def add_struct(definition, info):
global struct_types
name = definition['type']
- add_name(name, info, 'struct')
+ add_name(name, info, 'struct', source="'type'")
struct_types.append(definition)
def find_struct(name):
diff --git a/tests/qapi-schema/bad-ident.err b/tests/qapi-schema/bad-ident.err
index e69de29..42b490c 100644
--- a/tests/qapi-schema/bad-ident.err
+++ b/tests/qapi-schema/bad-ident.err
@@ -0,0 +1 @@
+tests/qapi-schema/bad-ident.json:2: 'type' does not allow optional name '*oops'
diff --git a/tests/qapi-schema/bad-ident.exit b/tests/qapi-schema/bad-ident.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/bad-ident.exit
+++ b/tests/qapi-schema/bad-ident.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/bad-ident.json b/tests/qapi-schema/bad-ident.json
index f139110..da949e8 100644
--- a/tests/qapi-schema/bad-ident.json
+++ b/tests/qapi-schema/bad-ident.json
@@ -1,2 +1,2 @@
-# FIXME: we should reject creating a type name with bad name
+# we reject creating a type name with bad name
{ 'type': '*oops', 'data': { 'i': 'int' } }
diff --git a/tests/qapi-schema/bad-ident.out b/tests/qapi-schema/bad-ident.out
index 165e346..e69de29 100644
--- a/tests/qapi-schema/bad-ident.out
+++ b/tests/qapi-schema/bad-ident.out
@@ -1,3 +0,0 @@
-[OrderedDict([('type', '*oops'), ('data', OrderedDict([('i', 'int')]))])]
-[]
-[OrderedDict([('type', '*oops'), ('data', OrderedDict([('i', 'int')]))])]
diff --git a/tests/qapi-schema/enum-bad-name.err
b/tests/qapi-schema/enum-bad-name.err
index e69de29..9c3c100 100644
--- a/tests/qapi-schema/enum-bad-name.err
+++ b/tests/qapi-schema/enum-bad-name.err
@@ -0,0 +1 @@
+tests/qapi-schema/enum-bad-name.json:2: Member of enum 'MyEnum' uses invalid
name 'not^possible'
diff --git a/tests/qapi-schema/enum-bad-name.exit
b/tests/qapi-schema/enum-bad-name.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/enum-bad-name.exit
+++ b/tests/qapi-schema/enum-bad-name.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/enum-bad-name.json
b/tests/qapi-schema/enum-bad-name.json
index 0c32448..8506562 100644
--- a/tests/qapi-schema/enum-bad-name.json
+++ b/tests/qapi-schema/enum-bad-name.json
@@ -1,2 +1,2 @@
-# FIXME: we should ensure all enum names can map to C
+# we ensure all enum names can map to C
{ 'enum': 'MyEnum', 'data': [ 'not^possible' ] }
diff --git a/tests/qapi-schema/enum-bad-name.out
b/tests/qapi-schema/enum-bad-name.out
index d24ea49..e69de29 100644
--- a/tests/qapi-schema/enum-bad-name.out
+++ b/tests/qapi-schema/enum-bad-name.out
@@ -1,3 +0,0 @@
-[OrderedDict([('enum', 'MyEnum'), ('data', ['not^possible'])])]
-[{'enum_name': 'MyEnum', 'enum_values': ['not^possible']}]
-[]
diff --git a/tests/qapi-schema/enum-dict-member.err
b/tests/qapi-schema/enum-dict-member.err
index 7e966a8..8ca146e 100644
--- a/tests/qapi-schema/enum-dict-member.err
+++ b/tests/qapi-schema/enum-dict-member.err
@@ -1 +1 @@
-tests/qapi-schema/enum-dict-member.json:2: Enum 'MyEnum' member
'OrderedDict([('value', 'str')])' is not a string
+tests/qapi-schema/enum-dict-member.json:2: Member of enum 'MyEnum' requires a
string name
diff --git a/tests/qapi-schema/flat-union-bad-discriminator.err
b/tests/qapi-schema/flat-union-bad-discriminator.err
index 507e2ba..c38cc8e 100644
--- a/tests/qapi-schema/flat-union-bad-discriminator.err
+++ b/tests/qapi-schema/flat-union-bad-discriminator.err
@@ -1 +1 @@
-tests/qapi-schema/flat-union-bad-discriminator.json:11: Flat union 'TestUnion'
discriminator must be a string
+tests/qapi-schema/flat-union-bad-discriminator.json:11: Discriminator of flat
union 'TestUnion' requires a string name
diff --git a/tests/qapi-schema/flat-union-optional-discriminator.err
b/tests/qapi-schema/flat-union-optional-discriminator.err
index e69de29..aaabedb 100644
--- a/tests/qapi-schema/flat-union-optional-discriminator.err
+++ b/tests/qapi-schema/flat-union-optional-discriminator.err
@@ -0,0 +1 @@
+tests/qapi-schema/flat-union-optional-discriminator.json:6: Discriminator of
flat union 'MyUnion' does not allow optional name '*switch'
diff --git a/tests/qapi-schema/flat-union-optional-discriminator.exit
b/tests/qapi-schema/flat-union-optional-discriminator.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/flat-union-optional-discriminator.exit
+++ b/tests/qapi-schema/flat-union-optional-discriminator.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/flat-union-optional-discriminator.json
b/tests/qapi-schema/flat-union-optional-discriminator.json
index ece0d31..25ce0e6 100644
--- a/tests/qapi-schema/flat-union-optional-discriminator.json
+++ b/tests/qapi-schema/flat-union-optional-discriminator.json
@@ -1,4 +1,4 @@
-# FIXME: we should require the discriminator to be non-optional
+# we require the discriminator to be non-optional
{ 'enum': 'Enum', 'data': [ 'one', 'two' ] }
{ 'type': 'Base',
'data': { '*switch': 'Enum' } }
diff --git a/tests/qapi-schema/flat-union-optional-discriminator.out
b/tests/qapi-schema/flat-union-optional-discriminator.out
index bb7db00..e69de29 100644
--- a/tests/qapi-schema/flat-union-optional-discriminator.out
+++ b/tests/qapi-schema/flat-union-optional-discriminator.out
@@ -1,7 +0,0 @@
-[OrderedDict([('enum', 'Enum'), ('data', ['one', 'two'])]),
- OrderedDict([('type', 'Base'), ('data', OrderedDict([('*switch', 'Enum')]))]),
- OrderedDict([('type', 'Branch'), ('data', OrderedDict([('name', 'str')]))]),
- OrderedDict([('union', 'MyUnion'), ('base', 'Base'), ('discriminator',
'*switch'), ('data', OrderedDict([('one', 'Branch'), ('two', 'Branch')]))])]
-[{'enum_name': 'Enum', 'enum_values': ['one', 'two']}]
-[OrderedDict([('type', 'Base'), ('data', OrderedDict([('*switch', 'Enum')]))]),
- OrderedDict([('type', 'Branch'), ('data', OrderedDict([('name', 'str')]))])]
diff --git a/tests/qapi-schema/union-optional-branch.err
b/tests/qapi-schema/union-optional-branch.err
index e69de29..3ada133 100644
--- a/tests/qapi-schema/union-optional-branch.err
+++ b/tests/qapi-schema/union-optional-branch.err
@@ -0,0 +1 @@
+tests/qapi-schema/union-optional-branch.json:2: Member of union 'Union' does
not allow optional name '*a'
diff --git a/tests/qapi-schema/union-optional-branch.exit
b/tests/qapi-schema/union-optional-branch.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/union-optional-branch.exit
+++ b/tests/qapi-schema/union-optional-branch.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/union-optional-branch.json
b/tests/qapi-schema/union-optional-branch.json
index c513db7..591615f 100644
--- a/tests/qapi-schema/union-optional-branch.json
+++ b/tests/qapi-schema/union-optional-branch.json
@@ -1,2 +1,2 @@
-# FIXME: union branches cannot be optional
+# union branches cannot be optional
{ 'union': 'Union', 'data': { '*a': 'int', 'b': 'str' } }
diff --git a/tests/qapi-schema/union-optional-branch.out
b/tests/qapi-schema/union-optional-branch.out
index b03b5d2..e69de29 100644
--- a/tests/qapi-schema/union-optional-branch.out
+++ b/tests/qapi-schema/union-optional-branch.out
@@ -1,3 +0,0 @@
-[OrderedDict([('union', 'Union'), ('data', OrderedDict([('*a', 'int'), ('b',
'str')]))])]
-[{'enum_name': 'UnionKind', 'enum_values': None}]
-[]
--
2.1.0
- [Qemu-devel] [PATCH v6 22/36] qapi: Unify type bypass and add tests, (continued)
- [Qemu-devel] [PATCH v6 22/36] qapi: Unify type bypass and add tests, Eric Blake, 2015/04/05
- [Qemu-devel] [PATCH v6 18/36] qapi: Better error messages for bad expressions, Eric Blake, 2015/04/05
- [Qemu-devel] [PATCH v6 16/36] qapi: Use 'alternate' to replace anonymous union, Eric Blake, 2015/04/05
- [Qemu-devel] [PATCH v6 20/36] qapi: Better error messages for duplicated expressions, Eric Blake, 2015/04/05
- [Qemu-devel] [PATCH v6 08/36] qapi: Add some union tests, Eric Blake, 2015/04/05
- [Qemu-devel] [PATCH v6 26/36] qapi: Whitelist commands that don't return dictionary, Eric Blake, 2015/04/05
- [Qemu-devel] [PATCH v6 27/36] qapi: More rigorous checking for type safety bypass, Eric Blake, 2015/04/05
- [Qemu-devel] [PATCH v6 25/36] qapi: Require valid names,
Eric Blake <=
- [Qemu-devel] [PATCH v6 24/36] qapi: More rigourous checking of types, Eric Blake, 2015/04/05
- [Qemu-devel] [PATCH v6 23/36] qapi: Add some type check tests, Eric Blake, 2015/04/05
- [Qemu-devel] [PATCH v6 28/36] qapi: Prefer 'struct' over 'type' in generator, Eric Blake, 2015/04/05
- [Qemu-devel] [PATCH v6 29/36] qapi: Document 'struct' metatype, Eric Blake, 2015/04/05
- [Qemu-devel] [PATCH v6 31/36] qapi: Merge UserDefTwo and UserDefNested in tests, Eric Blake, 2015/04/05