On Tue, Feb 18, 2014 at 04:09:09PM +1000, Peter Hutterer wrote:
> libevdev wraps the various peculiarities of the evdev kernel API into a
> type-safe API. It also buffers the device so checking for specific features at
> a later time is easier than re-issuing the ioctls. Plus, it gives us almost
> free support for SYN_DROPPED events (in the following patch).
>
> This patch switches all the bit checks over to libevdev and leaves the event
> processing as-is. Makes it easier to review.
>
> Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
Looks good to me as well, with one comment inline.
Reviewed-by: Jonas Ådahl <jad...@gmail.com>
> ---
> configure.ac | 7 ++---
> src/Makefile.am | 2 ++
> src/evdev-touchpad.c | 25 ++++++---------
> src/evdev.c | 87
> +++++++++++++++++++++-------------------------------
> src/evdev.h | 14 ++-------
> 5 files changed, 52 insertions(+), 83 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 44729a9..68e1d35 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -44,6 +44,7 @@ AC_CHECK_DECL(CLOCK_MONOTONIC,[],
> PKG_PROG_PKG_CONFIG()
> PKG_CHECK_MODULES(MTDEV, [mtdev >= 1.1.0])
> PKG_CHECK_MODULES(LIBUDEV, [libudev])
> +PKG_CHECK_MODULES(LIBEVDEV, [libevdev >= 0.4])
>
> if test "x$GCC" = "xyes"; then
> GCC_CFLAGS="-Wall -Wextra -Wno-unused-parameter -g -Wstrict-prototypes
> -Wmissing-prototypes -fvisibility=hidden"
> @@ -64,20 +65,16 @@ AC_ARG_ENABLE(tests,
> [build_tests="$enableval"],
> [build_tests="auto"])
>
> -PKG_CHECK_MODULES(LIBEVDEV, [libevdev >= 0.4], [HAVE_LIBEVDEV="yes"],
> [HAVE_LIBEVDEV="no"])
> PKG_CHECK_MODULES(CHECK, [check >= 0.9.9], [HAVE_CHECK="yes"],
> [HAVE_CHECK="no"])
>
> if test "x$build_tests" = "xauto"; then
> - if test "x$HAVE_CHECK" = "xyes" -a "x$HAVE_LIBEVDEV" = "xyes"; then
> + if test "x$HAVE_CHECK" = "xyes"; then
> build_tests="yes"
> fi
> fi
> if test "x$build_tests" = "xyes" -a "x$HAVE_CHECK" = "xno"; then
> AC_MSG_ERROR([Cannot build tests, check is missing])
> fi
> -if test "x$build_tests" = "xyes" -a "x$HAVE_LIBEVDEV" = "xno"; then
> - AC_MSG_ERROR([Cannot build tests, libevdev is missing])
> -fi
>
> AM_CONDITIONAL(BUILD_TESTS, [test "x$build_tests" = "xyes"])
>
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 6e27b3b..f544ccd 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -20,9 +20,11 @@ libinput_la_SOURCES = \
>
> libinput_la_LIBADD = $(MTDEV_LIBS) \
> $(LIBUDEV_LIBS) \
> + $(LIBEVDEV_LIBS) \
> -lm
> libinput_la_CFLAGS = $(MTDEV_CFLAGS) \
> $(LIBUDEV_CFLAGS) \
> + $(LIBEVDEV_CFLAGS) \
> $(GCC_CFLAGS)
>
> pkgconfigdir = $(libdir)/pkgconfig
> diff --git a/src/evdev-touchpad.c b/src/evdev-touchpad.c
> index d65ebb2..8185bf2 100644
> --- a/src/evdev-touchpad.c
> +++ b/src/evdev-touchpad.c
> @@ -170,16 +170,16 @@ struct touchpad_dispatch {
> static enum touchpad_model
> get_touchpad_model(struct evdev_device *device)
> {
> - struct input_id id;
> + int vendor, product;
> unsigned int i;
>
> - if (ioctl(device->fd, EVIOCGID, &id) < 0)
> - return TOUCHPAD_MODEL_UNKNOWN;
> + vendor = libevdev_get_id_vendor(device->evdev);
> + product = libevdev_get_id_product(device->evdev);
>
> for (i = 0; i < ARRAY_LENGTH(touchpad_spec_table); i++)
> - if (touchpad_spec_table[i].vendor == id.vendor &&
> + if (touchpad_spec_table[i].vendor == vendor &&
> (!touchpad_spec_table[i].product ||
> - touchpad_spec_table[i].product == id.product))
> + touchpad_spec_table[i].product == product))
> return touchpad_spec_table[i].model;
>
> return TOUCHPAD_MODEL_UNKNOWN;
> @@ -730,9 +730,7 @@ touchpad_init(struct touchpad_dispatch *touchpad,
> {
> struct motion_filter *accel;
>
> - unsigned long prop_bits[INPUT_PROP_MAX];
> - struct input_absinfo absinfo;
> - unsigned long abs_bits[NBITS(ABS_MAX)];
> + const struct input_absinfo *absinfo;
>
> bool has_buttonpad;
>
> @@ -746,16 +744,13 @@ touchpad_init(struct touchpad_dispatch *touchpad,
> /* Detect model */
> touchpad->model = get_touchpad_model(device);
>
> - ioctl(device->fd, EVIOCGPROP(sizeof(prop_bits)), prop_bits);
> - has_buttonpad = TEST_BIT(prop_bits, INPUT_PROP_BUTTONPAD);
> + has_buttonpad = libevdev_has_property(device->evdev,
> INPUT_PROP_BUTTONPAD);
>
> /* Configure pressure */
> - ioctl(device->fd, EVIOCGBIT(EV_ABS, sizeof(abs_bits)), abs_bits);
> - if (TEST_BIT(abs_bits, ABS_PRESSURE)) {
> - ioctl(device->fd, EVIOCGABS(ABS_PRESSURE), &absinfo);
> + if ((absinfo = libevdev_get_abs_info(device->evdev, ABS_PRESSURE))) {
> configure_touchpad_pressure(touchpad,
> - absinfo.minimum,
> - absinfo.maximum);
> + absinfo->minimum,
> + absinfo->maximum);
> }
>
> /* Configure acceleration factor */
> diff --git a/src/evdev.c b/src/evdev.c
> index d8dff65..ba28fc6 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -491,11 +491,7 @@ evdev_device_dispatch(void *data)
> static int
> evdev_configure_device(struct evdev_device *device)
> {
> - struct input_absinfo absinfo;
> - unsigned long ev_bits[NBITS(EV_MAX)];
> - unsigned long abs_bits[NBITS(ABS_MAX)];
> - unsigned long rel_bits[NBITS(REL_MAX)];
> - unsigned long key_bits[NBITS(KEY_MAX)];
> + const struct input_absinfo *absinfo;
> int has_abs, has_rel, has_mt;
> int has_button, has_keyboard, has_touch;
> unsigned int i;
> @@ -507,84 +503,71 @@ evdev_configure_device(struct evdev_device *device)
> has_keyboard = 0;
> has_touch = 0;
>
> - ioctl(device->fd, EVIOCGBIT(0, sizeof(ev_bits)), ev_bits);
> - if (TEST_BIT(ev_bits, EV_ABS)) {
> - ioctl(device->fd, EVIOCGBIT(EV_ABS, sizeof(abs_bits)),
> - abs_bits);
> + if (libevdev_has_event_type(device->evdev, EV_ABS)) {
>
> - if (TEST_BIT(abs_bits, ABS_X)) {
> - ioctl(device->fd, EVIOCGABS(ABS_X), &absinfo);
> - device->abs.min_x = absinfo.minimum;
> - device->abs.max_x = absinfo.maximum;
> + if ((absinfo = libevdev_get_abs_info(device->evdev, ABS_X))) {
> + device->abs.min_x = absinfo->minimum;
> + device->abs.max_x = absinfo->maximum;
> has_abs = 1;
> }
> - if (TEST_BIT(abs_bits, ABS_Y)) {
> - ioctl(device->fd, EVIOCGABS(ABS_Y), &absinfo);
> - device->abs.min_y = absinfo.minimum;
> - device->abs.max_y = absinfo.maximum;
> + if ((absinfo = libevdev_get_abs_info(device->evdev, ABS_Y))) {
> + device->abs.min_y = absinfo->minimum;
> + device->abs.max_y = absinfo->maximum;
> has_abs = 1;
> }
> /* We only handle the slotted Protocol B in weston.
> Devices with ABS_MT_POSITION_* but not ABS_MT_SLOT
> require mtdev for conversion. */
> - if (TEST_BIT(abs_bits, ABS_MT_POSITION_X) &&
> - TEST_BIT(abs_bits, ABS_MT_POSITION_Y)) {
> - ioctl(device->fd, EVIOCGABS(ABS_MT_POSITION_X),
> - &absinfo);
> - device->abs.min_x = absinfo.minimum;
> - device->abs.max_x = absinfo.maximum;
> - ioctl(device->fd, EVIOCGABS(ABS_MT_POSITION_Y),
> - &absinfo);
> - device->abs.min_y = absinfo.minimum;
> - device->abs.max_y = absinfo.maximum;
> + if (libevdev_has_event_code(device->evdev, EV_ABS,
> ABS_MT_POSITION_X) &&
> + libevdev_has_event_code(device->evdev, EV_ABS,
> ABS_MT_POSITION_Y)) {
> + absinfo = libevdev_get_abs_info(device->evdev,
> ABS_MT_POSITION_X);
> + device->abs.min_x = absinfo->minimum;
> + device->abs.max_x = absinfo->maximum;
> + absinfo = libevdev_get_abs_info(device->evdev,
> ABS_MT_POSITION_Y);
> + device->abs.min_y = absinfo->minimum;
> + device->abs.max_y = absinfo->maximum;
> device->is_mt = 1;
> has_touch = 1;
> has_mt = 1;
>
> - if (!TEST_BIT(abs_bits, ABS_MT_SLOT)) {
> + if (!libevdev_has_event_code(device->evdev, EV_ABS,
> ABS_MT_SLOT)) {
> device->mtdev = mtdev_new_open(device->fd);
> if (!device->mtdev)
> return -1;
> device->mt.slot =
> device->mtdev->caps.slot.value;
> } else {
> - ioctl(device->fd, EVIOCGABS(ABS_MT_SLOT),
> - &absinfo);
> - device->mt.slot = absinfo.value;
> + device->mt.slot =
> libevdev_get_current_slot(device->evdev);
> }
> }
> }
> - if (TEST_BIT(ev_bits, EV_REL)) {
> - ioctl(device->fd, EVIOCGBIT(EV_REL, sizeof(rel_bits)),
> - rel_bits);
> - if (TEST_BIT(rel_bits, REL_X) || TEST_BIT(rel_bits, REL_Y))
> + if (libevdev_has_event_code(device->evdev, EV_REL, REL_X) ||
> + libevdev_has_event_code(device->evdev, EV_REL, REL_Y))
> has_rel = 1;
> - }
> - if (TEST_BIT(ev_bits, EV_KEY)) {
> - ioctl(device->fd, EVIOCGBIT(EV_KEY, sizeof(key_bits)),
> - key_bits);
> - if (TEST_BIT(key_bits, BTN_TOOL_FINGER) &&
> - !TEST_BIT(key_bits, BTN_TOOL_PEN) &&
> +
> + if (libevdev_has_event_type(device->evdev, EV_KEY)) {
> + if (libevdev_has_event_code(device->evdev, EV_KEY,
> BTN_TOOL_FINGER) &&
> + !libevdev_has_event_code(device->evdev, EV_KEY,
> BTN_TOOL_PEN) &&
> (has_abs || has_mt)) {
> device->dispatch = evdev_touchpad_create(device);
> }
> for (i = KEY_ESC; i < KEY_MAX; i++) {
> if (i >= BTN_MISC && i < KEY_OK)
> continue;
> - if (TEST_BIT(key_bits, i)) {
> + if (libevdev_has_event_code(device->evdev, EV_KEY, i)) {
> has_keyboard = 1;
> break;
> }
> }
> - if (TEST_BIT(key_bits, BTN_TOUCH))
> + if (libevdev_has_event_code(device->evdev, EV_KEY, BTN_TOUCH))
> has_touch = 1;
> for (i = BTN_MISC; i < BTN_JOYSTICK; i++) {
> - if (TEST_BIT(key_bits, i)) {
> + if (libevdev_has_event_code(device->evdev, EV_KEY, i)) {
> has_button = 1;
> break;
> }
> }
> }
> - if (TEST_BIT(ev_bits, EV_LED))
> + if (libevdev_has_event_type(device->evdev, EV_LED))
> has_keyboard = 1;
>
> if ((has_abs || has_rel) && has_button)
> @@ -604,7 +587,7 @@ evdev_device_create(struct libinput_seat *seat,
> {
> struct libinput *libinput = seat->libinput;
> struct evdev_device *device;
> - char devname[256] = "unknown";
> + int rc;
> int fd;
> int unhandled_device = 0;
>
> @@ -624,6 +607,10 @@ evdev_device_create(struct libinput_seat *seat,
>
> libinput_device_init(&device->base, seat);
>
> + rc = libevdev_new_from_fd(fd, &device->evdev);
> + if (rc != 0)
> + return NULL;
> +
> device->seat_caps = 0;
> device->is_mt = 0;
> device->mtdev = NULL;
> @@ -635,10 +622,7 @@ evdev_device_create(struct libinput_seat *seat,
> device->dispatch = NULL;
> device->fd = fd;
> device->pending_event = EVDEV_NONE;
> -
> - ioctl(device->fd, EVIOCGNAME(sizeof(devname)), devname);
> - devname[sizeof(devname) - 1] = '\0';
> - device->devname = strdup(devname);
> + device->devname = libevdev_get_name(device->evdev);
This makes the assumption that the const char * returned by
libevdev_get_name() is valid until we destroy the device. Is this
guaranteed anywhere by libevdev?
>
> libinput_seat_ref(seat);
>
> @@ -742,8 +726,7 @@ evdev_device_destroy(struct evdev_device *device)
> dispatch->interface->destroy(dispatch);
>
> libinput_seat_unref(device->base.seat);
> -
> - free(device->devname);
> + libevdev_free(device->evdev);
> free(device->devnode);
> free(device->sysname);
> free(device);
> diff --git a/src/evdev.h b/src/evdev.h
> index 3c9f93a..a9e27bf 100644
> --- a/src/evdev.h
> +++ b/src/evdev.h
> @@ -27,6 +27,7 @@
> #include "config.h"
>
> #include <linux/input.h>
> +#include <libevdev/libevdev.h>
>
> #include "libinput-private.h"
>
> @@ -55,10 +56,11 @@ struct evdev_device {
> struct libinput_source *source;
>
> struct evdev_dispatch *dispatch;
> + struct libevdev *evdev;
> char *output_name;
> char *devnode;
> char *sysname;
> - char *devname;
> + const char *devname;
> int fd;
> struct {
> int min_x, max_x, min_y, max_y;
> @@ -86,16 +88,6 @@ struct evdev_device {
> int is_mt;
> };
>
> -/* copied from udev/extras/input_id/input_id.c */
> -/* we must use this kernel-compatible implementation */
> -#define BITS_PER_LONG (sizeof(unsigned long) * 8)
> -#define NBITS(x) ((((x)-1)/BITS_PER_LONG)+1)
> -#define OFF(x) ((x)%BITS_PER_LONG)
> -#define BIT(x) (1UL<<OFF(x))
> -#define LONG(x) ((x)/BITS_PER_LONG)
> -#define TEST_BIT(array, bit) ((array[LONG(bit)] >> OFF(bit)) & 1)
> -/* end copied */
> -
> #define EVDEV_UNHANDLED_DEVICE ((struct evdev_device *) 1)
>
> struct evdev_dispatch;
> --
> 1.8.4.2
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
