[PATCH] evdev: add phys property (EVIOCGPHYS) as stable identifier
Peter Korsgaard
jacmet at sunsite.dk
Tue Apr 27 23:39:34 PDT 2010
>>>>> "Peter" == Peter Hutterer <peter.hutterer at who-t.net> writes:
Hi,
>> +/* Physical location of input device as defined by the kernel */
>> +#define EVDEV_PROP_PHYSICAL "Evdev Physical"
>> +
Peter> Please name this "Evdev Physical Path".
Ok, will do.
Peter> I'm also wondering if we should make this a two-string property
Peter> to export the "Device" option and the Phys path at the same
Peter> time. especially for xorg.conf settings, there is no mapping
Peter> between what had a given device path and what had a phys.
Peter> Which also brings us to the question of whether this should be
Peter> implemented in the server instead then? I'm sure the other
Peter> drivers could benefit from this as well.
EVIOCGPHYS is evdev specific ofcourse, but I guess we could parse the
commonOptions and add the device property in xf86ProcessCommonOptions().
Does that sound good to you?
>> /* All devices the evdev driver has allocated and knows about.
>> @@ -2453,6 +2454,17 @@ EvdevInitProperty(DeviceIntPtr dev)
>> EvdevPtr pEvdev = pInfo->private;
>> int rc;
>> BOOL invert[2];
>> + char phys[256];
Peter> valgrind complains here, can you make this char phys[256] = {0};
Peter> instead please. (conditional jump uninitialized variables blah
Peter> blah)
>> + if (ioctl(pInfo->fd, EVIOCGPHYS(sizeof(phys) - 1), phys) >= 0)
Really? That sounds like a bug in valgrind then. EVIOCGPHYS is correctly
marked as an input ioctl (_IOC(_IOC_READ, 'E', 0x07, len)), so it should
know that phys contains valid data after the ioctl.
But I can certainly add the initializer, even though it's unneeded
bloat.
Peter> as you said on IRC, the man page hunks are missing, please
Peter> include these in the next version.
I will, thanks for reviewing.
--
Bye, Peter Korsgaard
More information about the xorg-devel
mailing list