[systemd-devel] [PATCH 1/5] udev-builtin-input_id: identify trackpoints and accelerometers
Hans de Goede
hdegoede at redhat.com
Thu Apr 2 03:39:23 PDT 2015
Hi,
On 02-04-15 12:21, Lennart Poettering wrote:
> 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?
Yes.
> If so, please add them to missing.h, where we try to centralize definitions of this kind.
Ok, will fix for v2.
>> /* 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?
unsigned long matches what is used in the kernel, it is a bit field,
I do not know what type is preferred for bit fields in systemd / udev.
>
>> 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?
If that can handle hex without a 0x prefix then yes.
Regards,
Hans
More information about the systemd-devel
mailing list