qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v11 19/28] qapi: Change munging of CamelCase enu


From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v11 19/28] qapi: Change munging of CamelCase enum values
Date: Fri, 13 Nov 2015 10:46:15 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0

On 11/10/2015 11:51 PM, Eric Blake wrote:
> When munging enum values, the fact that we were passing the entire
> prefix + value through camel_to_upper() meant that enum values
> spelled with CamelCase could be turned into CAMEL_CASE.  However,
> this provides a potential collision (both OneTwo and One-Two would
> munge into ONE_TWO).  By changing the generation of enum constants
> to always be prefix + '_' + c_name(value).upper(), we can avoid
> any risk of collisions (if we can also ensure no case collisions,
> in the next patch) without having to think about what the
> heuristics in camel_to_upper() will actually do to the value.
> 

> +++ b/scripts/qapi.py
> @@ -1439,7 +1439,7 @@ def camel_to_upper(value):
>  def c_enum_const(type_name, const_name, prefix=None):
>      if prefix is not None:
>          type_name = prefix
> -    return camel_to_upper(type_name + '_' + const_name)
> +    return camel_to_upper(type_name) + '_' + c_name(const_name, 
> False).upper()

Doesn't match the commit message, because it used c_name(,False), while
c_name(name) is short for c_name(name, True).

What's more, looking at it exposed a bug: c_name('_Thread-local')
returns '_Thread_local', but this collides with a C11 keyword (and was
supposed to be munged to q__Thread_local); add '_Thread-local':'int' to
a struct to see the resulting hilarity:

  CC    tests/test-qmp-output-visitor.o
In file included from tests/test-qmp-output-visitor.c:17:0:
tests/test-qapi-types.h:781:13: error: expected identifier or ‘(’ before
‘_Thread_local’
     int64_t _Thread_local;
             ^

But it also made me realize that c_name('Q-int') happily returns 'Q_int'
(we only reserved the leading 'q_' namespace, not 'Q_').  Alas, that
means c_name('Q-int', False).upper() and c_name('int', False).upper()
both produce 'Q_INT', and we have a collision.  So I think enum names
have to be munged by c_name(name, True).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

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