On Mon, Dec 08, 2014 at 09:32:00AM +0100, Hans de Goede wrote: > Hi, > > On 07-12-14 23:42, Peter Hutterer wrote: > >On Fri, Dec 05, 2014 at 02:21:15PM +0100, Hans de Goede wrote: > >>Hi Peter, > >> > >>Thanks for working on this! > >> > >>On 12/02/2014 06:01 AM, Peter Hutterer wrote: > >>>libinput's device handling and server-side fd handling are a bit of a > >>>mismatch, so this is hackier than one would hope for. > >>> > >>>The server sets pInfo->fd and the options "fd" and "device". > >>>The server requires pInfo->fd to be the one triggering select(2) to call > >>>the > >>>correct pInfo->read_input. We can't pass an fd to use into libinput, all we > >>>have is the open_restricted callback. That callback gives us the context, > >>>but > >>>not the pInfo with the fd we want. > >>> > >>>The solution: > >>>1) In PreInit, store the patch + fd combination in a driver-wide list. > >>>Search > >>>that list for an fd in open_restricted, return the pre-openend fd. > >>> > >>>2) Overwrite all devices' pInfo->fd with the libinput epollfd. In this > >>>driver > >>>we don't care about which device read_input is called on, we get the > >>>correct > >>>pInfo to post events from through the struct libinput_device of the > >>>libinput > >>>events. > >>> > >>>3) When a device is closed, swap the real fd back in so systemd-logind > >>>closes the > >>>right fd. > >>> > >>>This saves us worrying about keeping the right refcount on who currently > >>>has > >>>the fd set to the libinput fd. We just set it for all devices, let the > >>>server > >>>figure out which device to call (the first in inputInfo.devices) and we > >>>only > >>>need to remember to swap it back during DEVICE_OFF. > >>> > >>>If the server calls close on a pInfo->fd without telling the driver, > >>>that's a > >>>bug anyway. > >> > >>I've gone over this all twice, and I cannot find a fault in it, nor think of > >>a better way. I do have one small nitpick, as it currently stands this patch > >>adds an unchecked calloc as well as an unchecked strdup (both in fd_push), > >>please > >>fix that. I'm fine with just using the xnf variants (assuming those are > >>exported > >>to drivers). With that fixed this is: > >> > >>Reviewed-by: Hans de Goede <[email protected]> > > > >amended, thanks. > > > >AFAICT the only way to fix this fd mess is by having a libinput API that > >takes an fd in the path backend. > > Yes, I was thinking the same thing, but even then we would still need to swap > the server provided fd in pInfo with the epoll fd, which actually is the > ugliest / > hairiest bit. So I agree this is not worth the trouble.
tbh, that is something that we could fix, the API between drivers and server is up for grabs and not requiring pInfo->fd to be set to the select fd would be possible with a minor ABI bump. then again, effort vs gain... Cheers, Peter _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
