[PATCH libinput 1/2] Hook up libevdev as backend

Jonas Ådahl jadahl at gmail.com
Sat Feb 22 06:51:57 PST 2014


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.hutterer at who-t.net>

Looks good to me as well, with one comment inline.

Reviewed-by: Jonas Ådahl <jadahl at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel


More information about the wayland-devel mailing list