On 11/11/2015 07:53 AM, Markus Armbruster wrote: > Eric Blake <ebl...@redhat.com> writes: > >> In general, designing user interfaces that rely on case >> distinction is poor practice. Another benefit of enforcing >> a restriction against case-insensitive clashes is that we >> no longer have to worry about the situation of enum values >> that could be distinguished by case if mapped by c_name(), >> but which cannot be distinguished when mapped to C as >> ALL_CAPS by camel_to_upper(). > > With PATCH 19, they're mapped by c_name(N).upper(). >
Yep, need to reword that, thanks to rebase churn. >> Thus, having the generator >> look for case collisions up front will prevent developers >> from worrying whether different munging rules for member >> names compared to enum values as a discriminator will cause >> any problems in qapi unions. >> >> There is also the possibility that we may want to add a >> future extension to QMP of teaching it to be case-insensitive >> (the user could request command 'Quit' instead of 'quit', or >> could spell a struct field as 'CPU' instead of 'cpu'). But >> for that to be a practical extension, we cannot break >> backwards compatibility with any existing struct that was >> already relying on case sensitivity. Fortunately, after the >> previous patch cleaned up CpuInfo, there are no such existing >> qapi structs. Of course, the idea of a future extension is >> not as strong of a reason to make the change. >> >> At any rate, it is easier to be strict now, and relax things >> later if we find a reason to need case-sensitive QMP members, >> than it would be to wish we had the restriction in place. > > Suggest to briefly mention the new test. Will do. > >> Signed-off-by: Eric Blake <ebl...@redhat.com> > > Patch looks good. > Hmm - this only enforces member name case clashes. It would also be worth enforcing command/event/type collisions (technically, only command/event clashes are user-visible, but tracking all entities is probably easier) - probably by modifying QAPISchema._def_entity() and .lookup_entity() to do the same trick of keying the map by a munged name. I'll see about adding that as an additional patch. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
