2020-08-21 04:36:30

by Sonny Sasaka

[permalink] [raw]
Subject: [PATCH BlueZ] input/device: Unregister all UHID event listeners at UHID_DESTROY

When destroying UHID, we should also unregister all event listeners so
that they don't get double registered at reconnection. It fixes a bug
where battery report is not available to kernel after reconnection and
also prevents memory leak.

Tested with Logitech M535 mouse:
* Connect mouse to the device running BlueZ
* cat /sys/class/power_supply/hid-{addr}-battery/capacity # works
* Disconnect mouse
* Reconnect mouse
* cat /sys/class/power_supply/hid-{addr}-battery/capacity # still works

Reviewed-by: Archie Pusaka <[email protected]>

---
profiles/input/device.c | 2 ++
src/shared/uhid.c | 9 +++++++++
src/shared/uhid.h | 1 +
3 files changed, 12 insertions(+)

diff --git a/profiles/input/device.c b/profiles/input/device.c
index 8fc04be37..f6823b7b6 100644
--- a/profiles/input/device.c
+++ b/profiles/input/device.c
@@ -982,6 +982,8 @@ static int uhid_disconnect(struct input_device *idev)
if (!idev->uhid_created)
return 0;

+ bt_uhid_unregister_all(idev->uhid);
+
memset(&ev, 0, sizeof(ev));
ev.type = UHID_DESTROY;

diff --git a/src/shared/uhid.c b/src/shared/uhid.c
index 685f90243..71a4e04ba 100644
--- a/src/shared/uhid.c
+++ b/src/shared/uhid.c
@@ -219,6 +219,15 @@ bool bt_uhid_unregister(struct bt_uhid *uhid, unsigned int id)
return true;
}

+bool bt_uhid_unregister_all(struct bt_uhid *uhid)
+{
+ if (!uhid)
+ return false;
+
+ queue_remove_all(uhid->notify_list, NULL, NULL, free);
+ return true;
+}
+
int bt_uhid_send(struct bt_uhid *uhid, const struct uhid_event *ev)
{
ssize_t len;
diff --git a/src/shared/uhid.h b/src/shared/uhid.h
index 459a24900..dbdca852d 100644
--- a/src/shared/uhid.h
+++ b/src/shared/uhid.h
@@ -40,5 +40,6 @@ typedef void (*bt_uhid_callback_t)(struct uhid_event *ev, void *user_data);
unsigned int bt_uhid_register(struct bt_uhid *uhid, uint32_t event,
bt_uhid_callback_t func, void *user_data);
bool bt_uhid_unregister(struct bt_uhid *uhid, unsigned int id);
+bool bt_uhid_unregister_all(struct bt_uhid *uhid);

int bt_uhid_send(struct bt_uhid *uhid, const struct uhid_event *ev);
--
2.26.2


2020-08-21 17:56:19

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ] input/device: Unregister all UHID event listeners at UHID_DESTROY

Hi Sonny,

On Thu, Aug 20, 2020 at 9:38 PM Sonny Sasaka <[email protected]> wrote:
>
> When destroying UHID, we should also unregister all event listeners so
> that they don't get double registered at reconnection. It fixes a bug
> where battery report is not available to kernel after reconnection and
> also prevents memory leak.
>
> Tested with Logitech M535 mouse:
> * Connect mouse to the device running BlueZ
> * cat /sys/class/power_supply/hid-{addr}-battery/capacity # works
> * Disconnect mouse
> * Reconnect mouse
> * cat /sys/class/power_supply/hid-{addr}-battery/capacity # still works
>
> Reviewed-by: Archie Pusaka <[email protected]>
>
> ---
> profiles/input/device.c | 2 ++
> src/shared/uhid.c | 9 +++++++++
> src/shared/uhid.h | 1 +
> 3 files changed, 12 insertions(+)
>
> diff --git a/profiles/input/device.c b/profiles/input/device.c
> index 8fc04be37..f6823b7b6 100644
> --- a/profiles/input/device.c
> +++ b/profiles/input/device.c
> @@ -982,6 +982,8 @@ static int uhid_disconnect(struct input_device *idev)
> if (!idev->uhid_created)
> return 0;
>
> + bt_uhid_unregister_all(idev->uhid);
> +
> memset(&ev, 0, sizeof(ev));
> ev.type = UHID_DESTROY;
>
> diff --git a/src/shared/uhid.c b/src/shared/uhid.c
> index 685f90243..71a4e04ba 100644
> --- a/src/shared/uhid.c
> +++ b/src/shared/uhid.c
> @@ -219,6 +219,15 @@ bool bt_uhid_unregister(struct bt_uhid *uhid, unsigned int id)
> return true;
> }
>
> +bool bt_uhid_unregister_all(struct bt_uhid *uhid)
> +{
> + if (!uhid)
> + return false;
> +
> + queue_remove_all(uhid->notify_list, NULL, NULL, free);
> + return true;
> +}
> +
> int bt_uhid_send(struct bt_uhid *uhid, const struct uhid_event *ev)
> {
> ssize_t len;
> diff --git a/src/shared/uhid.h b/src/shared/uhid.h
> index 459a24900..dbdca852d 100644
> --- a/src/shared/uhid.h
> +++ b/src/shared/uhid.h
> @@ -40,5 +40,6 @@ typedef void (*bt_uhid_callback_t)(struct uhid_event *ev, void *user_data);
> unsigned int bt_uhid_register(struct bt_uhid *uhid, uint32_t event,
> bt_uhid_callback_t func, void *user_data);
> bool bt_uhid_unregister(struct bt_uhid *uhid, unsigned int id);
> +bool bt_uhid_unregister_all(struct bt_uhid *uhid);
>
> int bt_uhid_send(struct bt_uhid *uhid, const struct uhid_event *ev);
> --
> 2.26.2

Applied, thanks.

--
Luiz Augusto von Dentz