[PATCH libinput] evdev: fail before open_restricted if the devnode doesn't exist

Peter Hutterer peter.hutterer at who-t.net
Tue Feb 13 03:32:40 UTC 2018


On Fri, Feb 09, 2018 at 12:00:47PM -0600, Jeffrey Smith wrote:
> > +       if (!devnode) {
> > +               log_info(libinput, "%s: no device node associated\n", sysname);
> > +               return NULL;
> > +       }
> > +
> >         if (udev_device_should_be_ignored(udev_device)) {
> >                 log_debug(libinput, "%s: device is ignored\n", sysname);
> >                 return NULL;
> 
> As long as you are intentionally wanting this reported even if the
> device "should be ignored", then it LGTM.

I think that's a good idea, it'll help us figure out where the issue is
coming from. udev_device_should_be_ignored() is only true where
LIBINPUT_IGNORE_DEVICE is set and that's a niche-case to start with.

> > +       if (devnode == NULL)
> > +               return -ENODEV;
> > +
> 
> Checking (!devnode) would be better as it matches the other change,
> and in evdev.c implicit NULL comparison is much more common than
> explicit NULL comparison.

fixed, thanks.
 
> AFAICT, the return value from evdev_device_resume is never checked, so
> it seems that this may be a good place for a log message too.  Would
> it be noisy?  If so, maybe a log_debug would be enough.

It shouldn't be too noisy but I don't think there's much of a need for it.
You're resuming a device here that has since gone away (in a manner that I
can't figure out or reproduce though...).  Which means that you should get
the udev event to properly remove the device soon. Extra logs aren't really
needed here, IMO, this just papers over what is likely a short-window race
condition.

Cheers,
   Peter


More information about the wayland-devel mailing list