[PATCH libinput 1/9] evdev: Dynamically allocate slot array

Peter Hutterer peter.hutterer at who-t.net
Wed Apr 9 22:43:25 PDT 2014


On Wed, Apr 09, 2014 at 09:02:08PM +0200, Jonas Ådahl wrote:
> Don't have a hard coded (previously 16) slot array size; instead
> allocate dynamically depending what slots are assigned. There is still a
> hard coded max though, to protect from invalid input, but its changed
> to 60.
> 
> Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> ---
>  src/evdev.c | 82 +++++++++++++++++++++++++++++++++++++++++++++----------------
>  src/evdev.h | 13 ++++++----
>  2 files changed, 69 insertions(+), 26 deletions(-)
> 
> diff --git a/src/evdev.c b/src/evdev.c
> index 72e4086..cc0c4bf 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -280,6 +280,36 @@ evdev_process_key(struct evdev_device *device, struct input_event *e, int time)
>  	}
>  }
>  
> +static int
> +alloc_mt_slot_states(struct evdev_device *device, int len)

I'd prefer this to be size_t, and nslots instead of len to be more obvious
re:malloc.

> +{
> +	struct mt_slot *slots = device->mt.slots;
> +	int i;
> +
> +	slots = realloc(slots, sizeof(struct mt_slot) * len);
> +	if (!slots)
> +		return -1;
> +
> +	for (i = device->mt.slots_len; i < len; ++i) {
> +		slots[i].seat_slot = -1;
> +		slots[i].x = 0;
> +		slots[i].y = 0;
> +	}
> +	device->mt.slots = slots;
> +	device->mt.slots_len = len;
> +
> +	return 0;
> +}
> +
> +static void
> +set_active_slot(struct evdev_device *device, int32_t slot)
> +{
> +	slot = min(slot, MAX_SLOTS);
> +	if (slot >= device->mt.slots_len)
> +		alloc_mt_slot_states(device, device->mt.slots_len * 2);
> +	device->mt.slot = min(slot, device->mt.slots_len - 1);
> +}

couldn't we just do alloc_mt_slots(dev, libevdev_num_slots(evdev)) during
configure? the kernel filters events that are outside of the announced slot
range, libevdev filters events outside of the slot range so this code seems
to churn a lot for no reason.

libevdev 1.2 also gets rid of the MAX_SLOTS define, so we don't need this
limit anymore.

