Re: [Qemu-devel] [PATCH v11 20/28] qapi: Forbid case-insensitive clashes

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2384 bytes --]

On 11/11/2015 07:53 AM, Markus Armbruster wrote:
> Eric Blake <eblake@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 <eblake@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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]