Bug 85573

Main question here: better to get the udev_device handle directly, or the device node and leave the rest up to the caller?

What are the side-effects to each approach?.

If libinput caches the udev_device handle and returns that, there's some odd case where dropping privileges in the caller may give a device the caller does not have access to anymore (struggling to find a use-case for that though, since it would also break hotplugging).

If the cached handle is returned, a buggy caller could call udev_device_unref too often and cause a (hard to debug) segfault in libinput. I'd consider that a normal bug though, can be avoided by returning a new handle for the device.

If a new handle is returned, the code isn't much different than returning the device node, other than adding another udev dependency on the API. The path backend uses udev internally, but returning the device node would be simple enough.

Any new, uncached handle can suffer from race conditions if the device disappears. Not much different to normal error handling though.
Not all devices may have a udev handle, but then again not all devices have a device node.

Comments, more things to think about?

(In reply to Peter Hutterer from comment #1)
> Main question here: better to get the udev_device handle directly, or the
> device node and leave the rest up to the caller?
> 
> What are the side-effects to each approach?.
> 
> If libinput caches the udev_device handle and returns that, there's some odd
> case where dropping privileges in the caller may give a device the caller
> does not have access to anymore (struggling to find a use-case for that
> though, since it would also break hotplugging).
> 
> If the cached handle is returned, a buggy caller could call
> udev_device_unref too often and cause a (hard to debug) segfault in
> libinput. I'd consider that a normal bug though, can be avoided by returning
> a new handle for the device.

That's quite a flippant approach. If you couldn't depend the caller behaving properly, you'd get crashes anyway. And valgrind would tell you where the problem is without a doubt.

> If a new handle is returned, the code isn't much different than returning
> the device node, other than adding another udev dependency on the API. The
> path backend uses udev internally, but returning the device node would be
> simple enough.
> 
> Any new, uncached handle can suffer from race conditions if the device
> disappears. Not much different to normal error handling though.
> Not all devices may have a udev handle, but then again not all devices have
> a device node.

What devices wouldn't have udev handles, and what devices wouldn't have a device node?

(In reply to Bastien Nocera from comment #2)
> That's quite a flippant approach. If you couldn't depend the caller behaving
> properly, you'd get crashes anyway. And valgrind would tell you where the
> problem is without a doubt.

Yeah, but you'd see the crash in libinput first, then have to debug/valgrind to find out it's in the caller. all of which wastes human resources.

> What devices wouldn't have udev handles, and what devices wouldn't have a
> device node?

some virtualised/emulated devices may not have a normal node, e.g. keysym-producing keyboards.

decided to go with the udev handle after all, mainly to avoid race conditions of returning the wrong device path when a device has been removed since.

commit 8b7ef91c1a190fcd8363b0e128520f8d24de4bde
Author: Peter Hutterer <peter.hutterer@who-t.net>
Date:   Fri Oct 31 10:53:53 2014 +1000

    Add a function to retrieve the udev_device handle from a libinput device