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: <20210727195057.Bluez.v1.1.I20397b8350f98567b8d52b895442c768250a6ab3@changeid>
 <CABBYNZJkvaeuMJLKXueoLKq7hXBskxYSPbGujHEj72=qfF4+DQ@mail.gmail.com>
In-Reply-To: <CABBYNZJkvaeuMJLKXueoLKq7hXBskxYSPbGujHEj72=qfF4+DQ@mail.gmail.com>
From:   Yun-hao Chung <howardchung@google.com>
Date:   Wed, 28 Jul 2021 10:19:54 +0800
Message-ID: <CAPHZWUeCGeu+gpYkfhMUKxXGMPZY0ZeKTpHDO-QcCvhwarR0dw@mail.gmail.com>
Subject: Re: [Bluez PATCH v1] gatt-db: fix service in range check
To:     Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc:     "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        Yun-Hao Chung <howardchung@chromium.org>,
        Archie Pusaka <apusaka@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

I see. To be honest, I didn't encounter any issues with this function.
I was trying to solve another problem and found this might be a typo.
Thanks for the explanation.

On Wed, Jul 28, 2021 at 6:18 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Howard,
>
> On Tue, Jul 27, 2021 at 4:53 AM Howard Chung <howardchung@google.com> wrote:
> >
> > From: Yun-Hao Chung <howardchung@chromium.org>
> >
> > If foreach_data->start < svc_start < foreach_data->end < svc_end,
> > foreach_in_range runs foreach_service_in_range to this service.
> >
> > This patch fix the above bug.
> >
> > Reviewed-by: Archie Pusaka <apusaka@chromium.org>
> > ---
> >
> >  src/shared/gatt-db.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
> > index 8bff4d37aaa2..38d93f273a9e 100644
> > --- a/src/shared/gatt-db.c
> > +++ b/src/shared/gatt-db.c
> > @@ -1349,7 +1349,7 @@ static void foreach_in_range(void *data, void *user_data)
> >
> >         if (!foreach_data->attr) {
> >                 if (svc_start < foreach_data->start ||
> > -                                       svc_start > foreach_data->end)
> > +                                               svc_end > foreach_data->end)
> >                         return;
>
> Actually if I recall this check is correct, we want to iterate to
> every service within the range of start to end, if the range is lets
> say 1-9 and the service is 1-14 is still within the range since it
> matches the range of 1-9, or perhaps you want to explain why that
> would be a problem since that is not explained in the patch
> description, if we do switch to strict matching it is probably worth
> documenting that we would only match if the entire range of services
> is within the range, anyway I would still keep svc_start >
> foreach_data->end since that would stop iterating early instead of
> continuing to all the list.
>
> >                 return foreach_service_in_range(data, user_data);
> >         }
> > --
> > 2.32.0.432.gabb21c7263-goog
> >
>
>
> --
> Luiz Augusto von Dentz