[PATCH libinput 2/3] udev: split libinput_udev context init into two functions
Hans de Goede
hdegoede at redhat.com
Mon Jun 16 03:04:32 PDT 2014
Hi,
On 06/13/2014 04:48 AM, Peter Hutterer wrote:
> This is preparation work for context-specific log handlers.
>
> Callers are now encouraged to first initialize the context with
> libinput_udev_create_context() and then set the seat for this context with
> libinput_udev_set_seat().
>
> In the upcoming patch to support context-specific log handlers this enables a
> caller to set the log handler for a context before any devices are
> initialized. Otherwise, a log message generated by a new device may pass a
> libinput context that the caller is not yet aware of.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> doc/libinput.doxygen.in | 3 ++-
> src/libinput.h | 49 +++++++++++++++++++++++++++++++++++++++++++-
> src/udev-seat.c | 54 +++++++++++++++++++++++++++++++++++++++++--------
> test/udev.c | 48 ++++++++++++++++++++++++-------------------
> tools/event-debug.c | 8 +++++++-
> 5 files changed, 130 insertions(+), 32 deletions(-)
>
> diff --git a/doc/libinput.doxygen.in b/doc/libinput.doxygen.in
> index f78b7cf..9f931c3 100644
> --- a/doc/libinput.doxygen.in
> +++ b/doc/libinput.doxygen.in
> @@ -1570,7 +1570,8 @@ INCLUDE_FILE_PATTERNS =
> # undefined via #undef or recursively expanded use the := operator
> # instead of the = operator.
>
> -PREDEFINED = LIBINPUT_ATTRIBUTE_PRINTF(f,a)=
> +PREDEFINED = LIBINPUT_ATTRIBUTE_PRINTF(f, a)= \
> + LIBINPUT_ATTRIBUTE_DEPRECATED
>
> # If the MACRO_EXPANSION and EXPAND_ONLY_PREDEF tags are set to YES then
> # this tag can be used to specify a list of macro names that should be expanded.
> diff --git a/src/libinput.h b/src/libinput.h
> index 54c96e5..4501c66 100644
> --- a/src/libinput.h
> +++ b/src/libinput.h
> @@ -33,6 +33,7 @@ extern "C" {
>
> #define LIBINPUT_ATTRIBUTE_PRINTF(_format, _args) \
> __attribute__ ((format (printf, _format, _args)))
> +#define LIBINPUT_ATTRIBUTE_DEPRECATED __attribute__ ((deprecated))
>
> /**
> * @mainpage
> @@ -784,6 +785,47 @@ struct libinput_interface {
> /**
> * @ingroup base
> *
> + * Create a new libinput context from udev. This context is inactive until
> + * assigned a seat ID with libinput_udev_set_seat().
> + *
> + * @param interface The callback interface
> + * @param user_data Caller-specific data passed to the various callback
> + * interfaces.
> + * @param udev An already initialized udev context
> + *
> + * @return An initialized, but inactive libinput context or NULL on error
> + */
> +struct libinput *
> +libinput_udev_create_context(const struct libinput_interface *interface,
> + void *user_data,
> + struct udev *udev);
> +
> +/**
> + * @ingroup base
> + *
> + * Assign a seat to this libinput context. New devices or the removal of
> + * existing devices will appear as events during libinput_dispatch().
> + *
> + * libinput_udev_set_seat() succeeds even if no input devices are currently
> + * available on this seat, or if devices are available but fail to open in
> + * @ref libinput_interface::open_restricted. Devices that do not have the
> + * minimum capabilities to be recognized as pointer, keyboard or touch
> + * device are ignored. Such devices and those that failed to open
> + * ignored until the next call to libinput_resume().
> + *
> + * @param libinput A libinput context initialized with
> + * libinput_udev_create_context()
> + * @param seat_id A seat identifier. This string must not be NULL.
> + *
> + * @return 0 on success or -1 on failure.
> + */
> +int
> +libinput_udev_set_seat(struct libinput *libinput,
> + const char *seat_id);
> +
> +/**
> + * @ingroup base
> + *
> * Create a new libinput context from udev, for input devices matching
> * the given seat ID. New devices or devices removed will appear as events
> * during libinput_dispatch.
> @@ -803,12 +845,17 @@ struct libinput_interface {
> *
> * @return An initialized libinput context, ready to handle events or NULL on
> * error.
> + *
> + * @deprecated This function was deprecated in 0.4.0 and will be removed
> + * soon. Use libinput_udev_create_context() and libinput_udev_set_seat()
> + * instead.
> */
> struct libinput *
> libinput_udev_create_for_seat(const struct libinput_interface *interface,
> void *user_data,
> struct udev *udev,
> - const char *seat_id);
> + const char *seat_id)
> + LIBINPUT_ATTRIBUTE_DEPRECATED;
>
> /**
> * @ingroup base
> diff --git a/src/udev-seat.c b/src/udev-seat.c
> index 38a13b7..1e1307b 100644
> --- a/src/udev-seat.c
> +++ b/src/udev-seat.c
> @@ -333,14 +333,13 @@ static const struct libinput_interface_backend interface_backend = {
> };
>
> LIBINPUT_EXPORT struct libinput *
> -libinput_udev_create_for_seat(const struct libinput_interface *interface,
> - void *user_data,
> - struct udev *udev,
> - const char *seat_id)
> +libinput_udev_create_context(const struct libinput_interface *interface,
> + void *user_data,
> + struct udev *udev)
> {
> struct udev_input *input;
>
> - if (!interface || !udev || !seat_id)
> + if (!interface || !udev)
> return NULL;
>
> input = zalloc(sizeof *input);
> @@ -354,14 +353,53 @@ libinput_udev_create_for_seat(const struct libinput_interface *interface,
> }
>
> input->udev = udev_ref(udev);
> +
> + return &input->base;
> +}
> +
> +LIBINPUT_EXPORT int
> +libinput_udev_set_seat(struct libinput *libinput,
> + const char *seat_id)
> +{
> + struct udev_input *input = (struct udev_input*)libinput;
> +
> + if (!seat_id)
> + return -1;
> +
> + if (libinput->interface_backend != &interface_backend) {
> + log_bug_client("Mismatching backends.\n");
> + return -1;
> + }
> +
> input->seat_id = strdup(seat_id);
>
> if (udev_input_enable(&input->base) < 0) {
> - udev_unref(udev);
> - libinput_destroy(&input->base);
> free(input);
Leaving the free here is wrong, input is a cast of the libinput parameter,
so your freeing passed in memory here.
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +LIBINPUT_EXPORT struct libinput *
> +libinput_udev_create_for_seat(const struct libinput_interface *interface,
> + void *user_data,
> + struct udev *udev,
> + const char *seat_id)
> +{
> + struct libinput *libinput;
> +
> + if (!interface || !udev || !seat_id)
> + return NULL;
> +
> + libinput = libinput_udev_create_context(interface, user_data, udev);
> + if (!libinput)
> return NULL;
> +
> + if (libinput_udev_set_seat(libinput, seat_id) != 0) {
> + libinput_destroy(libinput);
And then using the passed in (and now free-ed) memory again here.
> + libinput = NULL;
> }
>
> - return &input->base;
> + return libinput;
> }
> diff --git a/test/udev.c b/test/udev.c
> index 6af2cb0..996bc02 100644
> --- a/test/udev.c
> +++ b/test/udev.c
> @@ -52,26 +52,24 @@ START_TEST(udev_create_NULL)
> {
> struct libinput *li;
> const struct libinput_interface interface;
> - struct udev *udev = (struct udev*)0xdeadbeef;
> - const char *seat = (const char*)0xdeaddead;
> + struct udev *udev;
>
> - li = libinput_udev_create_for_seat(NULL, NULL, NULL, NULL);
> - ck_assert(li == NULL);
> + udev = udev_new();
>
> - li = libinput_udev_create_for_seat(&interface, NULL, NULL, NULL);
> - ck_assert(li == NULL);
> - li = libinput_udev_create_for_seat(NULL, NULL, udev, NULL);
> - ck_assert(li == NULL);
> - li = libinput_udev_create_for_seat(NULL, NULL, NULL, seat);
> + li = libinput_udev_create_context(NULL, NULL, NULL);
> ck_assert(li == NULL);
>
> - li = libinput_udev_create_for_seat(&interface, NULL, udev, NULL);
> - ck_assert(li == NULL);
> - li = libinput_udev_create_for_seat(NULL, NULL, udev, seat);
> + li = libinput_udev_create_context(&interface, NULL, NULL);
> ck_assert(li == NULL);
>
> - li = libinput_udev_create_for_seat(&interface, NULL, NULL, seat);
> + li = libinput_udev_create_context(NULL, NULL, udev);
> ck_assert(li == NULL);
> +
> + li = libinput_udev_create_context(&interface, NULL, udev);
> + ck_assert(li != NULL);
> + ck_assert_int_eq(libinput_udev_set_seat(li, NULL), -1);
> + libinput_destroy(li);
> + udev_unref(udev);
> }
> END_TEST
>
> @@ -85,8 +83,9 @@ START_TEST(udev_create_seat0)
> udev = udev_new();
> ck_assert(udev != NULL);
>
> - li = libinput_udev_create_for_seat(&simple_interface, NULL, udev, "seat0");
> + li = libinput_udev_create_context(&simple_interface, NULL, udev);
> ck_assert(li != NULL);
> + ck_assert_int_eq(libinput_udev_set_seat(li, "seat0"), 0);
>
> fd = libinput_get_fd(li);
> ck_assert_int_ge(fd, 0);
> @@ -113,8 +112,9 @@ START_TEST(udev_create_empty_seat)
> ck_assert(udev != NULL);
>
> /* expect a libinput reference, but no events */
> - li = libinput_udev_create_for_seat(&simple_interface, NULL, udev, "seatdoesntexist");
> + li = libinput_udev_create_context(&simple_interface, NULL, udev);
> ck_assert(li != NULL);
> + ck_assert_int_eq(libinput_udev_set_seat(li, "seatdoesntexist"), 0);
>
> fd = libinput_get_fd(li);
> ck_assert_int_ge(fd, 0);
> @@ -147,8 +147,9 @@ START_TEST(udev_added_seat_default)
> udev = udev_new();
> ck_assert(udev != NULL);
>
> - li = libinput_udev_create_for_seat(&simple_interface, NULL, udev, "seat0");
> + li = libinput_udev_create_context(&simple_interface, NULL, udev);
> ck_assert(li != NULL);
> + ck_assert_int_eq(libinput_udev_set_seat(li, "seat0"), 0);
> libinput_dispatch(li);
>
> while (!default_seat_found && (event = libinput_get_event(li))) {
> @@ -184,8 +185,9 @@ START_TEST(udev_double_suspend)
> udev = udev_new();
> ck_assert(udev != NULL);
>
> - li = libinput_udev_create_for_seat(&simple_interface, NULL, udev, "seat0");
> + li = libinput_udev_create_context(&simple_interface, NULL, udev);
> ck_assert(li != NULL);
> + ck_assert_int_eq(libinput_udev_set_seat(li, "seat0"), 0);
>
> fd = libinput_get_fd(li);
> ck_assert_int_ge(fd, 0);
> @@ -215,8 +217,9 @@ START_TEST(udev_double_resume)
> udev = udev_new();
> ck_assert(udev != NULL);
>
> - li = libinput_udev_create_for_seat(&simple_interface, NULL, udev, "seat0");
> + li = libinput_udev_create_context(&simple_interface, NULL, udev);
> ck_assert(li != NULL);
> + ck_assert_int_eq(libinput_udev_set_seat(li, "seat0"), 0);
>
> fd = libinput_get_fd(li);
> ck_assert_int_ge(fd, 0);
> @@ -266,8 +269,9 @@ START_TEST(udev_suspend_resume)
> udev = udev_new();
> ck_assert(udev != NULL);
>
> - li = libinput_udev_create_for_seat(&simple_interface, NULL, udev, "seat0");
> + li = libinput_udev_create_context(&simple_interface, NULL, udev);
> ck_assert(li != NULL);
> + ck_assert_int_eq(libinput_udev_set_seat(li, "seat0"), 0);
>
> fd = libinput_get_fd(li);
> ck_assert_int_ge(fd, 0);
> @@ -305,8 +309,9 @@ START_TEST(udev_device_sysname)
> udev = udev_new();
> ck_assert(udev != NULL);
>
> - li = libinput_udev_create_for_seat(&simple_interface, NULL, udev, "seat0");
> + li = libinput_udev_create_context(&simple_interface, NULL, udev);
> ck_assert(li != NULL);
> + ck_assert_int_eq(libinput_udev_set_seat(li, "seat0"), 0);
>
> libinput_dispatch(li);
>
> @@ -342,8 +347,9 @@ START_TEST(udev_seat_recycle)
> udev = udev_new();
> ck_assert(udev != NULL);
>
> - li = libinput_udev_create_for_seat(&simple_interface, NULL, udev, "seat0");
> + li = libinput_udev_create_context(&simple_interface, NULL, udev);
> ck_assert(li != NULL);
> + ck_assert_int_eq(libinput_udev_set_seat(li, "seat0"), 0);
>
> libinput_dispatch(li);
> while ((ev = libinput_get_event(li))) {
> diff --git a/tools/event-debug.c b/tools/event-debug.c
> index 864f77e..af3f648 100644
> --- a/tools/event-debug.c
> +++ b/tools/event-debug.c
> @@ -140,12 +140,18 @@ open_udev(struct libinput **li)
> return 1;
> }
>
> - *li = libinput_udev_create_for_seat(&interface, NULL, udev, seat);
> + *li = libinput_udev_create_context(&interface, NULL, udev);
> if (!*li) {
> fprintf(stderr, "Failed to initialize context from udev\n");
> return 1;
> }
>
> + if (libinput_udev_set_seat(*li, seat)) {
> + fprintf(stderr, "Failed to set seat\n");
> + libinput_destroy(*li);
> + return 1;
> + }
> +
> return 0;
> }
>
>
With the free() removed this patch is:
Reviewed-by: Hans de Goede <hdegoede at redhat.com>
Regards,
Hans
More information about the wayland-devel
mailing list