From 5b5b6bca06562a3f558d8d582fbc90129f1f5d17 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Wed, 15 Jan 2014 16:31:29 +1000 Subject: [PATCH] Drop seat events seats are more a compositor concept than a concept of the input library. All devices in a libinput context are associated with the seat given on creation of the seat (maps to ID_SEAT in udev for the udev backend). A logical seat may be assigned to a device (e.g. WL_SEAT) but this does not necessarily map to the creation of the seat in the compositor. Drop the seat events but keep seat objects around so that the caller can still identify which seat a device belongs to. If the libinput_seat_unref() in the udev backend destroys the seat, the device list of that seat is invalid and we'd be accessing already freed bytes. To avoid this, ref the seat before the device removal loop, then unref it once we're done - that unref then may trigger the actual removal of the seat. Signed-off-by: Peter Hutterer --- src/libinput-private.h | 6 --- src/libinput.c | 68 ----------------------------- src/libinput.h | 18 -------- src/path.c | 2 - src/udev-seat.c | 14 +++--- test/path.c | 10 +++-- test/udev.c | 98 +++--------------------------------------- 7 files changed, 18 insertions(+), 198 deletions(-) diff --git a/src/libinput-private.h b/src/libinput-private.h index 3414388f..b08c0351 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -106,12 +106,6 @@ void libinput_device_init(struct libinput_device *device, struct libinput_seat *seat); -void -notify_added_seat(struct libinput_seat *seat); - -void -notify_removed_seat(struct libinput_seat *seat); - void notify_added_device(struct libinput_device *device); diff --git a/src/libinput.c b/src/libinput.c index d2e7596b..61e71026 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -54,16 +54,6 @@ struct libinput_event { union libinput_event_target target; }; -struct libinput_event_added_seat { - struct libinput_event base; - struct libinput_seat *seat; -}; - -struct libinput_event_removed_seat { - struct libinput_event base; - struct libinput_seat *seat; -}; - struct libinput_event_added_device { struct libinput_event base; struct libinput_device *device; @@ -140,18 +130,6 @@ libinput_event_get_context(struct libinput_event *event) return event->libinput; } -LIBINPUT_EXPORT struct libinput_seat * -libinput_event_added_seat_get_seat(struct libinput_event_added_seat *event) -{ - return event->seat; -} - -LIBINPUT_EXPORT struct libinput_seat * -libinput_event_removed_seat_get_seat(struct libinput_event_removed_seat *event) -{ - return event->seat; -} - LIBINPUT_EXPORT struct libinput_device * libinput_event_added_device_get_device( struct libinput_event_added_device *event) @@ -429,8 +407,6 @@ libinput_event_get_class(struct libinput_event *event) case LIBINPUT_EVENT_NONE: return LIBINPUT_EVENT_CLASS_NONE; - case LIBINPUT_EVENT_ADDED_SEAT: - case LIBINPUT_EVENT_REMOVED_SEAT: case LIBINPUT_EVENT_ADDED_DEVICE: case LIBINPUT_EVENT_REMOVED_DEVICE: return LIBINPUT_EVENT_CLASS_BASE; @@ -466,10 +442,6 @@ libinput_event_destroy(struct libinput_event *event) break; } - if (libinput_event_get_type(event) == LIBINPUT_EVENT_ADDED_SEAT || - libinput_event_get_type(event) == LIBINPUT_EVENT_REMOVED_SEAT) - libinput_seat_unref(((struct libinput_event_added_seat*)event)->seat); - free(event); } @@ -632,46 +604,6 @@ post_device_event(struct libinput_device *device, libinput_post_event(device->seat->libinput, event); } -void -notify_added_seat(struct libinput_seat *seat) -{ - struct libinput_event_added_seat *added_seat_event; - - added_seat_event = malloc(sizeof *added_seat_event); - if (!added_seat_event) - return; - - libinput_seat_ref(seat); - - *added_seat_event = (struct libinput_event_added_seat) { - .seat = seat, - }; - - post_base_event(seat->libinput, - LIBINPUT_EVENT_ADDED_SEAT, - &added_seat_event->base); -} - -void -notify_removed_seat(struct libinput_seat *seat) -{ - struct libinput_event_removed_seat *removed_seat_event; - - removed_seat_event = malloc(sizeof *removed_seat_event); - if (!removed_seat_event) - return; - - libinput_seat_ref(seat); - - *removed_seat_event = (struct libinput_event_removed_seat) { - .seat = seat, - }; - - post_base_event(seat->libinput, - LIBINPUT_EVENT_REMOVED_SEAT, - &removed_seat_event->base); -} - void notify_added_device(struct libinput_device *device) { diff --git a/src/libinput.h b/src/libinput.h index 5f4f51ed..52c7c1a4 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -124,8 +124,6 @@ enum libinput_touch_type { */ enum libinput_event_type { LIBINPUT_EVENT_NONE = 0, - LIBINPUT_EVENT_ADDED_SEAT, - LIBINPUT_EVENT_REMOVED_SEAT, LIBINPUT_EVENT_ADDED_DEVICE, LIBINPUT_EVENT_REMOVED_DEVICE, @@ -150,8 +148,6 @@ union libinput_event_target { }; struct libinput_event; -struct libinput_event_added_seat; -struct libinput_event_removed_seat; struct libinput_event_added_device; struct libinput_event_removed_device; struct libinput_event_keyboard_key; @@ -251,20 +247,6 @@ libinput_event_get_target(struct libinput_event *event); struct libinput* libinput_event_get_context(struct libinput_event *event); -/** - * @defgroup event_added_seat Added seat event - */ - -struct libinput_seat * -libinput_event_added_seat_get_seat(struct libinput_event_added_seat *event); - -/** - * @defgroup event_removed_seat Removed seat event - */ - -struct libinput_seat * -libinput_event_removed_seat_get_seat(struct libinput_event_removed_seat *event); - /** * @defgroup event_added_device Added device event */ diff --git a/src/path.c b/src/path.c index cafe1fe8..10f6b1e0 100644 --- a/src/path.c +++ b/src/path.c @@ -47,7 +47,6 @@ path_input_disable(struct libinput *libinput) /* should only be one seat anyway */ list_for_each_safe(seat, tmp, &libinput->seat_list, base.link) { - notify_removed_seat(&seat->base); list_remove(&seat->base.link); list_init(&seat->base.link); libinput_seat_unref(&seat->base); @@ -74,7 +73,6 @@ path_seat_create(struct path_input *input) libinput_seat_init(&seat->base, &input->base, seat->name, path_seat_destroy); list_insert(&input->base.seat_list, &seat->base.link); - notify_added_seat(&seat->base); return seat; } diff --git a/src/udev-seat.c b/src/udev-seat.c index 610b45d3..1522c30f 100644 --- a/src/udev-seat.c +++ b/src/udev-seat.c @@ -198,8 +198,6 @@ evdev_udev_handler(void *data) device->devname, device->devnode); close_restricted(libinput, device->fd); evdev_device_remove(device); - if (list_empty(&seat->base.devices_list)) - notify_removed_seat(&seat->base); libinput_seat_unref(&seat->base); break; } @@ -217,21 +215,22 @@ udev_input_remove_devices(struct udev_input *input) struct udev_seat *seat, *tmp; list_for_each_safe(seat, tmp, &input->base.seat_list, base.link) { + libinput_seat_ref(&seat->base); list_for_each_safe(device, next, &seat->base.devices_list, base.link) { close_restricted(&input->base, device->fd); evdev_device_remove(device); if (list_empty(&seat->base.devices_list)) { - notify_removed_seat(&seat->base); - /* seat is referenced by the event, so make - sure it's dropped from the seat list now, - to be freed whenever the device is - removed */ + /* if the seat may be referenced by the + client, so make sure it's dropped from + the seat list now, to be freed whenever + the device is removed */ list_remove(&seat->base.link); list_init(&seat->base.link); } libinput_seat_unref(&seat->base); } + libinput_seat_unref(&seat->base); } } @@ -327,7 +326,6 @@ udev_seat_create(struct udev_input *input, const char *seat_name) libinput_seat_init(&seat->base, &input->base, seat_name, udev_seat_destroy); list_insert(&input->base.seat_list, &seat->base.link); - notify_added_seat(&seat->base); return seat; } diff --git a/test/path.c b/test/path.c index 27f7c042..6a9da754 100644 --- a/test/path.c +++ b/test/path.c @@ -134,7 +134,8 @@ START_TEST(path_added_seat) struct litest_device *dev = litest_current_device(); struct libinput *li = dev->libinput; struct libinput_event *event; - struct libinput_event_added_seat *seat_event; + struct libinput_event_added_device *device_event; + struct libinput_device *device; struct libinput_seat *seat; const char *seat_name; enum libinput_event_type type; @@ -145,10 +146,11 @@ START_TEST(path_added_seat) ck_assert(event != NULL); type = libinput_event_get_type(event); - ck_assert_int_eq(type, LIBINPUT_EVENT_ADDED_SEAT); + ck_assert_int_eq(type, LIBINPUT_EVENT_ADDED_DEVICE); - seat_event = (struct libinput_event_added_seat*)event; - seat = libinput_event_added_seat_get_seat(seat_event); + device_event = (struct libinput_event_added_device*)event; + device = libinput_event_added_device_get_device(device_event); + seat = libinput_device_get_seat(device); ck_assert(seat != NULL); seat_name = libinput_seat_get_name(seat); diff --git a/test/udev.c b/test/udev.c index f40533a3..50f05211 100644 --- a/test/udev.c +++ b/test/udev.c @@ -138,7 +138,8 @@ START_TEST(udev_added_seat_default) struct libinput *li; struct libinput_event *event; struct udev *udev; - struct libinput_event_added_seat *seat_event; + struct libinput_event_added_device *device_event; + struct libinput_device *device; struct libinput_seat *seat; const char *seat_name; enum libinput_event_type type; @@ -153,13 +154,14 @@ START_TEST(udev_added_seat_default) while (!default_seat_found && (event = libinput_get_event(li))) { type = libinput_event_get_type(event); - if (type != LIBINPUT_EVENT_ADDED_SEAT) { + if (type != LIBINPUT_EVENT_ADDED_DEVICE) { libinput_event_destroy(event); continue; } - seat_event = (struct libinput_event_added_seat*)event; - seat = libinput_event_added_seat_get_seat(seat_event); + device_event = (struct libinput_event_added_device*)event; + device = libinput_event_added_device_get_device(device_event); + seat = libinput_device_get_seat(device); ck_assert(seat != NULL); seat_name = libinput_seat_get_name(seat); @@ -174,93 +176,6 @@ START_TEST(udev_added_seat_default) } END_TEST -START_TEST(udev_removed_seat) -{ - struct libinput *li; - struct libinput_event *event; - struct udev *udev; -#define MAX_SEATS 30 - char *seat_names[MAX_SEATS] = {0}; - int idx = 0; - - udev = udev_new(); - ck_assert(udev != NULL); - - li = libinput_create_from_udev(&simple_interface, NULL, udev, "seat0"); - ck_assert(li != NULL); - libinput_dispatch(li); - - while ((event = libinput_get_event(li))) { - enum libinput_event_type type; - struct libinput_event_added_seat *seat_event; - struct libinput_seat *seat; - const char *seat_name; - - type = libinput_event_get_type(event); - if (type != LIBINPUT_EVENT_ADDED_SEAT) { - libinput_event_destroy(event); - continue; - } - - seat_event = (struct libinput_event_added_seat*)event; - seat = libinput_event_added_seat_get_seat(seat_event); - ck_assert(seat != NULL); - - seat_name = libinput_seat_get_name(seat); - seat_names[idx++] = strdup(seat_name); - ck_assert_int_lt(idx, MAX_SEATS); - - libinput_event_destroy(event); - } - - libinput_suspend(li); - - while ((event = libinput_get_event(li))) { - enum libinput_event_type type; - struct libinput_event_removed_seat *seat_event; - struct libinput_seat *seat; - const char *seat_name; - int i; - - type = libinput_event_get_type(event); - if (type != LIBINPUT_EVENT_REMOVED_SEAT) { - libinput_event_destroy(event); - continue; - } - - seat_event = (struct libinput_event_removed_seat*)event; - seat = libinput_event_removed_seat_get_seat(seat_event); - ck_assert(seat != NULL); - - seat_name = libinput_seat_get_name(seat); - - for (i = 0; i < idx; i++) { - if (seat_names[i] && - strcmp(seat_names[i], seat_name) == 0) { - free(seat_names[i]); - seat_names[i] = NULL; - break; - } - } - ck_assert_msg(i < idx, - "Seat '%s' unknown or already removed\n", - seat_name); - - libinput_event_destroy(event); - } - - while(idx--) { - ck_assert_msg(seat_names[idx] == NULL, - "Seat '%s' not removed\n", - seat_names[idx]); - } - - libinput_destroy(li); - udev_unref(udev); -#undef MAX_SEATS -} -END_TEST - START_TEST(udev_double_suspend) { struct libinput *li; @@ -388,7 +303,6 @@ int main (int argc, char **argv) { litest_add_no_device("udev:create", udev_create_empty_seat); litest_add_no_device("udev:seat events", udev_added_seat_default); - litest_add_no_device("udev:seat events", udev_removed_seat); litest_add("udev:suspend", udev_double_suspend, LITEST_ANY, LITEST_ANY); litest_add("udev:suspend", udev_double_resume, LITEST_ANY, LITEST_ANY);