[PATCH libevdev 2/4] Let libevdev manage the /dev/uinput node if required
David Herrmann
dh.herrmann at gmail.com
Thu Aug 15 05:58:34 PDT 2013
Hi
On Thu, Aug 15, 2013 at 6:12 AM, Peter Hutterer
<peter.hutterer at who-t.net> wrote:
> Same API as before, but if the fd is LIBEVDEV_UINPUT_OPEN_MANAGED, libevdev
> opens the uinput node itself. That value is intentionally different to -1, to avoid
> code like the below from accidentally working:
>
> fd = open("/dev/uinput", O_RDWR); /* fails, fd is -1 */
> libevdev_uinput_create_from_device(dev, fd, &uidev); /* may hide the error */
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> libevdev/libevdev-uinput-int.h | 1 +
> libevdev/libevdev-uinput.c | 20 +++++++++++++++
> libevdev/libevdev-uinput.h | 17 ++++++++++---
> test/test-uinput.c | 57 ++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 91 insertions(+), 4 deletions(-)
>
> diff --git a/libevdev/libevdev-uinput-int.h b/libevdev/libevdev-uinput-int.h
> index 70fa1e1..f443984 100644
> --- a/libevdev/libevdev-uinput-int.h
> +++ b/libevdev/libevdev-uinput-int.h
> @@ -23,6 +23,7 @@
>
> struct libevdev_uinput {
> int fd; /**< file descriptor to uinput */
> + int fd_is_managed; /**< do we need to close it? */
> char *name; /**< device name */
> char *syspath; /**< /sys path */
> time_t ctime[2]; /**< before/after UI_DEV_CREATE */
> diff --git a/libevdev/libevdev-uinput.c b/libevdev/libevdev-uinput.c
> index 9fd36b4..830d681 100644
> --- a/libevdev/libevdev-uinput.c
> +++ b/libevdev/libevdev-uinput.c
> @@ -139,6 +139,16 @@ set_props(const struct libevdev *dev, int fd, struct uinput_user_dev *uidev)
> return rc;
> }
>
> +static int
> +open_uinput()
> +{
> + int fd = open("/dev/uinput", O_RDWR);
O_CLOEXEC?
> + if (fd < 0)
> + return -errno;
> +
> + return fd;
> +}
> +
> int
> libevdev_uinput_create_from_device(const struct libevdev *dev, int fd, struct libevdev_uinput** uinput_dev)
> {
> @@ -150,6 +160,14 @@ libevdev_uinput_create_from_device(const struct libevdev *dev, int fd, struct li
> if (!new_device)
> return -ENOMEM;
>
> + if (fd < 0) {
This conflicts with your commit-message. You actually need to compare
to LIBEVDEV_UINPUT_OPEN_MANAGED. And for the case fd < 0 && fd !=
LIBEVDEV_UINPUT_OPEN_MANAGED you should probably return an error.
> + fd = open_uinput();
> + if (fd < 0)
> + return fd;
> +
> + new_device->fd_is_managed = 1;
> + }
> +
> memset(&uidev, 0, sizeof(uidev));
>
> strncpy(uidev.name, libevdev_get_name(dev), UINPUT_MAX_NAME_SIZE - 1);
> @@ -199,6 +217,8 @@ error:
> void libevdev_uinput_destroy(struct libevdev_uinput *uinput_dev)
> {
> ioctl(uinput_dev->fd, UI_DEV_DESTROY, NULL);
> + if (uinput_dev->fd_is_managed)
> + close(uinput_dev->fd);
> free(uinput_dev->syspath);
> free(uinput_dev->name);
> free(uinput_dev);
> diff --git a/libevdev/libevdev-uinput.h b/libevdev/libevdev-uinput.h
> index 9e19ad2..c22c013 100644
> --- a/libevdev/libevdev-uinput.h
> +++ b/libevdev/libevdev-uinput.h
> @@ -99,6 +99,10 @@ struct libevdev_uinput;
> * @endcode
> */
>
> +enum EvdevUInputOpenMode {
> + LIBEVDEV_UINPUT_OPEN_MANAGED = -2,
I would add a short comment here why this is -2. Just copy it from the
commit-message?
> +};
> +
> /**
> * @ingroup uinput
> *
> @@ -106,6 +110,11 @@ struct libevdev_uinput;
> * will be an exact copy of the libevdev device, minus the bits that uinput doesn't
> * allow to be set.
> *
> + * If uinput_fd is LIBEVDEV_UINPUT_OPEN_MANAGED, libevdev_uinput_create_from_device()
> + * will open @c /dev/uinput in read/write mode and manage the file descriptor.
> + * Otherwise, uinput_fd must be opened by the caller and opened with the
> + * appropriate permissions.
> + *
> * The device's lifetime is tied to the uinput file descriptor, closing it will
> * destroy the uinput device. You should call libevdev_uinput_destroy() before
> * closing the file descriptor to free allocated resources.
> @@ -120,9 +129,7 @@ struct libevdev_uinput;
> * source device.
> *
> * @param dev The device to duplicate
> - * @param uinput_fd A file descriptor to @c /dev/uinput, opened with the required
> - * permissions to create a device. This fd may only be used once to create a
> - * uinput device.
> + * @param uinput_fd LIBEVDEV_UINPUT_OPEN_MANAGED or a file descriptor to @c /dev/uinput,
> * @param[out] uinput_dev The newly created libevdev device.
> *
> * @return 0 on success or a negative errno on failure. On failure, the value of
> @@ -139,7 +146,9 @@ int libevdev_uinput_create_from_device(const struct libevdev *dev,
> *
> * Destroy a previously created uinput device and free associated memory.
> *
> - * @note libevdev_uinput_destroy() does not close the fd.
> + * If the device was opened with LIBEVDEV_UINPUT_OPEN_MANAGED, libevdev_uinput_destroy()
> + * also closes the file descriptor. Otherwise, the fd is left as-is and
> + * must be closed by the caller.
> *
> * @param uinput_dev A previously created uinput device.
> *
> diff --git a/test/test-uinput.c b/test/test-uinput.c
> index 9d7fa85..6186552 100644
> --- a/test/test-uinput.c
> +++ b/test/test-uinput.c
> @@ -36,6 +36,62 @@ START_TEST(test_uinput_create_device)
> {
> struct libevdev *dev, *dev2;
> struct libevdev_uinput *uidev;
> + int fd, uinput_fd;
> + unsigned int type, code;
> + int rc;
> + char *devnode;
> +
> + dev = libevdev_new();
> + ck_assert(dev != NULL);
> + libevdev_set_name(dev, TEST_DEVICE_NAME);
> + libevdev_enable_event_type(dev, EV_SYN);
> + libevdev_enable_event_type(dev, EV_REL);
> + libevdev_enable_event_code(dev, EV_REL, REL_X, NULL);
> + libevdev_enable_event_code(dev, EV_REL, REL_Y, NULL);
> +
Maybe add a test-case for -1?
rc = libevdev_uinput_create_from_device(dev, -1, &uidev);
ck_assert_int_lt(rc, 0);
Regards
David
> + rc = libevdev_uinput_create_from_device(dev, LIBEVDEV_UINPUT_OPEN_MANAGED, &uidev);
> + ck_assert_int_eq(rc, 0);
> + ck_assert(uidev != NULL);
> +
> + uinput_fd = libevdev_uinput_get_fd(uidev);
> + ck_assert_int_gt(uinput_fd, -1);
> +
> + devnode = uinput_devnode_from_syspath(libevdev_uinput_get_syspath(uidev));
> + ck_assert(devnode != NULL);
> +
> + fd = open(devnode, O_RDONLY);
> + ck_assert_int_gt(fd, -1);
> + rc = libevdev_new_from_fd(fd, &dev2);
> + ck_assert_int_eq(rc, 0);
> +
> + for (type = 0; type < EV_MAX; type++) {
> + int max = libevdev_get_event_type_max(type);
> + if (max == -1)
> + continue;
> +
> + for (code = 0; code < max; code++) {
> + ck_assert_int_eq(libevdev_has_event_code(dev, type, code),
> + libevdev_has_event_code(dev2, type, code));
> + }
> + }
> +
> + libevdev_free(dev);
> + libevdev_free(dev2);
> + libevdev_uinput_destroy(uidev);
> + close(fd);
> + free(devnode);
> +
> + /* uinput fd is managed, so make sure it did get closed */
> + ck_assert_int_eq(close(uinput_fd), -1);
> + ck_assert_int_eq(errno, EBADF);
> +
> +}
> +END_TEST
> +
> +START_TEST(test_uinput_create_device_from_fd)
> +{
> + struct libevdev *dev, *dev2;
> + struct libevdev_uinput *uidev;
> int fd, fd2;
> unsigned int type, code;
> int rc;
> @@ -257,6 +313,7 @@ uinput_suite(void)
>
> TCase *tc = tcase_create("device creation");
> tcase_add_test(tc, test_uinput_create_device);
> + tcase_add_test(tc, test_uinput_create_device_from_fd);
> tcase_add_test(tc, test_uinput_check_syspath_time);
> tcase_add_test(tc, test_uinput_check_syspath_name);
> suite_add_tcase(s, tc);
> --
> 1.8.2.1
>
> _______________________________________________
> Input-tools mailing list
> Input-tools at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/input-tools
More information about the Input-tools
mailing list