Re: [PATCH BlueZ v5 4/7] shared/uinput-util: extract uinput utility function from AVCTP

[Luiz Augusto von Dentz <luiz.dentz@gmail.com>]

Hi Pauli,

On Thu, Dec 11, 2025 at 5:50 PM Pauli Virtanen <pav@iki.fi> wrote:
>
> to, 2025-12-11 kello 17:05 -0500, Luiz Augusto von Dentz kirjoitti:
> > Hi Pauli,
> >
> > On Thu, Dec 11, 2025 at 3:16 PM Pauli Virtanen <pav@iki.fi> wrote:
> > >
> > > Extract uinput utility function from AVCTP to src/shared so that it can
> > > be reused for MCS.
> > > ---
> > >  Makefile.am              |   4 +-
> > >  src/shared/uinput-util.c | 191 +++++++++++++++++++++++++++++++++++++++
> > >  src/shared/uinput-util.h |  31 +++++++
> > >  3 files changed, 225 insertions(+), 1 deletion(-)
> > >  create mode 100644 src/shared/uinput-util.c
> > >  create mode 100644 src/shared/uinput-util.h
> > >
> > > diff --git a/Makefile.am b/Makefile.am
> > > index ba0262d5f..4c7177886 100644
> > > --- a/Makefile.am
> > > +++ b/Makefile.am
> > > @@ -247,7 +247,9 @@ shared_sources = src/shared/io.h src/shared/timeout.h \
> > >                         src/shared/lc3.h src/shared/tty.h \
> > >                         src/shared/bap-defs.h \
> > >                         src/shared/asha.h src/shared/asha.c \
> > > -                       src/shared/battery.h src/shared/battery.c
> > > +                       src/shared/battery.h src/shared/battery.c \
> > > +                       src/shared/uinput-util.h \
> > > +                       src/shared/uinput-util.c
> > >
> > >  if READLINE
> > >  shared_sources += src/shared/shell.c src/shared/shell.h
> > > diff --git a/src/shared/uinput-util.c b/src/shared/uinput-util.c
> > > new file mode 100644
> > > index 000000000..4e9644661
> > > --- /dev/null
> > > +++ b/src/shared/uinput-util.c
> > > @@ -0,0 +1,191 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > +/*
> > > + *
> > > + *  BlueZ - Bluetooth protocol stack for Linux
> > > + *
> > > + *  Copyright (C) 2006-2010  Nokia Corporation
> > > + *  Copyright (C) 2004-2010  Marcel Holtmann <marcel@holtmann.org>
> > > + *  Copyright (C) 2011  Texas Instruments, Inc.
> > > + *
> > > + *
> > > + */
> > > +
> > > +#ifdef HAVE_CONFIG_H
> > > +#include <config.h>
> > > +#endif
> > > +
> > > +#include <unistd.h>
> > > +#include <fcntl.h>
> > > +#include <sys/ioctl.h>
> > > +#include <errno.h>
> > > +#include <string.h>
> > > +#include <stdio.h>
> > > +#include <stdarg.h>
> > > +#include <linux/uinput.h>
> > > +
> > > +#include "bluetooth/bluetooth.h"
> > > +
> > > +#include "src/shared/util.h"
> > > +#include "src/shared/uinput-util.h"
> > > +
> > > +
> > > +#define DBG(uinput, fmt, arg...) \
> > > +       uinput_debug(uinput->debug_func, uinput->debug_data, "%s:%s() " fmt, \
> > > +                                               __FILE__, __func__, ## arg)
> > > +
> > > +struct bt_uinput {
> > > +       int fd;
> > > +       bt_uinput_debug_func_t debug_func;
> > > +       void *debug_data;
> > > +};
> > > +
> > > +static void uinput_debug(bt_uinput_debug_func_t debug_func, void *debug_data,
> > > +                                                       const char *format, ...)
> > > +{
> > > +       va_list ap;
> > > +
> > > +       if (!debug_func || !format)
> > > +               return;
> > > +
> > > +       va_start(ap, format);
> > > +       util_debug_va(debug_func, debug_data, format, ap);
> > > +       va_end(ap);
> > > +}
> > > +
> > > +static int send_event(int fd, uint16_t type, uint16_t code, int32_t value)
> > > +{
> > > +       struct input_event event;
> > > +
> > > +       memset(&event, 0, sizeof(event));
> > > +       event.type      = type;
> > > +       event.code      = code;
> > > +       event.value     = value;
> > > +
> > > +       return write(fd, &event, sizeof(event));
> > > +}
> > > +
> > > +void bt_uinput_send_key(struct bt_uinput *uinput, uint16_t key, bool pressed)
> > > +{
> > > +       if (!uinput)
> > > +               return;
> > > +
> > > +       DBG(uinput, "%d", key);
> > > +
> > > +       send_event(uinput->fd, EV_KEY, key, pressed ? 1 : 0);
> > > +       send_event(uinput->fd, EV_SYN, SYN_REPORT, 0);
> > > +}
> > > +
> > > +struct bt_uinput *bt_uinput_new(const char *name, const char *suffix,
> > > +                                       const bdaddr_t *addr,
> > > +                                       const struct input_id *dev_id,
> > > +                                       const struct bt_uinput_key_map *key_map,
> > > +                                       bt_uinput_debug_func_t debug,
> > > +                                       void *user_data)
> > > +{
> > > +       struct bt_uinput *uinput;
> > > +       struct uinput_user_dev dev;
> > > +       int fd, err, i;
> > > +       char src[18];
> > > +
> > > +       uinput = new0(struct bt_uinput, 1);
> > > +       uinput->debug_func = debug;
> > > +       uinput->debug_data = user_data;
> > > +
> > > +       fd = open("/dev/uinput", O_RDWR);
> > > +       if (fd < 0) {
> > > +               fd = open("/dev/input/uinput", O_RDWR);
> > > +               if (fd < 0) {
> > > +                       fd = open("/dev/misc/uinput", O_RDWR);
> > > +                       if (fd < 0) {
> > > +                               err = errno;
> > > +                               DBG(uinput, "Can't open input device: %s (%d)",
> > > +                                                       strerror(err), err);
> > > +                               free(uinput);
> >
> > It is probably worth reordering the uinput allocation so it is after
> > the open, that way we don't need to free on bail out.
>
> This is on purpose for the DBG macro, so I'd not change it.

Just to be able to print that uinput was not be able to be open? I
suspect we want to decouple the opening of the uinput with the device
creation, like it is done in bt_uhid.

>
> > > +                               errno = err;
> > > +                               return NULL;
> > > +                       }
> > > +               }
> > > +       }
> > > +
> > > +       memset(&dev, 0, sizeof(dev));
> > > +
> > > +       if (name)
> > > +               snprintf(dev.name, UINPUT_MAX_NAME_SIZE, "%s", name);
> > > +
> > > +       if (suffix) {
> > > +               int len, slen;
> > > +
> > > +               len = strlen(dev.name);
> > > +               slen = strlen(suffix);
> > > +
> > > +               /* If name + suffix don't fit, truncate the name, then add the
> > > +                * suffix.
> > > +                */
> > > +               if (slen >= UINPUT_MAX_NAME_SIZE)
> > > +                       slen = UINPUT_MAX_NAME_SIZE - 1;
> > > +               if (len > UINPUT_MAX_NAME_SIZE - slen - 1)
> > > +                       len = UINPUT_MAX_NAME_SIZE - slen - 1;
> > > +
> > > +               snprintf(dev.name + len, UINPUT_MAX_NAME_SIZE - len,
> > > +                                                               "%s", suffix);
> > > +       }
> > > +
> > > +       if (dev_id) {
> > > +               dev.id.bustype = dev_id->bustype;
> > > +               dev.id.vendor = dev_id->vendor;
> > > +               dev.id.product = dev_id->product;
> > > +               dev.id.version = dev_id->version;
> > > +       } else {
> > > +               dev.id.bustype = BUS_VIRTUAL;
> > > +       }
> > > +
> > > +       if (write(fd, &dev, sizeof(dev)) < 0) {
> > > +               err = errno;
> > > +               DBG(uinput, "Can't write device information: %s (%d)",
> > > +                                                       strerror(err), err);
> > > +               close(fd);
> > > +               free(uinput);
> > > +               errno = err;
> > > +               return NULL;
> > > +       }
> > > +
> > > +       ioctl(fd, UI_SET_EVBIT, EV_KEY);
> > > +       ioctl(fd, UI_SET_EVBIT, EV_REL);
> > > +       ioctl(fd, UI_SET_EVBIT, EV_REP);
> > > +       ioctl(fd, UI_SET_EVBIT, EV_SYN);
> > > +
> > > +       ba2strlc(addr, src);
> > > +       ioctl(fd, UI_SET_PHYS, src);
> > > +
> > > +       for (i = 0; key_map[i].name != NULL; i++)
> > > +               ioctl(fd, UI_SET_KEYBIT, key_map[i].uinput);
> > > +
> > > +       if (ioctl(fd, UI_DEV_CREATE, NULL) < 0) {
> > > +               err = errno;
> > > +               DBG(uinput, "Can't create uinput device: %s (%d)",
> > > +                                                       strerror(err), err);
> > > +               close(fd);
> > > +               free(uinput);
> > > +               errno = err;
> > > +               return NULL;
> > > +       }
> > > +
> > > +       send_event(fd, EV_REP, REP_DELAY, 300);
> > > +
> > > +       DBG(uinput, "%p", uinput);
> > > +
> > > +       uinput->fd = fd;
> > > +       return uinput;
> > > +}
> > > +
> > > +void bt_uinput_destroy(struct bt_uinput *uinput)
> > > +{
> > > +       if (!uinput)
> > > +               return;
> > > +
> > > +       DBG(uinput, "%p", uinput);
> > > +
> > > +       ioctl(uinput->fd, UI_DEV_DESTROY);
> > > +       close(uinput->fd);
> > > +       free(uinput);
> > > +}
> > > diff --git a/src/shared/uinput-util.h b/src/shared/uinput-util.h
> > > new file mode 100644
> > > index 000000000..fb8f7e6bd
> > > --- /dev/null
> > > +++ b/src/shared/uinput-util.h
> > > @@ -0,0 +1,31 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-later
> >
> > In theory we should only place LGPL code into src/shared, now I see we
> > are copying some code thus it should continue using the same license
> > as the original code, but perhaps it is worth reworking the copied
> > code since it is quite simple and I think it is worth it to not
> > contaminate shared library with GPL.
>
> That, or decide it's small enough to not be copyrightable, given it's
> anyway partly rewritten already.

Yeah, most of the existing code comes from code snips from uinput
documentation anyway and since we are doing many changes to the code I
think it is probably safe to relicense under LGPL.

> > > +/*
> > > + *
> > > + *  BlueZ - Bluetooth protocol stack for Linux
> > > + *
> > > + *  Copyright (C) 2006-2010  Nokia Corporation
> > > + *  Copyright (C) 2004-2010  Marcel Holtmann <marcel@holtmann.org>
> > > + *  Copyright (C) 2011  Texas Instruments, Inc.
> > > + *
> > > + *
> > > + */
> > > +
> > > +struct bt_uinput;
> > > +
> > > +struct bt_uinput_key_map {
> > > +       const char *name;
> > > +       unsigned int code;
> > > +       uint16_t uinput;
> > > +};
> > > +
> > > +typedef void (*bt_uinput_debug_func_t)(const char *str, void *user_data);
> > > +
> > > +struct bt_uinput *bt_uinput_new(const char *name, const char *suffix,
> > > +                                       const bdaddr_t *addr,
> > > +                                       const struct input_id *dev_id,
> > > +                                       const struct bt_uinput_key_map *key_map,
> > > +                                       bt_uinput_debug_func_t debug,
> > > +                                       void *user_data);
> >
> > I'd leave the debug function to be initialized with its own function
> > (e.g. bt_uinput_set_debug).
> >
> > > +void bt_uinput_destroy(struct bt_uinput *uinput);
> > > +
> > > +void bt_uinput_send_key(struct bt_uinput *uinput, uint16_t key, bool pressed);
> > > --
> > > 2.51.1
> > >
> > >
> >
>
> --
> Pauli Virtanen
>
>


-- 
Luiz Augusto von Dentz