[PATCH RFC libevdev 7/7] Typedef the enums instead of using enums directly.

[David Herrmann]

Hi

On Wed, Aug 14, 2013 at 2:50 AM, Peter Hutterer
<peter.hutterer at who-t.net> wrote:
> This would allow us to - in the future - switch the type without breaking
> API, though it's unlikely that we'll ever need this. It's still nicer to
> read IMO.
>
> This is technically an API, but not an ABI change.

I am no big fan of typedefs, why not just use "enum EvdevReadFlags"
directly? libxkbcommon does that and I like it. But this patch is at
least better than using "int" as gcc can do typechecks for enums (even
for bitmasks!).

I also don't understand the need to change the underlying type. enums
are fine for normals numbers and for bitmasks. The only downside of
enums is the fact that their underlying type changes if you add more
values. Default is int, but if you add >32 masks to it, it will change
to "long". This silently breaks ABI, so one needs to look out for
that.

Cheers
David

> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
>  libevdev/libevdev.c |  8 ++++----
>  libevdev/libevdev.h | 18 +++++++++---------
>  2 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/libevdev/libevdev.c b/libevdev/libevdev.c
> index ba5b17f..4dc3ccc 100644
> --- a/libevdev/libevdev.c
> +++ b/libevdev/libevdev.c
> @@ -1084,7 +1084,7 @@ libevdev_kernel_set_abs_info(struct libevdev *dev, unsigned int code, const stru
>  }
>
>  int
> -libevdev_grab(struct libevdev *dev, int grab)
> +libevdev_grab(struct libevdev *dev, libevdev_grab_mode_t grab)
>  {
>         int rc = 0;
>
> @@ -1181,7 +1181,7 @@ libevdev_get_repeat(struct libevdev *dev, int *delay, int *period)
>  }
>
>  int
> -libevdev_kernel_set_led_value(struct libevdev *dev, unsigned int code, enum EvdevLEDValues value)
> +libevdev_kernel_set_led_value(struct libevdev *dev, unsigned int code, libevdev_led_value_t value)
>  {
>         return libevdev_kernel_set_led_values(dev, code, value, -1);
>  }
> @@ -1190,7 +1190,7 @@ int
>  libevdev_kernel_set_led_values(struct libevdev *dev, ...)
>  {
>         struct input_event ev[LED_MAX + 1];
> -       enum EvdevLEDValues val;
> +       libevdev_led_value_t val;
>         va_list args;
>         int code;
>         int rc = 0;
> @@ -1205,7 +1205,7 @@ libevdev_kernel_set_led_values(struct libevdev *dev, ...)
>                         rc = -EINVAL;
>                         break;
>                 }
> -               val = va_arg(args, enum EvdevLEDValues);
> +               val = va_arg(args, libevdev_led_value_t);
>                 if (val != LIBEVDEV_LED_ON && val != LIBEVDEV_LED_OFF) {
>                         rc = -EINVAL;
>                         break;
> diff --git a/libevdev/libevdev.h b/libevdev/libevdev.h
> index db6a2ec..de7182a 100644
> --- a/libevdev/libevdev.h
> +++ b/libevdev/libevdev.h
> @@ -284,14 +284,14 @@
>   */
>  struct libevdev;
>
> -enum EvdevReadFlags {
> +typedef enum {
>         LIBEVDEV_READ_SYNC              = 1, /**< Process data in sync mode */
>         LIBEVDEV_READ_NORMAL            = 2, /**< Process data in normal mode */
>         LIBEVDEV_FORCE_SYNC             = 4, /**< Pretend the next event is a SYN_DROPPED. There is
>                                                   no reason to ever use this except for
>                                                   automated tests, so don't. */
>         LIBEVDEV_READ_BLOCKING          = 8, /**< The fd is not in O_NONBLOCK and a read may block */
> -};
> +} libevdev_read_flag_t;
>
>  /**
>   * @ingroup init
> @@ -369,10 +369,10 @@ typedef void (*libevdev_log_func_t)(const char *format, va_list args);
>  void libevdev_set_log_handler(struct libevdev *dev, libevdev_log_func_t logfunc);
>
>
> -enum EvdevGrabModes {
> +typedef enum {
>         LIBEVDEV_GRAB = 3,
>         LIBEVDEV_UNGRAB = 4,
> -};
> +} libevdev_grab_mode_t;
>
>  /**
>   * Grab or ungrab the device through a kernel EVIOCGRAB. This prevents other
> @@ -390,7 +390,7 @@ enum EvdevGrabModes {
>   * @return 0 if the device was successfull grabbed or ungrabbed, or a
>   * negative errno in case of failure.
>   */
> -int libevdev_grab(struct libevdev *dev, int grab);
> +int libevdev_grab(struct libevdev *dev, libevdev_grab_mode_t grab);
>
>  /**
>   * @ingroup init
> @@ -1147,10 +1147,10 @@ int libevdev_disable_event_code(struct libevdev *dev, unsigned int type, unsigne
>  int libevdev_kernel_set_abs_info(struct libevdev *dev, unsigned int code, const struct input_absinfo *abs);
>
>
> -enum EvdevLEDValues {
> +typedef enum {
>         LIBEVDEV_LED_ON = 3,
>         LIBEVDEV_LED_OFF = 4,
> -};
> +} libevdev_led_value_t;
>
>  /**
>   * @ingroup kernel
> @@ -1165,7 +1165,7 @@ enum EvdevLEDValues {
>   * @param value Specifies whether to turn the LED on or off
>   * @return zero on success, or a negative errno on failure
>   */
> -int libevdev_kernel_set_led_value(struct libevdev *dev, unsigned int code, enum EvdevLEDValues value);
> +int libevdev_kernel_set_led_value(struct libevdev *dev, unsigned int code, libevdev_led_value_t value);
>
>  /**
>   * @ingroup kernel
> @@ -1186,7 +1186,7 @@ int libevdev_kernel_set_led_value(struct libevdev *dev, unsigned int code, enum
>   * @note enabling an LED requires write permissions on the device's file descriptor.
>   *
>   * @param dev The evdev device, already initialized with libevdev_set_fd()
> - * @param ... A pair of LED_* event codes and enum EvdevLEDValues, followed by
> + * @param ... A pair of LED_* event codes and libevdev_led_value_t, followed by
>   * -1 to terminate the list.
>   * @return zero on success, or a negative errno on failure
>   */
> --
> 1.8.2.1
>
> _______________________________________________
> Input-tools mailing list
> Input-tools at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/input-tools