[PATCH xorg-gtest 6/6] device: use inotify to wait for the device to appear

[Peter Hutterer]

On Wed, Aug 29, 2012 at 01:09:32PM -0700, Chase Douglas wrote:
> On 08/28/2012 11:14 PM, Peter Hutterer wrote:
> >uinput can be too slow, leaving us with GuessDeviceNode() failing and an
> >empty device string. Use inotify to tell us when a /dev/input/event device
> >appeared and then continue.  However, even when inotify tells us the device
> >is there, the EVIOCGNAME may still fail on the device, so we need to keep
> >the backup GuessDeviceNode() in place.
> >
> >This leaves us with a race condition - if a different device pops up while
> >we're waiting, then we may still not get the device name. Chance of that
> >happening is tiny.
> >
> >Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> >---
> >  src/device.cpp                     | 59 +++++++++++++++++++++++++++++++++++++-
> >  test/.gitignore                    |  1 +
> >  test/Makefile.am                   | 10 +++++--
> >  test/PIXART-USB-OPTICAL-MOUSE.desc | 41 ++++++++++++++++++++++++++
> >  test/device-test.cpp               | 33 +++++++++++++++++++++
> >  5 files changed, 141 insertions(+), 3 deletions(-)
> >  create mode 100644 test/PIXART-USB-OPTICAL-MOUSE.desc
> >  create mode 100644 test/device-test.cpp
> >
> >diff --git a/src/device.cpp b/src/device.cpp
> >index 232d3ae..ea98d17 100644
> >--- a/src/device.cpp
> >+++ b/src/device.cpp
> >@@ -31,6 +31,8 @@
> >  #include <linux/input.h>
> >  #include <fcntl.h>
> >  #include <dirent.h>
> >+#include <sys/inotify.h>
> >+#include <poll.h>
> >
> >  #include <stdexcept>
> >
> >@@ -110,6 +112,48 @@ void xorg::testing::evemu::Device::GuessDeviceNode(time_t ctime) {
> >    free(event_devices);
> >  }
> >
> >+static std::string wait_for_inotify(int fd)
> >+{
> >+  std::string devnode;
> >+  bool found = false;
> >+  struct pollfd pfd;
> >+
> >+  pfd.fd = fd;
> >+  pfd.events = POLLIN;
> >+
> >+  char buf[1024];
> >+  size_t bufidx = 0;
> >+
> >+  while (!found && poll(&pfd, 1, 2000) > 0) {
> >+    ssize_t r;
> >+
> >+    r = read(fd, buf + bufidx, sizeof(buf) - bufidx);
> >+    if (r == -1 && errno != EAGAIN) {
> >+      std::cerr << "inotify read failed with: " << std::string(strerror(errno)) << std::endl;
> >+      break;
> >+    }
> >+
> >+    bufidx += r;
> >+
> >+    struct inotify_event *e = reinterpret_cast<struct inotify_event*>(buf);
> >+
> >+    while (bufidx > sizeof(*e) && bufidx >= sizeof(*e) + e->len) {
> >+      if (strncmp(e->name, "event", 5) == 0) {
> >+        devnode = DEV_INPUT_DIR + std::string(e->name);
> >+        found = true;
> >+        break;
> >+      }
> >+
> >+      /* this packet didn't contain what we're looking for */
> >+      int len = sizeof(*e) + e->len;
> >+      memmove(buf, buf + len, bufidx - len);
> >+      bufidx -= len;
> >+    }
> >+  }
> >+
> >+  return devnode;
> >+}
> >+
> >  xorg::testing::evemu::Device::Device(const std::string& path)
> >      : d_(new Private) {
> >    static const char UINPUT_NODE[] = "/dev/uinput";
> >@@ -132,6 +176,14 @@ xorg::testing::evemu::Device::Device(const std::string& path)
> >
> >    fclose(fp);
> >
> >+  int ifd = inotify_init1(IN_NONBLOCK);
> >+  if (ifd == -1 || inotify_add_watch(ifd, DEV_INPUT_DIR, IN_CREATE) == -1) {
> >+    std::cerr << "Failed to create inotify watch" << std::endl;
> >+    if (ifd != -1)
> >+      close(ifd);
> >+    ifd = -1;
> >+  }
> >+
> >    d_->fd = open(UINPUT_NODE, O_WRONLY);
> >    if (d_->fd < 0) {
> >      evemu_delete(d_->device);
> >@@ -145,7 +197,12 @@ xorg::testing::evemu::Device::Device(const std::string& path)
> >      throw std::runtime_error("Failed to create evemu device");
> >    }
> >
> >-  GuessDeviceNode(d_->ctime);
> >+  if (ifd != -1) {
> >+    std::string devnode = wait_for_inotify(ifd);
> >+    if (event_is_device(devnode, evemu_get_name(d_->device), d_->ctime))
> >+        d_->device_node = devnode;
> >+    close(ifd);
> 
> We could instead loop around here waiting until the correct device
> is seen, potentially with a timeout? I'm guessing you've already
> thought of that, so I'm interested to hear your thoughts.

the problem here is the failed ioctl(EVIOCGNAME). inotify gives us the right
device, but because the ioctl fails we cannot be sure it really is our
device.

the alternative here is to loop until the ioctl succeeds but I don't know if
that can hang the test, and putting a loop limit on it just means we'll need
GuessDeviceNode() again.

you can try it, remove the GuessDeviceNode() call and run the new test in a
loop

    while [ $? -eq 0 ]; do sudo ./device-test; done

it'll run anywhere from a minute to several minutes before it fails.

Cheers,
  Peter

> >+  } /* else guess node when we'll need it */
> >  }
> >
> >  void xorg::testing::evemu::Device::Play(const std::string& path) const {
> >diff --git a/test/.gitignore b/test/.gitignore
> >index f1d10c3..51494ab 100644
> >--- a/test/.gitignore
> >+++ b/test/.gitignore
> >@@ -1,2 +1,3 @@
> >  process-test
> >  xserver-test
> >+device-test
> >diff --git a/test/Makefile.am b/test/Makefile.am
> >index ed6416a..4f847b3 100644
> >--- a/test/Makefile.am
> >+++ b/test/Makefile.am
> >@@ -25,10 +25,12 @@
> >  # SOFTWARE.
> >  #
> >
> >-noinst_PROGRAMS = process-test xserver-test
> >+noinst_PROGRAMS = process-test xserver-test device-test
> >+noinst_DATA = PIXART-USB-OPTICAL-MOUSE.desc
> >
> >  AM_CPPFLAGS = $(GTEST_CPPFLAGS)
> >-AM_CXXFLAGS = $(BASE_CXXFLAGS)
> >+AM_CXXFLAGS = $(BASE_CXXFLAGS) \
> >+	      -DTEST_ROOT_DIR=\"$(abs_top_srcdir)/test/\"
> >
> >  tests_libraries = \
> >  	libgtest.a \
> >@@ -45,6 +47,10 @@ xserver_test_SOURCES = xserver-test.cpp
> >  xserver_test_CPPFLAGS = $(AM_CPPFLAGS) -I$(top_srcdir)/include
> >  xserver_test_LDADD =  $(tests_libraries)
> >
> >+device_test_SOURCES = device-test.cpp
> >+device_test_CPPFLAGS = $(AM_CPPFLAGS) -I$(top_srcdir)/include
> >+device_test_LDADD =  $(tests_libraries)
> >+
> >  check_LIBRARIES = libgtest.a libxorg-gtest.a
> >
> >  # build googletest as static lib
> >diff --git a/test/PIXART-USB-OPTICAL-MOUSE.desc b/test/PIXART-USB-OPTICAL-MOUSE.desc
> >new file mode 100644
> >index 0000000..787c837
> >--- /dev/null
> >+++ b/test/PIXART-USB-OPTICAL-MOUSE.desc
> >@@ -0,0 +1,41 @@
> >+# standard USB mouse
> >+# Input driver version is 1.0.1
> >+# Input device ID: bus 0x3 vendor 0x93a product 0x2510 version 0x110
> >+# Input device name: "PIXART USB OPTICAL MOUSE"
> >+# Supported events:
> >+#   Event type 0 (EV_SYN)
> >+#   Event type 1 (EV_KEY)
> >+#     Event code 272 (BTN_LEFT)
> >+#     Event code 273 (BTN_RIGHT)
> >+#     Event code 274 (BTN_MIDDLE)
> >+#   Event type 2 (EV_REL)
> >+#     Event code 0 (REL_X)
> >+#     Event code 1 (REL_Y)
> >+#     Event code 8 (REL_WHEEL)
> >+#   Event type 4 (EV_MSC)
> >+#     Event code 4 (MSC_SCAN)
> >+#
> >+N: PIXART USB OPTICAL MOUSE
> >+I: 0003 093a 2510 0110
> >+P: 00 00 00 00 00 00 00 00
> >+B: 00 17 00 00 00 00 00 00 00
> >+B: 01 00 00 00 00 00 00 00 00
> >+B: 01 00 00 00 00 00 00 00 00
> >+B: 01 00 00 00 00 00 00 00 00
> >+B: 01 00 00 00 00 00 00 00 00
> >+B: 01 00 00 07 00 00 00 00 00
> >+B: 01 00 00 00 00 00 00 00 00
> >+B: 01 00 00 00 00 00 00 00 00
> >+B: 01 00 00 00 00 00 00 00 00
> >+B: 01 00 00 00 00 00 00 00 00
> >+B: 01 00 00 00 00 00 00 00 00
> >+B: 01 00 00 00 00 00 00 00 00
> >+B: 01 00 00 00 00 00 00 00 00
> >+B: 02 03 01 00 00 00 00 00 00
> >+B: 03 00 00 00 00 00 00 00 00
> >+B: 04 10 00 00 00 00 00 00 00
> >+B: 05 00 00 00 00 00 00 00 00
> >+B: 11 00 00 00 00 00 00 00 00
> >+B: 12 00 00 00 00 00 00 00 00
> >+B: 15 00 00 00 00 00 00 00 00
> >+B: 15 00 00 00 00 00 00 00 00
> >diff --git a/test/device-test.cpp b/test/device-test.cpp
> >new file mode 100644
> >index 0000000..56ec676
> >--- /dev/null
> >+++ b/test/device-test.cpp
> >@@ -0,0 +1,33 @@
> >+#include <gtest/gtest.h>
> >+#include <xorg/gtest/xorg-gtest.h>
> >+
> >+#ifdef HAVE_EVEMU
> >+
> >+using namespace xorg::testing;
> >+
> >+TEST(DEVICE, DeviceNode)
> >+{
> >+  SCOPED_TRACE("TESTCASE: Device node is /dev/input/eventX");
> >+
> >+  xorg::testing::evemu::Device d(TEST_ROOT_DIR "PIXART-USB-OPTICAL-MOUSE.desc");
> >+
> >+  ASSERT_EQ(d.GetDeviceNode().compare(0, 16, "/dev/input/event"), 0);
> >+}
> >+
> >+TEST(Device, InotifyWait)
> >+{
> >+  SCOPED_TRACE("TESTCASE: device node can never be empty.\n"
> >+               "This test may show false positives");
> >+
> >+  xorg::testing::evemu::Device d(TEST_ROOT_DIR "PIXART-USB-OPTICAL-MOUSE.desc");
> >+
> >+  ASSERT_FALSE(d.GetDeviceNode().empty());
> >+}
> >+
> >+#endif
> >+
> >+int main(int argc, char *argv[]) {
> >+  testing::InitGoogleTest(&argc, argv);
> >+  return RUN_ALL_TESTS();
> >+}
> >+
> >
>