[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 10/28] qapi: Rework name checking in preparation of stricter
From: |
Eric Blake |
Subject: |
Re: [PATCH 10/28] qapi: Rework name checking in preparation of stricter checking |
Date: |
Tue, 23 Mar 2021 09:20:49 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 |
On 3/23/21 4:40 AM, Markus Armbruster wrote:
> Naming rules differ for the various kinds of names. To prepare
> enforcing them, define functions to check them: check_name_upper(),
> check_name_lower(), and check_name_camel(). For now, these merely
> wrap around check_name_str(), but that will change shortly. Replace
> the other uses of check_name_str() by appropriate uses of the
> wrappers. No change in behavior just yet.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> scripts/qapi/expr.py | 51 +++++++++++++++++++++++++++++++-------------
> 1 file changed, 36 insertions(+), 15 deletions(-)
>
> +++ b/scripts/qapi/expr.py
> @@ -21,11 +21,12 @@
> from .error import QAPISemError
>
>
> -# Names must be letters, numbers, -, and _. They must start with letter,
> -# except for downstream extensions which must start with __RFQDN_.
> -# Dots are only valid in the downstream extension prefix.
> -valid_name = re.compile(r'^(__[a-zA-Z0-9.-]+_)?'
> - '[a-zA-Z][a-zA-Z0-9_-]*$')
I'm assuming python concatenates r'' with '' in the obvious manner...
> +# Names consist of letters, digits, -, and _, starting with a letter.
> +# An experimental name is prefixed with x-. A name of a downstream
> +# extension is prefixed with __RFQDN_. The latter prefix goes first.
> +valid_name = re.compile(r'(__[a-z0-9.-]+_)?'
> + r'(x-)?'
> + r'([a-z][a-z0-9_-]*)$', re.IGNORECASE)
...but like your explicit use of r'' r''.
Splitting out special handling of r'(x-)?' does not change behavior, but
is not otherwise mentioned in your commit message. I suspect you did it
to make it easier to permit x-EVENT_NAME in later patches where upper is
handled differently from lower or camel, so I won't withhold R-b, but it
may be worth a tweak to the commit message.
>
> def check_defn_name_str(name, info, meta):
> - check_name_str(name, info, meta, permit_upper=True)
> + if meta == 'event':
> + check_name_upper(name, info, meta)
> + elif meta == 'command':
> + check_name_lower(name, info, meta, permit_upper=True)
Why do commands need to permit upper? I guess just downstream FQDN
extensions?
Otherwise the patch makes sense.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
- Re: [PATCH 28/28] qapi: Enforce union and alternate branch naming rules, (continued)
[PATCH 10/28] qapi: Rework name checking in preparation of stricter checking, Markus Armbruster, 2021/03/23
Re: [PATCH 10/28] qapi: Rework name checking in preparation of stricter checking, John Snow, 2021/03/23
- Re: [PATCH 10/28] qapi: Rework name checking in preparation of stricter checking, Markus Armbruster, 2021/03/24
- Re: [PATCH 10/28] qapi: Rework name checking in preparation of stricter checking, John Snow, 2021/03/24
- Re: [PATCH 10/28] qapi: Rework name checking in preparation of stricter checking, Markus Armbruster, 2021/03/25
- Re: [PATCH 10/28] qapi: Rework name checking in preparation of stricter checking, John Snow, 2021/03/25
- Re: [PATCH 10/28] qapi: Rework name checking in preparation of stricter checking, Markus Armbruster, 2021/03/26