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

Chase Douglas chase.douglas at canonical.com
Wed Aug 29 13:09:32 PDT 2012


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.

> +  } /* 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();
> +}
> +
>



More information about the xorg-devel mailing list