From: Yun-Hao Chung <[email protected]>
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 <[email protected]>
---
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
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
Hi Howard,
On Tue, Jul 27, 2021 at 4:53 AM Howard Chung <[email protected]> wrote:
>
> From: Yun-Hao Chung <[email protected]>
>
> 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 <[email protected]>
> ---
>
> 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
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
<[email protected]> wrote:
>
> Hi Howard,
>
> On Tue, Jul 27, 2021 at 4:53 AM Howard Chung <[email protected]> wrote:
> >
> > From: Yun-Hao Chung <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> >
> > 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
> > > 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.
Hi Howard,
On Tue, Jul 27, 2021 at 7:51 PM Yun-hao Chung <[email protected]> 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