[PATCH libinput] Fix leaking device groups
Peter Hutterer
peter.hutterer at who-t.net
Sun Sep 6 20:11:11 PDT 2015
If a caller has a reference to a device group when the context is destroyed,
the memory for the group is never released. Calling
libinput_device_group_unref() will release it and there are no side-effects
since the group has no back-references. It's inconsistent with the rest of
libinput though - all other resources get released on libinput_unref().
Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
---
Not really sure about this one. If any caller is relying on this behaviour,
it's a hard break and can cause a crash. otoh this is probably the most
niche feature we have anyway until the tablet branch is merged. And it does
bring things in line with the rest of libinput.
src/evdev.c | 18 ++++--------------
src/libinput-private.h | 11 ++++++++++-
src/libinput.c | 37 ++++++++++++++++++++++++++++++++++++-
test/device.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 97 insertions(+), 16 deletions(-)
diff --git a/src/evdev.c b/src/evdev.c
index 094320e..080e0a1 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -2084,27 +2084,17 @@ static int
evdev_set_device_group(struct evdev_device *device,
struct udev_device *udev_device)
{
+ struct libinput *libinput = device->base.seat->libinput;
struct libinput_device_group *group = NULL;
const char *udev_group;
udev_group = udev_device_get_property_value(udev_device,
"LIBINPUT_DEVICE_GROUP");
- if (udev_group) {
- struct libinput_device *d;
-
- list_for_each(d, &device->base.seat->devices_list, link) {
- const char *identifier = d->group->identifier;
-
- if (identifier &&
- strcmp(identifier, udev_group) == 0) {
- group = d->group;
- break;
- }
- }
- }
+ if (udev_group)
+ group = libinput_device_group_find_group(libinput, udev_group);
if (!group) {
- group = libinput_device_group_create(udev_group);
+ group = libinput_device_group_create(libinput, udev_group);
if (!group)
return 1;
libinput_device_set_device_group(&device->base, group);
diff --git a/src/libinput-private.h b/src/libinput-private.h
index 8b161cc..be99af5 100644
--- a/src/libinput-private.h
+++ b/src/libinput-private.h
@@ -91,6 +91,8 @@ struct libinput {
enum libinput_log_priority log_priority;
void *user_data;
int refcount;
+
+ struct list device_group_list;
};
typedef void (*libinput_seat_destroy_func) (struct libinput_seat *seat);
@@ -224,6 +226,8 @@ struct libinput_device_group {
int refcount;
void *user_data;
char *identifier; /* unique identifier or NULL for singletons */
+
+ struct list link;
};
struct libinput_device {
@@ -321,7 +325,12 @@ libinput_device_init(struct libinput_device *device,
struct libinput_seat *seat);
struct libinput_device_group *
-libinput_device_group_create(const char *identifier);
+libinput_device_group_create(struct libinput *libinput,
+ const char *identifier);
+
+struct libinput_device_group *
+libinput_device_group_find_group(struct libinput *libinput,
+ const char *identifier);
void
libinput_device_set_device_group(struct libinput_device *device,
diff --git a/src/libinput.c b/src/libinput.c
index e564571..1eb35e2 100644
--- a/src/libinput.c
+++ b/src/libinput.c
@@ -212,6 +212,9 @@ libinput_log_set_handler(struct libinput *libinput,
}
static void
+libinput_device_group_destroy(struct libinput_device_group *group);
+
+static void
libinput_post_event(struct libinput *libinput,
struct libinput_event *event);
@@ -944,6 +947,7 @@ libinput_init(struct libinput *libinput,
libinput->refcount = 1;
list_init(&libinput->source_destroy_list);
list_init(&libinput->seat_list);
+ list_init(&libinput->device_group_list);
if (libinput_timer_subsys_init(libinput) != 0) {
free(libinput->events);
@@ -983,6 +987,7 @@ libinput_unref(struct libinput *libinput)
struct libinput_event *event;
struct libinput_device *device, *next_device;
struct libinput_seat *seat, *next_seat;
+ struct libinput_device_group *group, *next_group;
if (libinput == NULL)
return NULL;
@@ -1010,6 +1015,13 @@ libinput_unref(struct libinput *libinput)
libinput_seat_destroy(seat);
}
+ list_for_each_safe(group,
+ next_group,
+ &libinput->device_group_list,
+ link) {
+ libinput_device_group_destroy(group);
+ }
+
libinput_timer_subsys_destroy(libinput);
libinput_drop_destroyed_sources(libinput);
close(libinput->epoll_fd);
@@ -1962,7 +1974,8 @@ libinput_device_group_ref(struct libinput_device_group *group)
}
struct libinput_device_group *
-libinput_device_group_create(const char *identifier)
+libinput_device_group_create(struct libinput *libinput,
+ const char *identifier)
{
struct libinput_device_group *group;
@@ -1976,12 +1989,33 @@ libinput_device_group_create(const char *identifier)
if (!group->identifier) {
free(group);
group = NULL;
+ goto out;
}
}
+ list_init(&group->link);
+ list_insert(&libinput->device_group_list, &group->link);
+
+out:
return group;
}
+struct libinput_device_group *
+libinput_device_group_find_group(struct libinput *libinput,
+ const char *identifier)
+{
+ struct libinput_device_group *g;
+
+ list_for_each(g, &libinput->device_group_list, link) {
+ if (identifier && g->identifier &&
+ streq(g->identifier, identifier)) {
+ return g;
+ }
+ }
+
+ return g;
+}
+
void
libinput_device_set_device_group(struct libinput_device *device,
struct libinput_device_group *group)
@@ -1993,6 +2027,7 @@ libinput_device_set_device_group(struct libinput_device *device,
static void
libinput_device_group_destroy(struct libinput_device_group *group)
{
+ list_remove(&group->link);
free(group->identifier);
free(group);
}
diff --git a/test/device.c b/test/device.c
index 2aa090c..b7fa0e0 100644
--- a/test/device.c
+++ b/test/device.c
@@ -674,6 +674,22 @@ START_TEST(device_context)
}
END_TEST
+static int open_restricted(const char *path, int flags, void *data)
+{
+ int fd;
+ fd = open(path, flags);
+ return fd < 0 ? -errno : fd;
+}
+static void close_restricted(int fd, void *data)
+{
+ close(fd);
+}
+
+const struct libinput_interface simple_interface = {
+ .open_restricted = open_restricted,
+ .close_restricted = close_restricted,
+};
+
START_TEST(device_group_get)
{
struct litest_device *dev = litest_current_device();
@@ -722,6 +738,36 @@ START_TEST(device_group_ref)
}
END_TEST
+START_TEST(device_group_leak)
+{
+ struct libinput *li;
+ struct libinput_device *device;
+ struct libevdev_uinput *uinput;
+ struct libinput_device_group *group;
+
+ uinput = litest_create_uinput_device("test device", NULL,
+ EV_KEY, BTN_LEFT,
+ EV_KEY, BTN_RIGHT,
+ EV_REL, REL_X,
+ EV_REL, REL_Y,
+ -1);
+
+ li = libinput_path_create_context(&simple_interface, NULL);
+ device = libinput_path_add_device(li,
+ libevdev_uinput_get_devnode(uinput));
+
+ group = libinput_device_get_device_group(device);
+ libinput_device_group_ref(group);
+
+ libinput_path_remove_device(device);
+
+ libevdev_uinput_destroy(uinput);
+ libinput_unref(li);
+
+ /* the device group leaks, check valgrind */
+}
+END_TEST
+
START_TEST(abs_device_no_absx)
{
struct libevdev_uinput *uinput;
@@ -1239,6 +1285,7 @@ litest_setup_tests(void)
litest_add("device:group", device_group_get, LITEST_ANY, LITEST_ANY);
litest_add_no_device("device:group", device_group_ref);
+ litest_add_no_device("device:group", device_group_leak);
litest_add_no_device("device:invalid devices", abs_device_no_absx);
litest_add_no_device("device:invalid devices", abs_device_no_absy);
--
2.4.3
More information about the wayland-devel
mailing list