[PATCH libinput 1/9] evdev: Dynamically allocate slot array
Jonas Ådahl
jadahl at gmail.com
Wed Apr 9 23:17:37 PDT 2014
On Thu, Apr 10, 2014 at 03:43:25PM +1000, Peter Hutterer wrote:
> 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.
To use size_t here I had to change cast size_t to a signed integer (or
other way around), but sure, I'll make the change.
>
> > +{
> > + 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.
MAX_SLOTS is defined in libinput/src/evdev.h. I guess we could allocate
less ad-hoc; until now I've kept MAX_SLOTS check to protect against
potentially misbehaving driver (if a driver lazilly advertises max slots
to some very high number for example). Will libevdev protect against
this?
>
> > +
> > 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.
Sure, I'll split it out.
>
> 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