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
signature.asc
Description: OpenPGP digital signature
