From 13e9a1d7449d3b5db1dc7d9cf3aa6187911ee08c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20=C3=85dahl?= Date: Wed, 25 Jun 2014 00:06:57 +0200 Subject: [PATCH 1/3] Make ref count unref/ref() functions return resulting object pointer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In order to know if an unref() destroyed an object and to allow more convenient use of ref(), make both functions return a pointer to the object it was passed, or NULL if that object was destroyed. Signed-off-by: Jonas Ådahl Reviewed-by: Peter Hutterer Signed-off-by: Peter Hutterer --- src/libinput.c | 22 ++++++++++++++++------ src/libinput.h | 12 ++++++++---- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/libinput.c b/src/libinput.c index c4f7fe12..d4d57117 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -607,10 +607,11 @@ libinput_seat_init(struct libinput_seat *seat, list_insert(&libinput->seat_list, &seat->link); } -LIBINPUT_EXPORT void +LIBINPUT_EXPORT struct libinput_seat * libinput_seat_ref(struct libinput_seat *seat) { seat->refcount++; + return seat; } static void @@ -622,13 +623,17 @@ libinput_seat_destroy(struct libinput_seat *seat) seat->destroy(seat); } -LIBINPUT_EXPORT void +LIBINPUT_EXPORT struct libinput_seat * libinput_seat_unref(struct libinput_seat *seat) { assert(seat->refcount > 0); seat->refcount--; - if (seat->refcount == 0) + if (seat->refcount == 0) { libinput_seat_destroy(seat); + return NULL; + } else { + return seat; + } } LIBINPUT_EXPORT void @@ -663,10 +668,11 @@ libinput_device_init(struct libinput_device *device, device->refcount = 1; } -LIBINPUT_EXPORT void +LIBINPUT_EXPORT struct libinput_device * libinput_device_ref(struct libinput_device *device) { device->refcount++; + return device; } static void @@ -675,13 +681,17 @@ libinput_device_destroy(struct libinput_device *device) evdev_device_destroy((struct evdev_device *) device); } -LIBINPUT_EXPORT void +LIBINPUT_EXPORT struct libinput_device * libinput_device_unref(struct libinput_device *device) { assert(device->refcount > 0); device->refcount--; - if (device->refcount == 0) + if (device->refcount == 0) { libinput_device_destroy(device); + return NULL; + } else { + return device; + } } LIBINPUT_EXPORT int diff --git a/src/libinput.h b/src/libinput.h index b1b1124e..3503b76f 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -1080,8 +1080,9 @@ libinput_log_set_handler(libinput_log_handler log_handler, * the seat correctly to avoid dangling pointers. * * @param seat A previously obtained seat + * @return The passed seat */ -void +struct libinput_seat * libinput_seat_ref(struct libinput_seat *seat); /** @@ -1093,8 +1094,9 @@ libinput_seat_ref(struct libinput_seat *seat); * the seat correctly to avoid dangling pointers. * * @param seat A previously obtained seat + * @return NULL if seat was destroyed, otherwise the passed seat */ -void +struct libinput_seat * libinput_seat_unref(struct libinput_seat *seat); /** @@ -1167,8 +1169,9 @@ libinput_seat_get_logical_name(struct libinput_seat *seat); * the device correctly to avoid dangling pointers. * * @param device A previously obtained device + * @return The passed device */ -void +struct libinput_device * libinput_device_ref(struct libinput_device *device); /** @@ -1180,8 +1183,9 @@ libinput_device_ref(struct libinput_device *device); * the device correctly to avoid dangling pointers. * * @param device A previously obtained device + * @return NULL if device was destroyed, otherwise the passed device */ -void +struct libinput_device * libinput_device_unref(struct libinput_device *device); /** From faab25c25cd9cf1b76962e9cb8b26c2c754c0cde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20=C3=85dahl?= Date: Wed, 25 Jun 2014 00:06:58 +0200 Subject: [PATCH 2/3] Make context reference counted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of only allowing one owner keeping a libinput context alive, make context reference counted, replacing libinput_destroy() with libinput_unref() while adding another function libinput_ref(). Even though there might not be any current use cases, it doesn't mean we should hard code this usage model in the API. The old behaviour can be emulated by never calling libinput_ref() while replacing libinput_destroy() with libinput_unref(). Signed-off-by: Jonas Ådahl Reviewed-by: Peter Hutterer Signed-off-by: Peter Hutterer --- src/libinput-private.h | 1 + src/libinput.c | 21 ++++++++++++++++++--- src/libinput.h | 28 ++++++++++++++++++++++++---- src/udev-seat.c | 2 +- test/keyboard.c | 2 +- test/litest.c | 2 +- test/log.c | 10 +++++----- test/misc.c | 10 +++++----- test/path.c | 20 ++++++++++---------- test/pointer.c | 2 +- test/udev.c | 16 ++++++++-------- tools/event-debug.c | 4 ++-- tools/event-gui.c | 2 +- 13 files changed, 78 insertions(+), 42 deletions(-) diff --git a/src/libinput-private.h b/src/libinput-private.h index f0bda1f8..cfe6535b 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -57,6 +57,7 @@ struct libinput { const struct libinput_interface *interface; const struct libinput_interface_backend *interface_backend; void *user_data; + int refcount; }; typedef void (*libinput_seat_destroy_func) (struct libinput_seat *seat); diff --git a/src/libinput.c b/src/libinput.c index d4d57117..3aeca7dc 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -502,6 +502,7 @@ libinput_init(struct libinput *libinput, libinput->interface = interface; libinput->interface_backend = interface_backend; libinput->user_data = user_data; + libinput->refcount = 1; list_init(&libinput->source_destroy_list); list_init(&libinput->seat_list); @@ -530,15 +531,27 @@ libinput_drop_destroyed_sources(struct libinput *libinput) list_init(&libinput->source_destroy_list); } -LIBINPUT_EXPORT void -libinput_destroy(struct libinput *libinput) +LIBINPUT_EXPORT struct libinput * +libinput_ref(struct libinput *libinput) +{ + libinput->refcount++; + return libinput; +} + +LIBINPUT_EXPORT struct libinput * +libinput_unref(struct libinput *libinput) { struct libinput_event *event; struct libinput_device *device, *next_device; struct libinput_seat *seat, *next_seat; if (libinput == NULL) - return; + return NULL; + + assert(libinput->refcount > 0); + libinput->refcount--; + if (libinput->refcount > 0) + return libinput; libinput_suspend(libinput); @@ -562,6 +575,8 @@ libinput_destroy(struct libinput *libinput) libinput_drop_destroyed_sources(libinput); close(libinput->epoll_fd); free(libinput); + + return NULL; } LIBINPUT_EXPORT void diff --git a/src/libinput.h b/src/libinput.h index 3503b76f..72375f7b 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -793,6 +793,9 @@ struct libinput_interface { * device are ignored. Such devices and those that failed to open * ignored until the next call to libinput_resume(). * + * The reference count of the context is initialized to 1. See @ref + * libinput_unref. + * * @param interface The callback interface * @param user_data Caller-specific data passed to the various callback * interfaces. @@ -818,6 +821,9 @@ libinput_udev_create_for_seat(const struct libinput_interface *interface, * The context is fully initialized but will not generate events until at * least one device has been added. * + * The reference count of the context is initialized to 1. See @ref + * libinput_unref. + * * @param interface The callback interface * @param user_data Caller-specific data passed to the various callback * interfaces. @@ -967,13 +973,27 @@ libinput_suspend(struct libinput *libinput); /** * @ingroup base * - * Destroy the libinput context. After this, object references associated with - * the destroyed context are invalid and may not be interacted with. + * Add a reference to the context. A context is destroyed whenever the + * reference count reaches 0. See @ref libinput_unref. + * + * @param libinput A previously initialized valid libinput context + * @return The passed libinput context + */ +struct libinput * +libinput_ref(struct libinput *libinput); + +/** + * @ingroup base + * + * Dereference the libinput context. After this, the context may have been + * destroyed, if the last reference was dereferenced. If so, the context is + * invalid and may not be interacted with. * * @param libinput A previously initialized libinput context + * @return NULL if context was destroyed otherwise the passed context */ -void -libinput_destroy(struct libinput *libinput); +struct libinput * +libinput_unref(struct libinput *libinput); /** * @ingroup base diff --git a/src/udev-seat.c b/src/udev-seat.c index 38a13b72..5b61dee6 100644 --- a/src/udev-seat.c +++ b/src/udev-seat.c @@ -358,7 +358,7 @@ libinput_udev_create_for_seat(const struct libinput_interface *interface, if (udev_input_enable(&input->base) < 0) { udev_unref(udev); - libinput_destroy(&input->base); + libinput_unref(&input->base); free(input); return NULL; } diff --git a/test/keyboard.c b/test/keyboard.c index a518b66c..83153bb9 100644 --- a/test/keyboard.c +++ b/test/keyboard.c @@ -108,7 +108,7 @@ START_TEST(keyboard_seat_key_count) for (i = 0; i < num_devices; ++i) litest_delete_device(devices[i]); - libinput_destroy(libinput); + libinput_unref(libinput); } END_TEST diff --git a/test/litest.c b/test/litest.c index 0a9cc72d..55ba678b 100644 --- a/test/litest.c +++ b/test/litest.c @@ -573,7 +573,7 @@ litest_delete_device(struct litest_device *d) libinput_device_unref(d->libinput_device); if (d->owns_context) - libinput_destroy(d->libinput); + libinput_unref(d->libinput); libevdev_free(d->evdev); libevdev_uinput_destroy(d->uinput); memset(d,0, sizeof(*d)); diff --git a/test/log.c b/test/log.c index a2818207..fe67d68e 100644 --- a/test/log.c +++ b/test/log.c @@ -86,7 +86,7 @@ START_TEST(log_handler_invoked) ck_assert_int_gt(log_handler_called, 0); log_handler_called = 0; - libinput_destroy(li); + libinput_unref(li); libinput_log_set_priority(pri); } END_TEST @@ -106,7 +106,7 @@ START_TEST(log_userdata_NULL) ck_assert_int_gt(log_handler_called, 0); log_handler_called = 0; - libinput_destroy(li); + libinput_unref(li); libinput_log_set_priority(pri); } @@ -127,7 +127,7 @@ START_TEST(log_userdata) ck_assert_int_gt(log_handler_called, 0); log_handler_called = 0; - libinput_destroy(li); + libinput_unref(li); libinput_log_set_priority(pri); } END_TEST @@ -148,7 +148,7 @@ START_TEST(log_handler_NULL) log_handler_called = 0; libinput_log_set_handler(simple_log_handler, NULL); - libinput_destroy(li); + libinput_unref(li); libinput_log_set_priority(pri); } END_TEST @@ -173,7 +173,7 @@ START_TEST(log_priority) log_handler_called = 0; - libinput_destroy(li); + libinput_unref(li); libinput_log_set_priority(pri); } END_TEST diff --git a/test/misc.c b/test/misc.c index 133bdb60..ad2e1f66 100644 --- a/test/misc.c +++ b/test/misc.c @@ -133,7 +133,7 @@ START_TEST(event_conversion_device_notify) libinput_event_destroy(event); } - libinput_destroy(li); + libinput_unref(li); libevdev_uinput_destroy(uinput); ck_assert_int_gt(device_added, 0); @@ -194,7 +194,7 @@ START_TEST(event_conversion_pointer) libinput_event_destroy(event); } - libinput_destroy(li); + libinput_unref(li); libevdev_uinput_destroy(uinput); ck_assert_int_gt(motion, 0); @@ -254,7 +254,7 @@ START_TEST(event_conversion_pointer_abs) libinput_event_destroy(event); } - libinput_destroy(li); + libinput_unref(li); libevdev_uinput_destroy(uinput); ck_assert_int_gt(motion, 0); @@ -304,7 +304,7 @@ START_TEST(event_conversion_key) libinput_event_destroy(event); } - libinput_destroy(li); + libinput_unref(li); libevdev_uinput_destroy(uinput); ck_assert_int_gt(key, 0); @@ -364,7 +364,7 @@ START_TEST(event_conversion_touch) libinput_event_destroy(event); } - libinput_destroy(li); + libinput_unref(li); libevdev_uinput_destroy(uinput); ck_assert_int_gt(touch, 0); diff --git a/test/path.c b/test/path.c index 24f60e01..99b474eb 100644 --- a/test/path.c +++ b/test/path.c @@ -65,7 +65,7 @@ START_TEST(path_create_NULL) ck_assert(li == NULL); li = libinput_path_create_context(&simple_interface, NULL); ck_assert(li != NULL); - libinput_destroy(li); + libinput_unref(li); ck_assert_int_eq(open_func_count, 0); ck_assert_int_eq(close_func_count, 0); @@ -92,7 +92,7 @@ START_TEST(path_create_invalid) ck_assert_int_eq(open_func_count, 0); ck_assert_int_eq(close_func_count, 0); - libinput_destroy(li); + libinput_unref(li); ck_assert_int_eq(close_func_count, 0); open_func_count = 0; @@ -126,7 +126,7 @@ START_TEST(path_create_destroy) ck_assert_int_eq(open_func_count, 1); libevdev_uinput_destroy(uinput); - libinput_destroy(li); + libinput_unref(li); ck_assert_int_eq(close_func_count, 1); open_func_count = 0; @@ -372,7 +372,7 @@ START_TEST(path_suspend) libinput_resume(li); libevdev_uinput_destroy(uinput); - libinput_destroy(li); + libinput_unref(li); open_func_count = 0; close_func_count = 0; @@ -406,7 +406,7 @@ START_TEST(path_double_suspend) libinput_resume(li); libevdev_uinput_destroy(uinput); - libinput_destroy(li); + libinput_unref(li); open_func_count = 0; close_func_count = 0; @@ -440,7 +440,7 @@ START_TEST(path_double_resume) libinput_resume(li); libevdev_uinput_destroy(uinput); - libinput_destroy(li); + libinput_unref(li); open_func_count = 0; close_func_count = 0; @@ -523,7 +523,7 @@ START_TEST(path_add_device_suspend_resume) libevdev_uinput_destroy(uinput1); libevdev_uinput_destroy(uinput2); - libinput_destroy(li); + libinput_unref(li); open_func_count = 0; close_func_count = 0; @@ -614,7 +614,7 @@ START_TEST(path_add_device_suspend_resume_fail) ck_assert_int_eq(nevents, 2); libevdev_uinput_destroy(uinput2); - libinput_destroy(li); + libinput_unref(li); open_func_count = 0; close_func_count = 0; @@ -704,7 +704,7 @@ START_TEST(path_add_device_suspend_resume_remove_device) ck_assert_int_eq(nevents, 1); libevdev_uinput_destroy(uinput1); - libinput_destroy(li); + libinput_unref(li); open_func_count = 0; close_func_count = 0; @@ -790,7 +790,7 @@ START_TEST(path_seat_recycle) ck_assert(found == 1); - libinput_destroy(li); + libinput_unref(li); libevdev_uinput_destroy(uinput); } diff --git a/test/pointer.c b/test/pointer.c index 346e59b7..7d5668f8 100644 --- a/test/pointer.c +++ b/test/pointer.c @@ -292,7 +292,7 @@ START_TEST(pointer_seat_button_count) for (i = 0; i < num_devices; ++i) litest_delete_device(devices[i]); - libinput_destroy(libinput); + libinput_unref(libinput); } END_TEST diff --git a/test/udev.c b/test/udev.c index 6af2cb08..09c2a94f 100644 --- a/test/udev.c +++ b/test/udev.c @@ -97,7 +97,7 @@ START_TEST(udev_create_seat0) ck_assert(event != NULL); libinput_event_destroy(event); - libinput_destroy(li); + libinput_unref(li); udev_unref(udev); } END_TEST @@ -124,7 +124,7 @@ START_TEST(udev_create_empty_seat) ck_assert(event == NULL); libinput_event_destroy(event); - libinput_destroy(li); + libinput_unref(li); udev_unref(udev); } END_TEST @@ -169,7 +169,7 @@ START_TEST(udev_added_seat_default) ck_assert(default_seat_found); - libinput_destroy(li); + libinput_unref(li); udev_unref(udev); } END_TEST @@ -200,7 +200,7 @@ START_TEST(udev_double_suspend) libinput_resume(li); libinput_event_destroy(event); - libinput_destroy(li); + libinput_unref(li); udev_unref(udev); } END_TEST @@ -231,7 +231,7 @@ START_TEST(udev_double_resume) libinput_resume(li); libinput_event_destroy(event); - libinput_destroy(li); + libinput_unref(li); udev_unref(udev); } END_TEST @@ -289,7 +289,7 @@ START_TEST(udev_suspend_resume) process_events_count_devices(li, &num_devices); ck_assert_int_gt(num_devices, 0); - libinput_destroy(li); + libinput_unref(li); udev_unref(udev); } END_TEST @@ -322,7 +322,7 @@ START_TEST(udev_device_sysname) libinput_event_destroy(ev); } - libinput_destroy(li); + libinput_unref(li); udev_unref(udev); } END_TEST @@ -396,7 +396,7 @@ START_TEST(udev_seat_recycle) ck_assert(found == 1); - libinput_destroy(li); + libinput_unref(li); udev_unref(udev); } END_TEST diff --git a/tools/event-debug.c b/tools/event-debug.c index 34acfce2..95e76286 100644 --- a/tools/event-debug.c +++ b/tools/event-debug.c @@ -163,7 +163,7 @@ open_device(struct libinput **li, const char *path) device = libinput_path_add_device(*li, path); if (!device) { fprintf(stderr, "Failed to initialized device %s\n", path); - libinput_destroy(*li); + libinput_unref(*li); return 1; } @@ -478,7 +478,7 @@ main(int argc, char **argv) mainloop(li); - libinput_destroy(li); + libinput_unref(li); if (udev) udev_unref(udev); diff --git a/tools/event-gui.c b/tools/event-gui.c index 95540936..9c39c213 100644 --- a/tools/event-gui.c +++ b/tools/event-gui.c @@ -467,7 +467,7 @@ main(int argc, char *argv[]) gtk_main(); - libinput_destroy(li); + libinput_unref(li); udev_unref(udev); return 0; From 89aa3ca176130104f2fc5c034400cbb4c41cf479 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20=C3=85dahl?= Date: Wed, 25 Jun 2014 00:06:59 +0200 Subject: [PATCH 3/3] test: Add context reference counting test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This test relies on valgrind detecting the leak and use-after-free. Signed-off-by: Jonas Ådahl Reviewed-by: Peter Hutterer Signed-off-by: Peter Hutterer --- test/misc.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/misc.c b/test/misc.c index ad2e1f66..bea7e889 100644 --- a/test/misc.c +++ b/test/misc.c @@ -371,12 +371,32 @@ START_TEST(event_conversion_touch) } END_TEST +START_TEST(context_ref_counting) +{ + struct libinput *li; + + /* These tests rely on valgrind to detect memory leak and use after + * free errors. */ + + li = libinput_path_create_context(&simple_interface, NULL); + ck_assert_notnull(li); + ck_assert_ptr_eq(libinput_unref(li), NULL); + + li = libinput_path_create_context(&simple_interface, NULL); + ck_assert_notnull(li); + ck_assert_ptr_eq(libinput_ref(li), li); + ck_assert_ptr_eq(libinput_unref(li), li); + ck_assert_ptr_eq(libinput_unref(li), NULL); +} +END_TEST + int main (int argc, char **argv) { litest_add_no_device("events:conversion", event_conversion_device_notify); litest_add_no_device("events:conversion", event_conversion_pointer); litest_add_no_device("events:conversion", event_conversion_pointer_abs); litest_add_no_device("events:conversion", event_conversion_key); litest_add_no_device("events:conversion", event_conversion_touch); + litest_add_no_device("context:refcount", context_ref_counting); return litest_run(argc, argv); }