Re: [Qemu-devel] [PATCH v13 12/14] qapi: Enforce (or whitelist) case conventions on qapi members

[Eric Blake <eblake@redhat.com>]

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

On 11/27/2015 02:03 AM, Markus Armbruster wrote:
> Eric Blake <eblake@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.
> 

>> +    'X86CPURegister32',
> 
> Because of all enum members.
> 
> *Not* visible in QMP, thus fixable.  Fix or TODO comment, please.

Not visible directly in QMP, but unfortunately visible in QOM.  See
target-i386/cpu.c, where x86_reg_info_32[] references the all-caps
names, and where x86_cpu_get_feature_words() calls
visit_type_X86CPUFeatureWordInfoList() and thereby exposes
X86CPURegister32 to the caller through 'qom-get'.

I can certainly add comments, though.

>> +++ b/tests/qapi-schema/args-member-case.json
>> @@ -0,0 +1,3 @@
>> +# Member names should be 'lower-case' unless the struct/command is whitelisted
>> +{ 'command': 'UuidInfo', 'data': { 'Arg': 'int' } }
>> +{ 'command': 'Foo', 'data': { 'Arg': 'int' } }
> 
> We normally put positive tests in qapi-schema-test.json, but I think
> keeping this one here makes more sense.

The idea of a positive test prior to a negative test, in order to prove
that the .err file refers only to the line number of the negative test,
was copied from returns-whitelist.json.  But maybe I can reuse the
'no-way-this-will-get-whitelisted' name from that test :)

-- 
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 --]