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>
In-Reply-To: <20210727195057.Bluez.v1.1.I20397b8350f98567b8d52b895442c768250a6ab3@changeid>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Tue, 27 Jul 2021 15:17:48 -0700
Message-ID: <CABBYNZJkvaeuMJLKXueoLKq7hXBskxYSPbGujHEj72=qfF4+DQ@mail.gmail.com>
Subject: Re: [Bluez PATCH v1] gatt-db: fix service in range check
To:     Howard Chung <howardchung@google.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

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