[PATCH] evdev: add phys property (EVIOCGPHYS) as stable identifier

[Peter Korsgaard]

>>>>> "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