Eric Blake <ebl...@redhat.com> writes:
> Previous commits demonstrated that the generator overlooked various
> bad naming situations:
> - types, commands, and events need a valid name
> - union and alternate branches cannot be marked optional
>
> The set of valid names includes [a-zA-Z0-9._-] (where '.' is
> useful only in downstream extensions).
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
> ---
> scripts/qapi.py | 57
> ++++++++++++++++------
> 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/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 | 5 --
> 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 --
> 14 files changed, 53 insertions(+), 32 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index c42683b..ed5385a 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -15,6 +15,7 @@ import re
> from ordereddict import OrderedDict
> import os
> import sys
> +import string
>
> builtin_types = {
> 'str': 'QTYPE_QSTRING',
> @@ -276,8 +277,27 @@ def discriminator_find_enum_define(expr):
>
> return find_enum(discriminator_type)
>
> +valid_characters = set(string.ascii_letters + string.digits + '.' + '-' +
> '_')
> +def check_name(expr_info, source, name, allow_optional = False):
> + membername = name
> +
> + if not isinstance(name, str):
> + raise QAPIExprError(expr_info,
> + "%s requires a string name" % source)
> + if name == '**':
> + return
I'm afraid this conditional is superfluous or wrong. Our schemata don't
trigger it.
As far as I know, '**' may occur as pseudo-type in a command's 'data' or
'returns', and nowhere else.
Example 1 (qom-get):
'returns': '**'
Example 2 (qom-set):
'data': { 'path': 'str', 'property': 'str', 'value': '**' },
Example 3 (hypothetical)
'returns': { 'frob': '**' }
Both 'data' and 'returns' are checked by check_type().
Example 1 takes the early exit on data = '**' there.
Example 2 goes through this loop
for (key, value) in data.items():
check_name(expr_info, "Member of %s" % source, key,
allow_optional=allow_optional)
check_type(expr_info, "Member '%s' of %s" % (key, source), value,
allow_array=True,
allowed_metas=['built-in', 'union', 'alternate', 'struct',
'enum'],
allow_dict=True, allow_optional=True)
The '**' is fed to check_type(), *not* to check_name(). check_type()
takes the same early exit.
[...]
