[PATCH libevdev 2/2] Add support for EVIOCREVOKE

[Peter Hutterer]

On Wed, Jan 08, 2014 at 01:09:27PM +0100, David Herrmann wrote:
> Hi
> 
> On Wed, Jan 8, 2014 at 2:28 AM, Peter Hutterer <peter.hutterer at who-t.net> wrote:
> > New in 3.12, EVIOCREVOKE revokes access to an evdev device.
> > The process calling revoke is unlikely to be the same process as the one
> > handling the device, so provide a convenience call to just wrap the
> > ioctl instead of requiring a full libevdev device.
> >
> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > ---
> > In two minds about this one: I suspect that the only users of EVIOCREVOKE
> > are low-level tools that probably don't use libevdev anyway, so even the
> > libevdev_revoke_fd() call won't make them drag in libevdev as dependency.
> >
> > So I'm mostly sending this to the list for the archivse, we probably
> > shouldn't merge this until we have a real need for it.
> 
> Hm, I fully agree here, but I also like to see the test-cases in the
> libevdev suite. So how about we add the functions but don't export it?
> Our GNU-ld --gc-sections flag will remove it from the DSO, anyway, so
> we only add compile-time overhead. I like the fact that with libevdev
> we have a quite comprehensive test-suite for the kernel evdev
> interface and I think we should extend it as much as possible.

in that case it'd be easier to just change the test to use the ioctls
directly. I'm not a fan of having functions in the library that aren't
exported, they still have a maintainance requirement. Changing this test to
just ioctl directly is simple and would do that job.

plus, and that ties in with the other comment below, having a separate
binary for tests that directly use ioctls instead of libevdev seems like a
perfect fit too.
 
> >  libevdev/libevdev.c       |  24 ++++++++++
> >  libevdev/libevdev.h       |  37 ++++++++++++++
> >  test/test-libevdev-init.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 181 insertions(+)
> 
> autotools-TESTS actually supports skipping tests (I think by returning
> 72 or something). We currently run all tests as a single test-binary,
> so we cannot use that. Maybe some day we can split them up so the
> test-results properly resemble the real tests-status.

they used to be split in early versions and I merged them together for gcov.
having one binary that runs everything gives us more useful coverage
information. 

a simple way of getting around this would be to build test-libevdev as it is
now, but also compile separate binaries and then run those as TESTS (and
change test-libevdev as a gcov-dependent target). even then you won't get a
single binary per test but that seems a bit over the top to me anyway (plus
I'm not sure how automake would handle that without causing nightmares). 

if you need the "skipped" information, you could move the tests that
potentially need skipping out into such a separate binary (one each).

> And I really hate the fact that we get EINVAL instead of ENOSYS.. but
> that's how evdev always worked for invalid ioctls, *sigh*.

that was one of the motivations for libevdev, at least we're guaranteed that
the ioctl was correctly issued, so EINVAL is equivalent to ENOSYS (most
times anyway).

Cheers,
   Peter


 
> > +       }
> > +
> > +       rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev1);
> > +       ck_assert_int_eq(rc, -ENODEV);
> > +
> > +       rc = libevdev_next_event(dev2, LIBEVDEV_READ_FLAG_NORMAL, &ev2);
> > +       ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
> > +
> > +out:
> > +       uinput_device_free(uidev);
> > +       libevdev_free(dev);
> > +       libevdev_free(dev2);
> > +       close(fd);
> > +}
> > +END_TEST
> > +
> > +START_TEST(test_revoke_fd)
> > +{
> > +       struct uinput_device* uidev;
> > +       struct libevdev *dev, *dev2;
> > +       int rc, fd;
> > +       struct input_event ev1, ev2;
> > +
> > +       rc = test_create_device(&uidev, &dev,
> > +                               EV_SYN, SYN_REPORT,
> > +                               EV_REL, REL_X,
> > +                               EV_REL, REL_Y,
> > +                               EV_REL, REL_WHEEL,
> > +                               EV_KEY, BTN_LEFT,
> > +                               EV_KEY, BTN_MIDDLE,
> > +                               EV_KEY, BTN_RIGHT,
> > +                               -1);
> > +       ck_assert_msg(rc == 0, "Failed to create device: %s", strerror(-rc));
> > +
> > +       fd = open(uinput_device_get_devnode(uidev), O_RDONLY|O_NONBLOCK);
> > +       ck_assert_int_gt(fd, -1);
> > +       rc = libevdev_new_from_fd(fd, &dev2);
> > +       ck_assert_msg(rc == 0, "Failed to create second device: %s", strerror(-rc));
> > +
> > +       uinput_device_event(uidev, EV_REL, REL_X, 1);
> > +       uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0);
> > +
> > +       rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev1);
> > +       ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
> > +
> > +       rc = libevdev_next_event(dev2, LIBEVDEV_READ_FLAG_NORMAL, &ev2);
> > +       ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
> > +
> > +       ck_assert_int_eq(ev1.type, ev2.type);
> > +       ck_assert_int_eq(ev1.code, ev2.code);
> > +       ck_assert_int_eq(ev1.value, ev2.value);
> > +
> > +       /* revoke second device based on fd */
> > +       rc = libevdev_revoke_fd(fd);
> > +       if (rc == -EINVAL) {
> > +               fprintf(stderr, "::: WARNING: skipping libevdev_revoke_fd test, not suported by current kernel\n");
> > +               goto out;
> > +       }
> > +
> > +       rc = libevdev_next_event(dev2, LIBEVDEV_READ_FLAG_NORMAL, &ev2);
> > +       ck_assert_int_eq(rc, -ENODEV);
> > +
> > +       rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev1);
> > +       ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
> > +
> > +out:
> > +       uinput_device_free(uidev);
> > +       libevdev_free(dev);
> > +       libevdev_free(dev2);
> > +       close(fd);
> > +}
> > +END_TEST
> > +
> >  Suite *
> >  libevdev_init_test(void)
> >  {
> > @@ -397,5 +512,10 @@ libevdev_init_test(void)
> >         tcase_add_test(tc, test_clock_id_events);
> >         suite_add_tcase(s, tc);
> >
> > +       tc = tcase_create("revoke");
> > +       tcase_add_test(tc, test_revoke);
> > +       tcase_add_test(tc, test_revoke_fd);
> > +       suite_add_tcase(s, tc);
> > +
> >         return s;
> >  }
> > --
> > 1.8.4.2
> >
> > _______________________________________________
> > Input-tools mailing list
> > Input-tools at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/input-tools