[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 25/36] qapi: Require valid names
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v6 25/36] qapi: Require valid names |
Date: |
Tue, 28 Apr 2015 13:46:27 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Eric Blake <address@hidden> writes:
> 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
Meh. Didn't see the patch, or else I would've shot down these names.
Too late now.
> small hack is used to pass the same regex.
A bit vague.
>
> 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
This is the hack.
Less vague commit message:
Valid names match [a-zA-Z_][a-zA-Z0-9._-]*. Except for enumeration
names, which match [a-zA-Z0-9._-]+. By convention, '.' is used only
in downstream extensions.
> + 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
>
We could reject new enumeration names beginning with a digit with a
whitelist, similar to how we reject new commands returning
non-dictionaries in the next patch. Probably not worth the bother.
Reviewed-by: Markus Armbruster <address@hidden>
- [Qemu-devel] [PATCH v6 18/36] qapi: Better error messages for bad expressions, (continued)
- [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, 2015/04/05
- Re: [Qemu-devel] [PATCH v6 25/36] qapi: Require valid names,
Markus Armbruster <=
- [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
- [Qemu-devel] [PATCH v6 34/36] qapi: Drop inline nested structs in query-pci, Eric Blake, 2015/04/05