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

From: "Andreas Färber" <afaerber@suse.de>
To: Eric Blake <eblake@redhat.com>, qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	Michael Roth <mdroth@linux.vnet.ibm.com>,
	qemu-block <qemu-block@nongnu.org>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Jason Wang <jasowang@redhat.com>,
	armbru@redhat.com, Luiz Capitulino <lcapitulino@redhat.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	Amit Shah <amit.shah@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v11 19/28] qapi: Change munging of CamelCase enum values
Date: Wed, 11 Nov 2015 14:29:28 +0100	[thread overview]
Message-ID: <564342B8.60103@suse.de> (raw)
In-Reply-To: <1447224690-9743-20-git-send-email-eblake@redhat.com>

Am 11.11.2015 um 07:51 schrieb Eric Blake:
> 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.
> 
> Thankfully, only two enums are affected: ErrorClass and InputButton.
> 
> Suggested by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>
[...]
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index a35098f..1c39432 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -461,7 +461,7 @@ static BusState *qbus_find(const char *path, Error **errp)
>          pos += len;
>          dev = qbus_find_dev(bus, elem);
>          if (!dev) {
> -            error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> +            error_set(errp, ERROR_CLASS_DEVICENOTFOUND,
>                        "Device '%s' not found", elem);
>              qbus_list_dev(bus, errp);
>              return NULL;
> @@ -788,7 +788,7 @@ void qmp_device_del(const char *id, Error **errp)
>      }
> 
>      if (!obj) {
> -        error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> +        error_set(errp, ERROR_CLASS_DEVICENOTFOUND,
>                    "Device '%s' not found", id);
>          return;
>      }
[...]
> diff --git a/qom/object.c b/qom/object.c
> index c0decb6..425b7c3 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1018,7 +1018,7 @@ Object *object_property_get_link(Object *obj, const char *name,
>      if (str && *str) {
>          target = object_resolve_path(str, NULL);
>          if (!target) {
> -            error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> +            error_set(errp, ERROR_CLASS_DEVICENOTFOUND,
>                        "Device '%s' not found", str);
>          }
>      }
> @@ -1337,7 +1337,7 @@ static Object *object_resolve_link(Object *obj, const char *name,
>          if (target || ambiguous) {
>              error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name, target_type);
>          } else {
> -            error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> +            error_set(errp, ERROR_CLASS_DEVICENOTFOUND,
>                        "Device '%s' not found", path);
>          }
>          target = NULL;

That spelling is not exactly an improvement, but well,

Reviewed-by: Andreas Färber <afaerber@suse.de>

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)