[PATCH libinput 1/2] Make context reference counted

Ran Benita ran234 at gmail.com
Mon Jun 23 15:20:47 PDT 2014


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 <jadahl at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel


More information about the wayland-devel mailing list