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. That's doable but gets a bit iffy and needs a lot of special-casing for the libinput_suspend/resume case. I'm not sure yet it's worth adding all this to libinput. Cheers, Peter _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
