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

[Markus Armbruster <armbru@redhat.com>]

Eric Blake <eblake@redhat.com> writes:

> 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.

This is the good part: the rules for clashes become much simpler.

Bonus: the implementation for detecting them will be simple, too.

> Thankfully, only two enums are affected: ErrorClass and InputButton.

By convention (see CODING_STYLE), we use CamelCase for type names, and
nothing else.

Only enums violating this naming convention can be affected.  The bad
part: they exist.

InputButton has two camels: WheelUp and WheelDown.  The C enumeration
constants change from INPUT_BUTTON_WHEEL_UP/WHEEL_DOWN to
INPUT_BUTTON_WHEELUP/WHEELDOWN.  Not exactly an improvement, but one,
there are just 21 occurences in 11 files, and two, I think we can still
fix the enumeration to "lower case with dash", as it's only used by
x-input-send-event.

ErrorClass's members are all camels.  The C enumeration constants change
as follows

    ERROR_CLASS_GENERIC_ERROR           ERROR_CLASS_GENERICERROR
    ERROR_CLASS_COMMAND_NOT_FOUND       ERROR_CLASS_COMMANDNOTFOUND
    ERROR_CLASS_DEVICE_ENCRYPTED        ERROR_CLASS_DEVICEENCRYPTED
    ERROR_CLASS_DEVICE_NOT_ACTIVE       ERROR_CLASS_DEVICENOTACTIVE
    ERROR_CLASS_DEVICE_NOT_FOUND        ERROR_CLASS_DEVICENOTFOUND
    ERROR_CLASS_KVM_MISSING_CAP         ERROR_CLASS_KVMMISSINGCAP

Again, not an improvement, but perhaps tolerabe, because these constants
aren't used much anymore: 55 occurences in 20 files.

If we find it not tolerable, we can manually add aliases: rename the
QAPI type out of the way, say 'QAPIErrorClass', then stick

    typedef enum ErrorClass {
        ERROR_CLASS_GENERIC_ERROR = QAPI_ERROR_CLASS_GENERICERROR,
        ERROR_CLASS_COMMAND_NOT_FOUND = QAPI_ERROR_CLASS_COMMANDNOTFOUND,
        ERROR_CLASS_DEVICE_ENCRYPTED = QAPI_ERROR_CLASS_DEVICEENCRYPTED,
        ERROR_CLASS_DEVICE_NOT_ACTIVE = QAPI_ERROR_CLASS_DEVICENOTACTIVE,
        ERROR_CLASS_DEVICE_NOT_FOUND = QAPI_ERROR_CLASS_DEVICENOTFOUND,
        ERROR_CLASS_KVM_MISSING_CAP = QAPI_ERROR_CLASS_KVMMISSINGCAP,
    } ErrorClass;

into error.h with a suitable comment.

Opinions?