Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200821043536.15332-1-sonnysasaka@chromium.org>
In-Reply-To: <20200821043536.15332-1-sonnysasaka@chromium.org>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Fri, 21 Aug 2020 10:54:05 -0700
Message-ID: <CABBYNZK7n6dYz_70wrcSG0066HRW=BJwS5tqn2ZB5qGW3qJmYA@mail.gmail.com>
Subject: Re: [PATCH BlueZ] input/device: Unregister all UHID event listeners
 at UHID_DESTROY
To:     Sonny Sasaka <sonnysasaka@chromium.org>
Cc:     "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        Archie Pusaka <apusaka@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Hi Sonny,

On Thu, Aug 20, 2020 at 9:38 PM Sonny Sasaka <sonnysasaka@chromium.org> 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 <apusaka@chromium.org>
>
> ---
>  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