From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32952) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZzR0e-0001Xr-So for qemu-devel@nongnu.org; Thu, 19 Nov 2015 10:20:33 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZzR0d-0000SG-OI for qemu-devel@nongnu.org; Thu, 19 Nov 2015 10:20:28 -0500 Received: from mx1.redhat.com ([209.132.183.28]:45156) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZzR0d-0000SB-GN for qemu-devel@nongnu.org; Thu, 19 Nov 2015 10:20:27 -0500 References: <1447836791-369-28-git-send-email-eblake@redhat.com> <1447887345-1170-1-git-send-email-eblake@redhat.com> <874mgi9ksi.fsf@blackfin.pond.sub.org> From: Eric Blake Message-ID: <564DE8B5.3070509@redhat.com> Date: Thu, 19 Nov 2015 08:20:21 -0700 MIME-Version: 1.0 In-Reply-To: <874mgi9ksi.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="N7fH927ulR5mHCLeGpGLvoI7CcuTq1mod" Subject: Re: [Qemu-devel] [PATCH] fixup! qapi: Forbid case-insensitive clashes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org, Michael Roth This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --N7fH927ulR5mHCLeGpGLvoI7CcuTq1mod Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 11/19/2015 06:32 AM, Markus Armbruster wrote: > Eric Blake writes: >=20 >> [Replace the old commit message with this:] >> >> qapi: Forbid case-insensitive clashes >> >> However, we DO have to care about the fact that we have a >> command 'stop' and an event 'STOP' (currently the only case >> collision of any of our .json entities). To solve that, add >> a new QAPISchemaEntity.c_namecase() that returns a munged >> version of the name that can be used in checking for case >> clashes, >=20 > For [-._] clashes, too. Easiest fix it to just say "checking for > clashes". >=20 >> and override it in QAPISchemaEvent to use a different >> case (thus, indexing by .c_namecase() will create two natural >> key partitions), then add a new _clash_dict to >> QAPISchema._def_entity() that checks for case collisions after >=20 > To QAPISchema, actually. In _def_entity(), we only add an entry. Easy fixes. >=20 >> ruling out identical spellings. This works because all >> entities have at least a leading letter in their name. >> >> We could go further and enforce that events are named with >> 'ALL_CAPS' and that nothing else is named in that manner; but >> that can be done as a followup if desired. We could also >> separate commands from type names (the way we already separated >> events into a separate namespace), but then we'd have to make >> .c_namecase() pick something other than the convenient upper() >> vs. lower() in order to partition dictionary keys into three >> sets (perhaps a leading digit, since all entities start with a >> letter). >=20 > The upper / lower partition works for events / anything else, and is > kind of cute, although two separate clash dictionaries might be clearer= =2E > I'd like to sketch that next, to help us decide. Okay, sounds like I should post a v12.5 with the alternative of 2 separate clash dictionaries (3 dictionaries total), and as a complete patch rather than a fixup. > | @@ -800,6 +800,9 @@ class QAPISchemaEntity(object): > | def c_name(self): > | return c_name(self.name) > | =20 > | + def c_namecase(self): > | + return c_name(self.name).lower() > | + > | def check(self, schema): > | pass > | =20 >=20 > The entities are: commands, events, the various types. This > implementation of c_namecase() covers commands and types. It gets > overwritten for events. >=20 > Commands should not use upper case. If we enforced that, the .lower() > would be superfluous there. But it isn't wrong. I won't need c_namecase() with separate dictionaries. Once we have the two approaches to compare, we can decide which one looks nicer. >=20 > | @@ -1040,7 +1043,7 @@ class QAPISchemaObjectTypeMember(object): > | assert self.type > | =20 > | def check_clash(self, info, seen): > | - cname =3D c_name(self.name) > | + cname =3D c_name(self.name).lower() > | if cname in seen: > | raise QAPIExprError(info, > | "%s collides with %s" >=20 > Not an entity, therefore QAPISchemaEntity.c_namecase() isn't inherited.= >=20 > Members should not use upper case. If we enforced that, the .lower() > would be superfluous. Even if we enforce it, we have to whitelist exceptions for backward compatibility; those exceptions would have to use .lower(), or we can just skip collision detection on those whitelisted types (where the existence of the whitelist reminds us to maintain no case collisions by hand). > | @@ -1184,12 +1187,16 @@ class QAPISchemaEvent(QAPISchemaEntity): > | def visit(self, visitor): > | visitor.visit_event(self.name, self.info, self.arg_type) > | =20 > | + def c_namecase(self): > | + return c_name(self.name).upper() > | + >=20 > Event names should not use lower case. If we enforced that, the > .upper() would be superfluous. Looks like we could enforce this one without a whitelist. >=20 > If we enforced our naming conventions, namely types in CamelCase, event= s > in ALL_CAPS, everything else lower-case, the c_namecase() would become > slightly simpler: it's same as c_name() except for types, where it > additionally folds to lower case. The name would be misleading then, > however. Anyway, we can consider that once we enforce naming > conventions. And enforcing conventions certainly won't happen any sooner than the rest of the pending queue is flushed. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --N7fH927ulR5mHCLeGpGLvoI7CcuTq1mod Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJWTei1AAoJEKeha0olJ0NqO3wH/2VoPMdHD0hieyAKUQ6W410A VEMERDIevJxtB+KmeKj2BZoo6/NV40+AZX6QEvkw9xxP3VRmJkgyIwHexWzpAXV7 alMRXgFtOaaXPrOWh5rGAAmSx7I/az+Jj+qAY9/Xo8udtf5xknfitJUUPZ19P2tc TMfwyMoiLzXUzuW3v/oAPxOzOwUc9ARc2Lv0xsfBKgNy2UZr5HVZBKEUESjdYN0v 0ZstO8xgx4ZXEb2CjRi0tqaxafTiUAQDkyygofr3iRxpjDxB0uzin/YVNMWMwfX3 sEc8WqfGSRWitjXgNid+U+cNdneMe/Loni/u6ZgbB1iAM5xzvvNSvMgURB2bzMk= =+sAM -----END PGP SIGNATURE----- --N7fH927ulR5mHCLeGpGLvoI7CcuTq1mod--