> +
>  static void
>  evdev_process_touch(struct evdev_device *device,
>  		    struct input_event *e,
> @@ -288,7 +318,7 @@ evdev_process_touch(struct evdev_device *device,
>  	switch (e->code) {
>  	case ABS_MT_SLOT:
>  		evdev_flush_pending_event(device, time);
> -		device->mt.slot = e->value;
> +		set_active_slot(device, e->value);
>  		break;
>  	case ABS_MT_TRACKING_ID:
>  		if (device->pending_event != EVDEV_NONE &&
> @@ -543,9 +573,11 @@ evdev_device_dispatch(void *data)
>  static int
>  evdev_configure_device(struct evdev_device *device)
>  {
> +	struct libevdev *evdev = device->evdev;

tbh, I'd have preferred this to be a separate change. without this change,
the next hunks would only be 3 lines or so and much easier to review.

Cheers,
   Peter


>  	const struct input_absinfo *absinfo;
>  	int has_abs, has_rel, has_mt;
>  	int has_button, has_keyboard, has_touch;
> +	int active_slot;
>  	unsigned int i;
>  
>  	has_rel = 0;
> @@ -555,14 +587,14 @@ evdev_configure_device(struct evdev_device *device)
>  	has_keyboard = 0;
>  	has_touch = 0;
>  
> -	if (libevdev_has_event_type(device->evdev, EV_ABS)) {
> +	if (libevdev_has_event_type(evdev, EV_ABS)) {
>  
> -		if ((absinfo = libevdev_get_abs_info(device->evdev, ABS_X))) {
> +		if ((absinfo = libevdev_get_abs_info(evdev, ABS_X))) {
>  			device->abs.min_x = absinfo->minimum;
>  			device->abs.max_x = absinfo->maximum;
>  			has_abs = 1;
>  		}
> -		if ((absinfo = libevdev_get_abs_info(device->evdev, ABS_Y))) {
> +		if ((absinfo = libevdev_get_abs_info(evdev, ABS_Y))) {
>  			device->abs.min_y = absinfo->minimum;
>  			device->abs.max_y = absinfo->maximum;
>  			has_abs = 1;
> @@ -570,56 +602,58 @@ evdev_configure_device(struct evdev_device *device)
>                  /* We only handle the slotted Protocol B in weston.
>                     Devices with ABS_MT_POSITION_* but not ABS_MT_SLOT
>                     require mtdev for conversion. */
> -		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);
> +		if (libevdev_has_event_code(evdev, EV_ABS, ABS_MT_POSITION_X) &&
> +		    libevdev_has_event_code(evdev, EV_ABS, ABS_MT_POSITION_Y)) {
> +			absinfo = libevdev_get_abs_info(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);
> +			absinfo = libevdev_get_abs_info(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 (!libevdev_has_event_code(device->evdev, EV_ABS, ABS_MT_SLOT)) {
> +			if (!libevdev_has_event_code(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;
> +				active_slot = device->mtdev->caps.slot.value;
>  			} else {
> -				device->mt.slot = libevdev_get_current_slot(device->evdev);
> +				active_slot = libevdev_get_current_slot(evdev);
>  			}
> +			set_active_slot(device, active_slot);
>  		}
>  	}
> -	if (libevdev_has_event_code(device->evdev, EV_REL, REL_X) ||
> -	    libevdev_has_event_code(device->evdev, EV_REL, REL_Y))
> +	if (libevdev_has_event_code(evdev, EV_REL, REL_X) ||
> +	    libevdev_has_event_code(evdev, EV_REL, REL_Y))
>  			has_rel = 1;
>  
> -	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) &&
> +	if (libevdev_has_event_type(evdev, EV_KEY)) {
> +		if (libevdev_has_event_code(evdev, EV_KEY, BTN_TOOL_FINGER) &&
> +		    !libevdev_has_event_code(evdev, EV_KEY, BTN_TOOL_PEN) &&
>  		    (has_abs || has_mt)) {
>  			device->dispatch = evdev_mt_touchpad_create(device);
>  		}
>  		for (i = KEY_ESC; i < KEY_MAX; i++) {
>  			if (i >= BTN_MISC && i < KEY_OK)
>  				continue;
> -			if (libevdev_has_event_code(device->evdev, EV_KEY, i)) {
> +			if (libevdev_has_event_code(evdev, EV_KEY, i)) {
>  				has_keyboard = 1;
>  				break;
>  			}
>  		}
> -		if (libevdev_has_event_code(device->evdev, EV_KEY, BTN_TOUCH))
> +		if (libevdev_has_event_code(evdev, EV_KEY, BTN_TOUCH))
>  			has_touch = 1;
>  		for (i = BTN_MISC; i < BTN_JOYSTICK; i++) {
> -			if (libevdev_has_event_code(device->evdev, EV_KEY, i)) {
> +			if (libevdev_has_event_code(evdev, EV_KEY, i)) {
>  				has_button = 1;
>  				break;
>  			}
>  		}
>  	}
> -	if (libevdev_has_event_type(device->evdev, EV_LED))
> +	if (libevdev_has_event_type(evdev, EV_LED))
>  		has_keyboard = 1;
>  
>  	if ((has_abs || has_rel) && has_button)
> @@ -670,7 +704,6 @@ evdev_device_create(struct libinput_seat *seat,
>  	device->mtdev = NULL;
>  	device->devnode = strdup(devnode);
>  	device->sysname = strdup(sysname);
> -	device->mt.slot = -1;
>  	device->rel.dx = 0;
>  	device->rel.dy = 0;
>  	device->dispatch = NULL;
> @@ -680,6 +713,12 @@ evdev_device_create(struct libinput_seat *seat,
>  
>  	libinput_seat_ref(seat);
>  
> +	device->mt.slot = -1;
> +	device->mt.slots = NULL;
> +	device->mt.slots_len = 0;
> +	if (alloc_mt_slot_states(device, 4) == -1)
> +		goto err;
> +
>  	if (evdev_configure_device(device) == -1)
>  		goto err;
>  
> @@ -786,6 +825,7 @@ evdev_device_destroy(struct evdev_device *device)
>  
>  	libinput_seat_unref(device->base.seat);
>  	libevdev_free(device->evdev);
> +	free(device->mt.slots);
>  	free(device->devnode);
>  	free(device->sysname);
>  	free(device);
> diff --git a/src/evdev.h b/src/evdev.h
> index 0ab9572..4ec387a 100644
> --- a/src/evdev.h
> +++ b/src/evdev.h
> @@ -31,7 +31,7 @@
>  
>  #include "libinput-private.h"
>  
> -#define MAX_SLOTS 16
> +#define MAX_SLOTS 60
>  
>  enum evdev_event_type {
>  	EVDEV_NONE,
> @@ -50,6 +50,11 @@ enum evdev_device_seat_capability {
>  	EVDEV_DEVICE_TOUCH = (1 << 2)
>  };
>  
> +struct mt_slot {
> +	int32_t seat_slot;
> +	int32_t x, y;
> +};
> +
>  struct evdev_device {
>  	struct libinput_device base;
>  
> @@ -74,10 +79,8 @@ struct evdev_device {
>  
>  	struct {
>  		int slot;
> -		struct {
> -			int32_t seat_slot;
> -			int32_t x, y;
> -		} slots[MAX_SLOTS];
> +		struct mt_slot *slots;
> +		int slots_len;
>  	} mt;
>  	struct mtdev *mtdev;
>  
> -- 
> 1.8.3.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