Re: [PATCH BlueZ v2 1/2] input/hog: Fix double registering report value callbacks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Luiz,

Sorry I missed your reply before. I think it's a good idea using
notifyid, let me give it a try.

On Thu, Jan 21, 2021 at 4:39 PM Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
>
> Hi Sonny,
>
> On Thu, Jan 21, 2021 at 4:19 PM Sonny Sasaka <sonnysasaka@xxxxxxxxxxxx> wrote:
> >
> > In commit 23b69ab3e484 ("input/hog: Cache the HID report map"), we
> > optimized HOG reconnection by registering report value callbacks early,
> > but there was a bug where we also re-register the same report value
> > callbacks after at CCC write callback. We should handle this case by
> > avoiding the second callback register if we know we have done it
> > earlier.
> >
> > ---
> >  profiles/input/hog-lib.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c
> > index 1f132aa4c..089f42826 100644
> > --- a/profiles/input/hog-lib.c
> > +++ b/profiles/input/hog-lib.c
> > @@ -80,6 +80,7 @@ struct bt_hog {
> >         struct bt_uhid          *uhid;
> >         int                     uhid_fd;
> >         bool                    uhid_created;
> > +       bool                    report_value_cb_registered;
> >         gboolean                has_report_id;
> >         uint16_t                bcdhid;
> >         uint8_t                 bcountrycode;
> > @@ -336,6 +337,13 @@ static void report_ccc_written_cb(guint8 status, const guint8 *pdu,
> >                 return;
> >         }
> >
> > +       /* If we already had the report map cache, we must have registered UHID
> > +        * and the report value callbacks. In that case, don't re-register the
> > +        * report value callbacks here.
> > +        */
> > +       if (hog->report_value_cb_registered)
> > +               return;
> > +
>
> Didn't I comment on this problem before? I do recall suggesting using
> notifyid instead of adding yet another flag.
>
> >         report->notifyid = g_attrib_register(hog->attrib,
> >                                         ATT_OP_HANDLE_NOTIFY,
> >                                         report->value_handle,
> > @@ -1703,6 +1711,8 @@ bool bt_hog_attach(struct bt_hog *hog, void *gatt)
> >                                         report_value_cb, r, NULL);
> >         }
> >
> > +       hog->report_value_cb_registered = true;
> > +
> >         return true;
> >  }
> >
> > @@ -1753,6 +1763,8 @@ void bt_hog_detach(struct bt_hog *hog)
> >                 }
> >         }
> >
> > +       hog->report_value_cb_registered = false;
> > +
> >         if (hog->scpp)
> >                 bt_scpp_detach(hog->scpp);
> >
> > --
> > 2.29.2
> >
>
>
> --
> Luiz Augusto von Dentz



[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux