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 <arm...@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 <ebl...@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
