On 11/27/2015 02:42 AM, Markus Armbruster wrote: > Eric Blake <ebl...@redhat.com> writes: > >> We document that members of enums and objects should be >> 'lower-case', although we were not enforcing it. We have to >> whitelist a few pre-existing entities that violate the norms. >> Add three new tests to expose the new error message, each of >> which first uses the whitelisted name 'UuidInfo' to prove the >> whitelist works, then triggers the failure. >> >> Note that by adding this check, we have effectively forbidden >> an entity with a case-insensitive clash of member names, for >> any entity that is not on the whitelist (although there is >> still the possibility to clash via '-' vs. '_'). >> >> Signed-off-by: Eric Blake <ebl...@redhat.com> > [...] >> @@ -1039,6 +1054,10 @@ class QAPISchemaMember(object): >> >> def check_clash(self, info, seen): >> cname = c_name(self.name) >> + if cname.lower() != cname and info['name'] not in case_whitelist: >> + raise QAPIExprError(info, >> + "Member '%s' of '%s' should use lowercase" >> + % (self.name, info['name'])) >> if cname in seen: >> raise QAPIExprError(info, >> "%s collides with %s" > > As far as I can tell, this is the only use of info['name'] in this > series.
Yes, although I may find more uses for it later.
>
> Can you give an example where info['name'] != self.owner?
Sure; this triggers lots of debug lines before crashing[1]:
diff --git i/scripts/qapi.py w/scripts/qapi.py
index 6a77db4..ec59682 100644
--- i/scripts/qapi.py
+++ w/scripts/qapi.py
@@ -1054,6 +1054,8 @@ class QAPISchemaMember(object):
def check_clash(self, info, seen):
cname = c_name(self.name)
+ if info['name'] != self.owner:
+ print ' ** checking differs in %s, owner is %s' %
(info['name'], self.owner)
if cname.lower() != cname and info['name'] not in case_whitelist:
raise QAPIExprError(info,
"Member '%s' of '%s' should use lowercase"
The very first one is:
** checking differs in block_passwd, owner is :obj-block_passwd-arg
Remember, QAPISchemaMember.owner is the innermost (possibly-implicit)
type that owns the member, while info['name'] is the name of the
top-level entity that encloses the member. So the two are not always
equal. member._pretty_owner() converts from an implicit struct name
back to the top-level entity, but not directly (it is a human-readable
phrase, not the plain entity name).
Furthermore, look at CpuInfo's member 'CPU': there, we have two call
paths (one with info['name'] == 'CpuInfo', the other with it as
'CpuInfoBase') but both call paths would see only self.owner ==
'CpuInfoBase'. The whitelist covers both struct names. Perhaps
whitelisting only 'self.owner' names would be sufficient; but then the
whitelist would have to use implicit type names rather than entity names
from the .json file.
[1] The crash is "TypeError: 'NoneType' object has no attribute
'__getitem__'" at the point where QType is being tested. Normally,
QType is well-formed, so even though it is a builtin type and therefore
has info == None, the 'cname.lower() != cname' test never fails and we
short-circuit past an attempt to dereference None; but not so with my
temporary print hack.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
