qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH RFC 3/5] qapi: Use common name mangling for enum


From: Daniel P. Berrange
Subject: Re: [Qemu-devel] [PATCH RFC 3/5] qapi: Use common name mangling for enumeration constants
Date: Thu, 5 Nov 2015 16:01:14 +0000
User-agent: Mutt/1.5.23 (2015-06-09)

On Thu, Nov 05, 2015 at 04:30:00PM +0100, Markus Armbruster wrote:
> QAPI names needn't be valid C identifiers, so we mangle them with
> c_name().  Except for enumeration constants, which we mangle with
> camel_to_upper().
> 
> c_name() is easy enough to understand: replace '.' and '-' by '_',
> prefix certain ticklish identifiers with 'q_'.
> 
> camel_to_upper() is a hairball of heuristics, and guessing how it'll
> mangle interesting input could serve as a (nerdy) game.  Despite some
> tweaking (commit 5d371f4), it's still inadqeuate for some QAPI names
> (commit 351d36e).
> 
> Example: QAPI definition
> 
>     { 'enum': 'BlockDeviceIoStatus', 'data': [ 'ok', 'failed', 'nospace' ] }
> 
> generates
> 
>     typedef enum BlockDeviceIoStatus {
>         BLOCK_DEVICE_IO_STATUS_OK = 0,
>         BLOCK_DEVICE_IO_STATUS_FAILED = 1,
>         BLOCK_DEVICE_IO_STATUS_NOSPACE = 2,
>         BLOCK_DEVICE_IO_STATUS_MAX = 3,
>     } BlockDeviceIoStatus;
> 
> Observe that c_name() maps BlockDeviceIoStatus to itself, and
> camel_to_upper() maps it to BLOCK_DEVICE_IO_STATUS, i.e. the
> enumeration constants are shouted, the enumeration type isn't.
> 
> Because mangled names must not clash, name mangling restricts the
> names you can use.  For example, you can't have member 'a-b' and
> 'a_b'.
> 
> Having two separate manglings complicates this.  Enumeration constants
> must be distinct after mangling with camel_to_upper().  But as soon as
> you use the enumeration as union tag, they must *also* be distinct
> after mangling with c_name().
> 
> Having shouted enumeration constants isn't worth the complexity cost.

I rather disagree with this. Having all-uppercase for enum constants
is a really widely used convention and I think it is pretty useful
when reading to code to have constants (whether #define/enum) clearly
marked in uppercase.  After this change we'll have situation where
QAPI enums uses CamelCase, while all other non-QAPI enums (whether
defined in QEMU source, or via #included 3rd party headers use
UPPER_CASE for constants. I think that's rather unpleasant.


I agree with your premise that predicting the output of the qapi
name mangler though is essentially impossible for mortals. How
about a counter proposal....

First make 'prefix' compulsory for enums, instead of trying to
figure out a suitable prefix automatically. Then, have a very
simple rule for the enum constants where you just uppercase a-z
and translate any non-alpha-numeric character into a _. Don't
try todo anything else special like figuring out word boundaries.

That would get of much of the complexity in the qapi name mangler
and give a easily predictable enum constant name. Thus the vast
majority of .c source files (possibly even all of them) would not
need any change.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|



reply via email to

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