On Mon, Jun 23, 2014 at 11:56:41PM +0200, Jonas Ådahl wrote:
> 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 <jad...@gmail.com>
> ---
>
> There are no users I know of which currently would benefit from this but
> I'm thinking it might be better to at least not disallow. Yay or nay?
>
> Jonas
>
> src/libinput-private.h | 1 +
> src/libinput.c | 14 +++++++++++++-
> src/libinput.h | 25 ++++++++++++++++++++++---
> 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, 71 insertions(+), 39 deletions(-)
>
> diff --git a/src/libinput-private.h b/src/libinput-private.h
> index f0bda1f..cfe6535 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 c4f7fe1..d3e2149 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);
>
> @@ -531,7 +532,13 @@ libinput_drop_destroyed_sources(struct libinput
> *libinput)
> }
>
> LIBINPUT_EXPORT void
> -libinput_destroy(struct libinput *libinput)
> +libinput_ref(struct libinput *libinput)
How about making this function return its argument? Often you have code
like
void obj_new(struct libinput *libinput)
{
[...]
obj->libinput = libinput;
libinput_ref(libinput);
[...]
}
I think this is better:
obj->libinput = libinput_ref(libinput);
Ran
> +{
> + libinput->refcount++;
> +}
> +
> +LIBINPUT_EXPORT void
> +libinput_unref(struct libinput *libinput)
> {
> struct libinput_event *event;
> struct libinput_device *device, *next_device;
> @@ -540,6 +547,11 @@ libinput_destroy(struct libinput *libinput)
> if (libinput == NULL)
> return;
>
> + assert(libinput->refcount > 0);
> + libinput->refcount--;
> + if (libinput->refcount > 0)
> + return;
> +
> libinput_suspend(libinput);
>
> libinput->interface_backend->destroy(libinput);
> diff --git a/src/libinput.h b/src/libinput.h
> index b1b1124..1eada8f 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,26 @@ 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
> + */
> +void
> +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. It is up to the caller to keep
> + * track of the number of references held.
> *
> * @param libinput A previously initialized libinput context
> */
> void
> -libinput_destroy(struct libinput *libinput);
> +libinput_unref(struct libinput *libinput);
>
> /**
> * @ingroup base
> diff --git a/src/udev-seat.c b/src/udev-seat.c
> index 38a13b7..5b61dee 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 a518b66..83153bb 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 0a9cc72..55ba678 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 a281820..fe67d68 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 133bdb6..ad2e1f6 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 24f60e0..99b474e 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 346e59b..7d5668f 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 6af2cb0..09c2a94 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 34acfce..95e7628 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 e080ea8..d2b0428 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;
> --
> 1.9.1
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
