qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v11 23/24] qapi: Reserve 'u' member name


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH v11 23/24] qapi: Reserve 'u' member name
Date: Tue, 27 Oct 2015 16:14:09 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)

Eric Blake <address@hidden> writes:

> On 10/27/2015 02:21 AM, Markus Armbruster wrote:
>> Eric Blake <address@hidden> writes:
>> 
>>> Now that we have separated union tag values from colliding with
>>> non-variant C names, by naming the union 'u', we should reserve
>>> this name for our use.  Note that we want to forbid 'u' even in
>>> a struct with no variants, because it is possible for a future
>>> qemu release to extend QMP in a backwards-compatible manner while
>>> converting from a struct to a flat union.  Fortunately, no
>>> existing clients were using this member name.  If we ever find
>>> the need for QMP to have a member 'u', we could at that time
>>> relax things, perhaps by having c_name() munge the QMP member to
>>> 'q_u'.
>>>
>>> Note that we cannot forbid 'u' everywhere (by adding the
>>> rejection code to check_name()), because the existing QKeyCode
>>> enum already uses it; therefore we only reserve it as a struct
>>> type member name.
>>>
>>> Signed-off-by: Eric Blake <address@hidden>
>>>
>>> ---
>>> v11: commit message tweaks, use c_name(), tweak test names
>
>>> -        if c_name(key).startswith('has_'):
>>> +        if c_name(key, False) == 'u' or c_name(key).startswith('has_'):
>> 
>> Slightly odd: new c_name() has protect=False, the existing one doesn't.
>> While we don't really need protect=False, it feels a bit cleaner.  If
>> you like, I can add it to the existing one when it gets added in PATCH
>> 05.
>
> You're right - either both places need it, or neither place does.
> Argument _for_ using c_name(, False): that's what we do in check_name()
> when looking for 'q_', because we have to (if we use the default
> c_name(, True), then the name gets munged and starts with q_ even though
> the original did not).  So even though we don't munge 'u' or 'has_' now,
> if c_name() starts munging them in the future, consistently using
> c_name(, False) here will protect us.  So yes, make the change in patch 5.

Done.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]