From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38008) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZwWjR-0006Ki-Lr for qemu-devel@nongnu.org; Wed, 11 Nov 2015 09:50:42 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZwWjL-00042C-Dr for qemu-devel@nongnu.org; Wed, 11 Nov 2015 09:50:41 -0500 From: Markus Armbruster References: <1447224690-9743-1-git-send-email-eblake@redhat.com> <1447224690-9743-20-git-send-email-eblake@redhat.com> Date: Wed, 11 Nov 2015 15:50:15 +0100 In-Reply-To: <1447224690-9743-20-git-send-email-eblake@redhat.com> (Eric Blake's message of "Tue, 10 Nov 2015 23:51:21 -0700") Message-ID: <87egfwbnfc.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v11 19/28] qapi: Change munging of CamelCase enum values List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Kevin Wolf , Peter Maydell , "open list:Block layer core" , "Michael S. Tsirkin" , Jason Wang , qemu-devel@nongnu.org, Michael Roth , Gerd Hoffmann , Amit Shah , Luiz Capitulino , Andreas =?utf-8?Q?F=C3=A4rber?= Eric Blake 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?