[PATCH 01/12] dbus-core: Move to hw/xfree86/common dir

[Peter Hutterer]

On Tue, Jan 28, 2014 at 10:12:30AM +0100, Hans de Goede wrote:
> >>> 8d972e0c xf86Xinput: Modify API for server-managed fd support
> >>>     I worry a little that drivers won't necessarily support this -
> >>> would be nice to check by forcing them to define
> >>> I_SUPPORT_SERVER_MANAGED_FDS or something; either that, or extend
> >>> InputDriverRec with an equivalent flag which must be set.
> >>
> >> Interesting idea, I've investigating this on my todo list for Monday
> >> (I'm not working tomorrow).
> > 
> > how about adding the pre-opened fd into a separate field and provide a 
> > xf86GetManagedFD() call that merely returns that field.
> > that way the driver code comes essentially down to:
> > 
> > pInfo->fd = xf86GetManagedFD(pInfo);
> > if (pInfo->fd < 0)
> >     pInfo->fd = open("path");
> > 
> > and supporting this in the drivers is easy with a few ifdefs, and the server
> > is simple as well for the non-systemd case.  
> 
> I don't see how this helps with Daniel's worry about drivers not being ported,
> the way I see it for non ported drivers we don't want to call systemd_logind_get_fd
> at all. So we need to check if the driver is capable of dealing with server
> managed fds before calling systemd_logind_get_fd, which means delaying the
> systemd_logind_get_fd till we have figured out which driver to use, and having some
> flag in the driver to indicate it can deal with managed fds.
> 
> This seems like an almost orthogonal problem to the API for the driver getting
> the fd to me.

but wouldn't a new call do exactly that? the input driver sequence is
something like this:

int
SomeDriverPreInit()
{
    const char *device = xf86SetStrOption(pInfo->options, "Device", NULL);
    if (!device)
        return BadValue;

    pInfo->fd = open(device, O_RDWR|O_NONBLOCK);
    ...
}

one of the side-effects you get by re-using pInfo->fd is that both the
server and the driver may believe they're in charge of the fd. Which
shouldn't matter, but could become an issue. e.g. evdev leaves the fd open
after PreInit as an "optimization", so if suddenly something else closes it
that could cause issues.

OTOH, if you add a specific call to the driver the code changes to something
like:

int
SomeDriverPreInit()
{
    if (!xf86GetManagedFd(pInfo, &pInfo->fd)) {
        const char *device = xf86SetStrOption(pInfo->options, "Device", NULL);
        if (!device)
            return BadValue;

        pInfo->fd = open(device, O_RDWR|O_NONBLOCK);
    }
    ...
}

and xf86GetManagedFd() does the systemd_logind_get_fd() call.

This way, any non-ported drivers call the normal open and fail if they lack
permissions. Ported drivers continue to work regardless of systemd-logind
support.

And if for some reason you can't do the get_fd() call in xf86GetManagedFd()
you do what you're doing now (i.e. in the options) but just close the
systemd-logind fd after EnableDevice if the driver didn't call
xf86GetManagedFd().

Cheers,
   Peter