linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [Bluez PATCH v1] gatt-db: fix service in range check
@ 2021-07-27 11:51 Howard Chung
  2021-07-27 12:52 ` [Bluez,v1] " bluez.test.bot
  2021-07-27 22:17 ` [Bluez PATCH v1] " Luiz Augusto von Dentz
  0 siblings, 2 replies; 6+ messages in thread
From: Howard Chung @ 2021-07-27 11:51 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Yun-Hao Chung, Archie Pusaka

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;
 		return foreach_service_in_range(data, user_data);
 	}
-- 
2.32.0.432.gabb21c7263-goog


^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [Bluez,v1] gatt-db: fix service in range check
  2021-07-27 11:51 [Bluez PATCH v1] gatt-db: fix service in range check Howard Chung
@ 2021-07-27 12:52 ` bluez.test.bot
  2021-07-27 22:17 ` [Bluez PATCH v1] " Luiz Augusto von Dentz
  1 sibling, 0 replies; 6+ messages in thread
From: bluez.test.bot @ 2021-07-27 12:52 UTC (permalink / raw)
  To: linux-bluetooth, howardchung

[-- Attachment #1: Type: text/plain, Size: 1953 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=521971

---Test result---

Test Summary:
CheckPatch                    PASS      0.37 seconds
GitLint                       PASS      0.14 seconds
Prep - Setup ELL              PASS      48.34 seconds
Build - Prep                  PASS      0.13 seconds
Build - Configure             PASS      8.49 seconds
Build - Make                  PASS      224.26 seconds
Make Check                    PASS      9.09 seconds
Make Distcheck                PASS      255.44 seconds
Build w/ext ELL - Configure   PASS      8.56 seconds
Build w/ext ELL - Make        PASS      201.55 seconds

Details
##############################
Test: CheckPatch - PASS
Desc: Run checkpatch.pl script with rule in .checkpatch.conf

##############################
Test: GitLint - PASS
Desc: Run gitlint with rule in .gitlint

##############################
Test: Prep - Setup ELL - PASS
Desc: Clone, build, and install ELL

##############################
Test: Build - Prep - PASS
Desc: Prepare environment for build

##############################
Test: Build - Configure - PASS
Desc: Configure the BlueZ source tree

##############################
Test: Build - Make - PASS
Desc: Build the BlueZ source tree

##############################
Test: Make Check - PASS
Desc: Run 'make check'

##############################
Test: Make Distcheck - PASS
Desc: Run distcheck to check the distribution

##############################
Test: Build w/ext ELL - Configure - PASS
Desc: Configure BlueZ source with '--enable-external-ell' configuration

##############################
Test: Build w/ext ELL - Make - PASS
Desc: Build BlueZ source with '--enable-external-ell' configuration



---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Bluez PATCH v1] gatt-db: fix service in range check
  2021-07-27 11:51 [Bluez PATCH v1] gatt-db: fix service in range check Howard Chung
  2021-07-27 12:52 ` [Bluez,v1] " bluez.test.bot
@ 2021-07-27 22:17 ` Luiz Augusto von Dentz
  2021-07-28  2:19   ` Yun-hao Chung
  1 sibling, 1 reply; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2021-07-27 22:17 UTC (permalink / raw)
  To: Howard Chung; +Cc: linux-bluetooth, Yun-Hao Chung, Archie Pusaka

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Bluez PATCH v1] gatt-db: fix service in range check
  2021-07-27 22:17 ` [Bluez PATCH v1] " Luiz Augusto von Dentz
@ 2021-07-28  2:19   ` Yun-hao Chung
  2021-07-28  2:51     ` Yun-hao Chung
  0 siblings, 1 reply; 6+ messages in thread
From: Yun-hao Chung @ 2021-07-28  2:19 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth, Yun-Hao Chung, Archie Pusaka

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Bluez PATCH v1] gatt-db: fix service in range check
  2021-07-28  2:19   ` Yun-hao Chung
@ 2021-07-28  2:51     ` Yun-hao Chung
  2021-07-28  3:53       ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 6+ messages in thread
From: Yun-hao Chung @ 2021-07-28  2:51 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth, Yun-Hao Chung, Archie Pusaka

> > >                 if (svc_start < foreach_data->start ||
> > >                                       svc_start > foreach_data->end)
> > >                         return;
After discussing with Archie, if we understand you correctly, we think
this early return can be removed.
Let's say the searched range is 5-9 and the service is 1-14, then we
should consider the service is in range.
If we want to keep the early return, svc_start > foreach_data->end is
already checked a few lines before, so this check is redundant.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Bluez PATCH v1] gatt-db: fix service in range check
  2021-07-28  2:51     ` Yun-hao Chung
@ 2021-07-28  3:53       ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2021-07-28  3:53 UTC (permalink / raw)
  To: Yun-hao Chung; +Cc: linux-bluetooth, Yun-Hao Chung, Archie Pusaka

Hi Howard,

On Tue, Jul 27, 2021 at 7:51 PM Yun-hao Chung <howardchung@google.com> wrote:
>
> > > >                 if (svc_start < foreach_data->start ||
> > > >                                       svc_start > foreach_data->end)
> > > >                         return;
> After discussing with Archie, if we understand you correctly, we think
> this early return can be removed.
> Let's say the searched range is 5-9 and the service is 1-14, then we
> should consider the service is in range.
> If we want to keep the early return, svc_start > foreach_data->end is
> already checked a few lines before, so this check is redundant.

Yep, that seems we have already had the same check so Im fine dropping
the check.

-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-07-28  3:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-27 11:51 [Bluez PATCH v1] gatt-db: fix service in range check Howard Chung
2021-07-27 12:52 ` [Bluez,v1] " bluez.test.bot
2021-07-27 22:17 ` [Bluez PATCH v1] " Luiz Augusto von Dentz
2021-07-28  2:19   ` Yun-hao Chung
2021-07-28  2:51     ` Yun-hao Chung
2021-07-28  3:53       ` Luiz Augusto von Dentz

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox