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

diff --git a/src/libinput-private.h b/src/libinput-private.h
index 3414388fac07e103b95bbab8e9af297af71e20c0..b08c03516801dcaa52fc5c031d6f4110945f910e 100644 (file)
--- 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 d2e7596b42681ef77bf70108370a591726401083..61e71026bce03c5726c52df95c249ec2d5e4d572 100644 (file)
--- 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 5f4f51ed93ea7de1620d66a8609c0058cf210ff1..52c7c1a45e1a07104b448b8e612bcb36a00395f8 100644 (file)
--- 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 cafe1fe8ffdd728923e2c385af998bb70ca703b8..10f6b1e0b12f71d4005bcc325356bb90a6e8b67d 100644 (file)
--- 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 610b45d36d3c3f3bd1a9be1bd43b8c17cc948e24..1522c30faa379bc7c6a6d19a865ecd86609c7f51 100644 (file)
--- 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 27f7c042219b27a332e41c6a0dcb723586d43de8..6a9da7543dc47caf715d39573d9120ab5ad16db3 100644 (file)
--- 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 f40533a3078e193f7fc12ef9e0d8d0adaac77f21..50f05211f93725437d79b049648dda04add48a59 100644 (file)
--- 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);