On 10/23/2015 07:05 AM, Markus Armbruster wrote: > Eric Blake <ebl...@redhat.com> writes: > >> c_name() produces names starting with 'q_' when protecting >> a QMP member name that would fail to directly compile, but >> in doing so can cause clashes with any QMP name already >> beginning with 'q-' or 'q_'. Likewise, we create a C name >> 'has_' for any optional member, that can clash with any QMP >> name beginning with 'has-' or 'has_'. >> >> Technically, rather than blindly reserving the namespace, >> we could try to complain about user names only when an actual >> collision occurs, or even teach c_name() how to munge names >> to avoid collisions. But it is not trivial, especially when >> collisions can occur across multiple types (such as via >> inheritance or flat unions). Besides, no existing .json >> files are trying to use these names. So it's easier to just >> outright forbid the potential for collision. We can always >> relax things in the future if a real need arises for QMP to >> express member names that have been forbidden here. >> >> 'has_' only has to be reserved for struct/union member names, >> while 'q_' is reserved everywhere (matching the fact that >> we only have optional members, but use c_name() everywhere). >> >> Update and add tests to cover the new error messages. >> >> Signed-off-by: Eric Blake <ebl...@redhat.com> >>
>> +++ b/scripts/qapi.py
>> @@ -376,7 +376,9 @@ def check_name(expr_info, source, name,
>> allow_optional=False,
>> # code always prefixes it with the enum name
>> if enum_member:
>> membername = '_' + membername
>> - if not valid_name.match(membername):
>> + # Reserve the entire 'q_' namespace for c_name()
>> + if not valid_name.match(membername) or \
>> + membername.replace('-', '_').startswith('q_'):
>
> Why not c_name(membername).startswith('q_')?
Recursion. c_name('unix') is 'q_unix', which would be rejected, even
though we want to accept it. (And yes, that was my first try, before the
approach you see here)
>> +++ b/tests/qapi-schema/args-has-clash.json
>> @@ -1,6 +1,6 @@
>> # C member name collision
>> -# FIXME - This parses, but fails to compile, because the C struct is given
>> -# two 'has_a' members, one from the flag for optional 'a', and the other
>> -# from member 'has-a'. Either reject this at parse time, or munge the C
>> -# names to avoid the collision.
>> +# This would attempt to create two 'has_a' members of the C struct, one
>> +# from the flag for optional 'a', and the other from member 'has-a'; so
>> +# instead we reject any member with a name that would collide with 'has_'.
>
> Suggest something like:
>
> # We reject names like 'has-a', because they can collide with the flag
> # for an optional 'a' in generated C.
Sure, your wording changes are fine.
>
> Remind me, why do we need both reserved-command.json and
> reserved-member.json?
Because they are different namespaces. Your comment in 1/25 pointed out
that we could change the command munging to allow qmp_q_unix()
independently from qmp_unix(), even when we can't do the same for members.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
