[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: |
Markus Armbruster |
Subject: |
Re: [PATCH 10/28] qapi: Rework name checking in preparation of stricter checking |
Date: |
Tue, 23 Mar 2021 17:25:16 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
Eric Blake <eblake@redhat.com> writes:
> 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,
Yes.
> so I won't withhold R-b, but it
> may be worth a tweak to the commit message.
Probably. I'm failing at coming up with a concise text that isn't
confusing.
>> 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?
This is just so that the patch doesn't change behavior. PATCH 24 will
flip it to False.
> Otherwise the patch makes sense.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Thanks!
- Re: [PATCH 13/28] qapi: Enforce event 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
[PATCH 08/28] qapi: Support flat unions tag values with leading digit, Markus Armbruster, 2021/03/23