[systemd-devel] [PATCH 1/5] udev-builtin-input_id: identify trackpoints and accelerometers

[Lennart Poettering]

On Thu, 02.04.15 11:48, Hans de Goede (hdegoede at redhat.com) wrote:

> The kernel has been setting the INPUT_PROP_POINTING_STICK property bit
> on trackpoints for a while now, and this is useful information to have
> in various places, so make input_id aware of this and make it set
> ID_INPUT_POINTING_STICK on trackpoints.
> 
> While adding support for querying properties, also add support for the
> recently added INPUT_PROP_ACCELEROMETER property bit.
> 
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
>  src/udev/udev-builtin-input_id.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/src/udev/udev-builtin-input_id.c b/src/udev/udev-builtin-input_id.c
> index 46f1c53..5b1790c 100644
> --- a/src/udev/udev-builtin-input_id.c
> +++ b/src/udev/udev-builtin-input_id.c
> @@ -33,6 +33,14 @@
>  #include "udev.h"
>  #include "util.h"
>  
> +#ifndef INPUT_PROP_POINTING_STICK
> +#define INPUT_PROP_POINTING_STICK       0x05
> +#endif
> +
> +#ifndef INPUT_PROP_ACCELEROMETER
> +#define INPUT_PROP_ACCELEROMETER        0x06
> +#endif

Are these definitions normally defined in input.h? If so, please add
them to missing.h, where we try to centralize definitions of this kind.

>  /* we must use this kernel-compatible implementation */
>  #define BITS_PER_LONG (sizeof(unsigned long) * 8)
>  #define NBITS(x) ((((x)-1)/BITS_PER_LONG)+1)
> @@ -131,6 +139,7 @@ static void test_pointers (struct udev_device *dev,
>                             const unsigned long* bitmask_abs,
>                             const unsigned long* bitmask_key,
>                             const unsigned long* bitmask_rel,
> +                           unsigned long prop,

"unsigned long"? Is this really necessary? Shouldn't we just use
uint64_t? here?

>                             bool test) {
>          int is_mouse = 0;
>          int is_touchpad = 0;
> @@ -182,6 +191,10 @@ static void test_pointers (struct udev_device *dev,
>                  udev_builtin_add_property(dev, test, "ID_INPUT_MOUSE", "1");
>          if (is_touchpad)
>                  udev_builtin_add_property(dev, test, "ID_INPUT_TOUCHPAD", "1");
> +        if (prop & (1 << INPUT_PROP_POINTING_STICK))
> +                udev_builtin_add_property(dev, test, "ID_INPUT_TRACKPOINT", "1");
> +        if (prop & (1 << INPUT_PROP_ACCELEROMETER))
> +                udev_builtin_add_property(dev, test, "ID_INPUT_ACCELEROMETER", "1");
>  }
>  
>  /* key like devices */
> @@ -232,7 +245,8 @@ static int builtin_input_id(struct udev_device *dev, int argc, char *argv[], boo
>          unsigned long bitmask_abs[NBITS(ABS_MAX)];
>          unsigned long bitmask_key[NBITS(KEY_MAX)];
>          unsigned long bitmask_rel[NBITS(REL_MAX)];
> -        const char *sysname, *devnode;
> +        unsigned long prop = 0;
> +        const char *sysname, *devnode, *prop_str;
>  
>          /* walk up the parental chain until we find the real input device; the
>           * argument is very likely a subdevice of this, like eventN */
> @@ -248,7 +262,10 @@ static int builtin_input_id(struct udev_device *dev, int argc, char *argv[], boo
>                  get_cap_mask(dev, pdev, "capabilities/abs", bitmask_abs, sizeof(bitmask_abs), test);
>                  get_cap_mask(dev, pdev, "capabilities/rel", bitmask_rel, sizeof(bitmask_rel), test);
>                  get_cap_mask(dev, pdev, "capabilities/key", bitmask_key, sizeof(bitmask_key), test);
> -                test_pointers(dev, bitmask_ev, bitmask_abs, bitmask_key, bitmask_rel, test);
> +                prop_str = udev_device_get_property_value(pdev, "PROP");
> +                if (prop_str)
> +                       prop = strtoul(prop_str, NULL, 16);

Hmm, we try to avoid direct invocations of strtoul() and friends, due
to the annoying error handling... Can't we use safe_atou64() here, or so?

Lennart

-- 
Lennart Poettering, Red Hat