| Message ID | 20210323094025.3569441-10-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qapi: Enforce naming rules | expand |
On 3/23/21 4:40 AM, Markus Armbruster wrote: > check_name_str() masks leading digits when passed enum_member=True. > Only check_enum() does. Lift the masking into check_enum(). > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > scripts/qapi/expr.py | 23 ++++++++++------------- > 1 file changed, 10 insertions(+), 13 deletions(-) > Reviewed-by: Eric Blake <eblake@redhat.com>
On 3/23/21 5:40 AM, Markus Armbruster wrote: > check_name_str() masks leading digits when passed enum_member=True. > Only check_enum() does. Lift the masking into check_enum(). > > Signed-off-by: Markus Armbruster <armbru@redhat.com> Sure. Reviewed-by: John Snow <jsnow@redhat.com> > --- > scripts/qapi/expr.py | 23 ++++++++++------------- > 1 file changed, 10 insertions(+), 13 deletions(-) > > diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py > index 507550c340..e00467636c 100644 > --- a/scripts/qapi/expr.py > +++ b/scripts/qapi/expr.py > @@ -34,18 +34,11 @@ def check_name_is_str(name, info, source): > > > def check_name_str(name, info, source, > - enum_member=False, > permit_upper=False): > - membername = name > - > - # Enum members can start with a digit, because the generated C > - # code always prefixes it with the enum name > - if enum_member and membername[0].isdigit(): > - membername = 'D' + membername > # Reserve the entire 'q_' namespace for c_name(), and for 'q_empty' > # and 'q_obj_*' implicit type names. > - if not valid_name.match(membername) or \ > - c_name(membername, False).startswith('q_'): > + if not valid_name.match(name) or \ > + c_name(name, False).startswith('q_'): > raise QAPISemError(info, "%s has an invalid name" % source) > if not permit_upper and name.lower() != name: > raise QAPISemError( > @@ -213,11 +206,15 @@ def check_enum(expr, info): > for m in members] > for member in members: > source = "'data' member" > + member_name = member['name'] > check_keys(member, info, source, ['name'], ['if']) > - check_name_is_str(member['name'], info, source) > - source = "%s '%s'" % (source, member['name']) > - check_name_str(member['name'], info, source, > - enum_member=True, permit_upper=permit_upper) > + check_name_is_str(member_name, info, source) > + source = "%s '%s'" % (source, member_name) > + # Enum members may start with a digit > + if member_name[0].isdigit(): > + member_name = 'd' + member_name # Hack: hide the digit > + check_name_str(member_name, info, source, > + permit_upper=permit_upper) > check_if(member, info, source) > > >
On 3/23/21 5:40 AM, Markus Armbruster wrote: > check_name_str() masks leading digits when passed enum_member=True. > Only check_enum() does. Lift the masking into check_enum(). > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > scripts/qapi/expr.py | 23 ++++++++++------------- > 1 file changed, 10 insertions(+), 13 deletions(-) > > diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py > index 507550c340..e00467636c 100644 > --- a/scripts/qapi/expr.py > +++ b/scripts/qapi/expr.py > @@ -34,18 +34,11 @@ def check_name_is_str(name, info, source): > > > def check_name_str(name, info, source, > - enum_member=False, > permit_upper=False): > - membername = name > - > - # Enum members can start with a digit, because the generated C > - # code always prefixes it with the enum name > - if enum_member and membername[0].isdigit(): > - membername = 'D' + membername > # Reserve the entire 'q_' namespace for c_name(), and for 'q_empty' > # and 'q_obj_*' implicit type names. > - if not valid_name.match(membername) or \ > - c_name(membername, False).startswith('q_'): > + if not valid_name.match(name) or \ > + c_name(name, False).startswith('q_'): > raise QAPISemError(info, "%s has an invalid name" % source) > if not permit_upper and name.lower() != name: > raise QAPISemError( > @@ -213,11 +206,15 @@ def check_enum(expr, info): > for m in members] > for member in members: > source = "'data' member" > + member_name = member['name'] > check_keys(member, info, source, ['name'], ['if']) > - check_name_is_str(member['name'], info, source) > - source = "%s '%s'" % (source, member['name']) > - check_name_str(member['name'], info, source, > - enum_member=True, permit_upper=permit_upper) > + check_name_is_str(member_name, info, source) > + source = "%s '%s'" % (source, member_name) > + # Enum members may start with a digit > + if member_name[0].isdigit(): > + member_name = 'd' + member_name # Hack: hide the digit Actually, can you put in one more space here? ^ > + check_name_str(member_name, info, source, > + permit_upper=permit_upper) > check_if(member, info, source) > > >
John Snow <jsnow@redhat.com> writes: > On 3/23/21 5:40 AM, Markus Armbruster wrote: >> check_name_str() masks leading digits when passed enum_member=True. >> Only check_enum() does. Lift the masking into check_enum(). >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> scripts/qapi/expr.py | 23 ++++++++++------------- >> 1 file changed, 10 insertions(+), 13 deletions(-) >> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py >> index 507550c340..e00467636c 100644 >> --- a/scripts/qapi/expr.py >> +++ b/scripts/qapi/expr.py >> @@ -34,18 +34,11 @@ def check_name_is_str(name, info, source): >> >> def check_name_str(name, info, source, >> - enum_member=False, >> permit_upper=False): >> - membername = name >> - >> - # Enum members can start with a digit, because the generated C >> - # code always prefixes it with the enum name >> - if enum_member and membername[0].isdigit(): >> - membername = 'D' + membername >> # Reserve the entire 'q_' namespace for c_name(), and for 'q_empty' >> # and 'q_obj_*' implicit type names. >> - if not valid_name.match(membername) or \ >> - c_name(membername, False).startswith('q_'): >> + if not valid_name.match(name) or \ >> + c_name(name, False).startswith('q_'): >> raise QAPISemError(info, "%s has an invalid name" % source) >> if not permit_upper and name.lower() != name: >> raise QAPISemError( >> @@ -213,11 +206,15 @@ def check_enum(expr, info): >> for m in members] >> for member in members: >> source = "'data' member" >> + member_name = member['name'] >> check_keys(member, info, source, ['name'], ['if']) >> - check_name_is_str(member['name'], info, source) >> - source = "%s '%s'" % (source, member['name']) >> - check_name_str(member['name'], info, source, >> - enum_member=True, permit_upper=permit_upper) >> + check_name_is_str(member_name, info, source) >> + source = "%s '%s'" % (source, member_name) >> + # Enum members may start with a digit >> + if member_name[0].isdigit(): >> + member_name = 'd' + member_name # Hack: hide the digit > > Actually, can you put in one more space here? ^ Too late, pull request is out, I should've waited for your review. We'll need to tidy up on top. I'm prone to this style mistake, because Emacs M-; makes it. >> + check_name_str(member_name, info, source, >> + permit_upper=permit_upper) >> check_if(member, info, source) >> >>
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index 507550c340..e00467636c 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -34,18 +34,11 @@ def check_name_is_str(name, info, source): def check_name_str(name, info, source, - enum_member=False, permit_upper=False): - membername = name - - # Enum members can start with a digit, because the generated C - # code always prefixes it with the enum name - if enum_member and membername[0].isdigit(): - membername = 'D' + membername # Reserve the entire 'q_' namespace for c_name(), and for 'q_empty' # and 'q_obj_*' implicit type names. - if not valid_name.match(membername) or \ - c_name(membername, False).startswith('q_'): + if not valid_name.match(name) or \ + c_name(name, False).startswith('q_'): raise QAPISemError(info, "%s has an invalid name" % source) if not permit_upper and name.lower() != name: raise QAPISemError( @@ -213,11 +206,15 @@ def check_enum(expr, info): for m in members] for member in members: source = "'data' member" + member_name = member['name'] check_keys(member, info, source, ['name'], ['if']) - check_name_is_str(member['name'], info, source) - source = "%s '%s'" % (source, member['name']) - check_name_str(member['name'], info, source, - enum_member=True, permit_upper=permit_upper) + check_name_is_str(member_name, info, source) + source = "%s '%s'" % (source, member_name) + # Enum members may start with a digit + if member_name[0].isdigit(): + member_name = 'd' + member_name # Hack: hide the digit + check_name_str(member_name, info, source, + permit_upper=permit_upper) check_if(member, info, source)
check_name_str() masks leading digits when passed enum_member=True. Only check_enum() does. Lift the masking into check_enum(). Signed-off-by: Markus Armbruster <armbru@redhat.com> --- scripts/qapi/expr.py | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-)