2015-01-30 05:46:07

by Lukasz Rymanowski

Subject: [PATCH 1/2] android/gatt: Fix Find By Type request

With this patch GATT will correctly response with attribute not found if
there is no attributes matching.

Issue found and fix tested on UPF50
---
android/gatt.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/android/gatt.c b/android/gatt.c
index 466c1ea..3b0b81d 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -6258,6 +6258,7 @@ struct find_by_type_request_data {
uint8_t *search_value;
size_t search_vlen;
uint8_t error;
+ bool found;
};

static void find_by_type_request_cb(struct gatt_db_attribute *attrib,
@@ -6289,6 +6290,9 @@ static void find_by_type_request_cb(struct gatt_db_attribute *attrib,
find_data->search_vlen);

queue_push_tail(find_data->device->pending_requests, request_data);
+
+ if (!find_data->found)
+ find_data->found = true;
}

static uint8_t find_by_type_request(const uint8_t *cmd, uint16_t cmd_len,
@@ -6315,11 +6319,12 @@ static uint8_t find_by_type_request(const uint8_t *cmd, uint16_t cmd_len,
data.search_vlen = search_vlen;
data.search_value = search_value;
data.device = device;
+ data.found = false;

gatt_db_find_by_type(gatt_db, start, end, &uuid,
find_by_type_request_cb, &data);

- if (data.error == ATT_ECODE_ATTR_NOT_FOUND) {
+ if (!data.found) {
size_t mtu;
uint8_t *rsp = g_attrib_get_buffer(device->attrib, &mtu);

--
1.8.4

2015-01-30 05:46:08

by Lukasz Rymanowski

Subject: [PATCH 2/2] android/gatt: Add return check for queue push

---
android/gatt.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/android/gatt.c b/android/gatt.c
index 3b0b81d..0f4136e 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -6289,7 +6289,12 @@ static void find_by_type_request_cb(struct gatt_db_attribute *attrib,
memcpy(request_data->filter_value, find_data->search_value,
find_data->search_vlen);

- queue_push_tail(find_data->device->pending_requests, request_data);
+ if (!queue_push_tail(find_data->device->pending_requests,
+ request_data)) {
+ destroy_pending_request(request_data);
+ find_data->error = ATT_ECODE_INSUFF_RESOURCES;
+ return;
+ }

if (!find_data->found)
find_data->found = true;
--
1.8.4

2015-02-05 10:58:59

by Lukasz Rymanowski

Subject: Re: [PATCH 1/2] android/gatt: Fix Find By Type request

Hi Jakub,

On Wed, Feb 4, 2015 at 10:30 AM, Jakub Tyszkowski <[email protected]> wrote:
> Hi Lukasz,
>
> 2015-01-30 6:46 GMT+01:00 Lukasz Rymanowski <[email protected]>:
>> With this patch GATT will correctly response with attribute not found if
>> there is no attributes matching.
>>
>> Issue found and fix tested on UPF50
>> ---
>> android/gatt.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/android/gatt.c b/android/gatt.c
>> index 466c1ea..3b0b81d 100644
>> --- a/android/gatt.c
>> +++ b/android/gatt.c
>> @@ -6258,6 +6258,7 @@ struct find_by_type_request_data {
>> uint8_t *search_value;
>> size_t search_vlen;
>> uint8_t error;
>> + bool found;
>> };
>>
>> static void find_by_type_request_cb(struct gatt_db_attribute *attrib,
>> @@ -6289,6 +6290,9 @@ static void find_by_type_request_cb(struct gatt_db_attribute *attrib,
>> find_data->search_vlen);
>>
>> queue_push_tail(find_data->device->pending_requests, request_data);
>> +
>> + if (!find_data->found)
>> + find_data->found = true;
>> }
>>
>> static uint8_t find_by_type_request(const uint8_t *cmd, uint16_t cmd_len,
>> @@ -6315,11 +6319,12 @@ static uint8_t find_by_type_request(const uint8_t *cmd, uint16_t cmd_len,
>> data.search_vlen = search_vlen;
>> data.search_value = search_value;
>> data.device = device;
>> + data.found = false;
>>
>> gatt_db_find_by_type(gatt_db, start, end, &uuid,
>> find_by_type_request_cb, &data);
>>
>> - if (data.error == ATT_ECODE_ATTR_NOT_FOUND) {
>> + if (!data.found) {
>> size_t mtu;
>> uint8_t *rsp = g_attrib_get_buffer(device->attrib, &mtu);
>>
>> --
>> 1.8.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
> Maybe this issue can be solved like this:
>
> @@ -6274,17 +6274,9 @@ static uint8_t find_by_type_request(const
> uint8_t *cmd, uint16_t cmd_len,
> gatt_db_find_by_type(gatt_db, start, end, &uuid,
> find_by_type_request_cb, &data);
>
> - if (data.error == ATT_ECODE_ATTR_NOT_FOUND) {
> - size_t mtu;
> - uint8_t *rsp = g_attrib_get_buffer(device->attrib, &mtu);
> -
> - len = enc_error_resp(ATT_OP_FIND_BY_TYPE_REQ, start,
> - ATT_ECODE_ATTR_NOT_FOUND, rsp, mtu);
> - g_attrib_send(device->attrib, 0, rsp, len, NULL, NULL, NULL);
> - return 0;
> - }
> -
> - if (!data.error)
> + if (queue_isempty(device->pending_requests))
> + data.error = ATT_ECODE_ATTR_NOT_FOUND;
> + else
> process_dev_pending_requests(device, ATT_OP_FIND_BY_TYPE_REQ);
>
> return data.error;
>
>

I considered that but this is still not a clear solution for me.
I think rather to make gatt_db_find_by_type() return number of found attributes.

\Lukasz
> Regards,
> Jakub Tyszkowski
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2015-02-04 09:30:36

by Jakub Tyszkowski

Subject: Re: [PATCH 1/2] android/gatt: Fix Find By Type request

Hi Lukasz,

2015-01-30 6:46 GMT+01:00 Lukasz Rymanowski <[email protected]>:
> With this patch GATT will correctly response with attribute not found if
> there is no attributes matching.
>
> Issue found and fix tested on UPF50
> ---
> android/gatt.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/android/gatt.c b/android/gatt.c
> index 466c1ea..3b0b81d 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -6258,6 +6258,7 @@ struct find_by_type_request_data {
> uint8_t *search_value;
> size_t search_vlen;
> uint8_t error;
> + bool found;
> };
>
> static void find_by_type_request_cb(struct gatt_db_attribute *attrib,
> @@ -6289,6 +6290,9 @@ static void find_by_type_request_cb(struct gatt_db_attribute *attrib,
> find_data->search_vlen);
>
> queue_push_tail(find_data->device->pending_requests, request_data);
> +
> + if (!find_data->found)
> + find_data->found = true;
> }
>
> static uint8_t find_by_type_request(const uint8_t *cmd, uint16_t cmd_len,
> @@ -6315,11 +6319,12 @@ static uint8_t find_by_type_request(const uint8_t *cmd, uint16_t cmd_len,
> data.search_vlen = search_vlen;
> data.search_value = search_value;
> data.device = device;
> + data.found = false;
>
> gatt_db_find_by_type(gatt_db, start, end, &uuid,
> find_by_type_request_cb, &data);
>
> - if (data.error == ATT_ECODE_ATTR_NOT_FOUND) {
> + if (!data.found) {
> size_t mtu;
> uint8_t *rsp = g_attrib_get_buffer(device->attrib, &mtu);
>
> --
> 1.8.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


Maybe this issue can be solved like this:

@@ -6274,17 +6274,9 @@ static uint8_t find_by_type_request(const
uint8_t *cmd, uint16_t cmd_len,
gatt_db_find_by_type(gatt_db, start, end, &uuid,
find_by_type_request_cb, &data);

- if (data.error == ATT_ECODE_ATTR_NOT_FOUND) {
- size_t mtu;
- uint8_t *rsp = g_attrib_get_buffer(device->attrib, &mtu);
-
- len = enc_error_resp(ATT_OP_FIND_BY_TYPE_REQ, start,
- ATT_ECODE_ATTR_NOT_FOUND, rsp, mtu);
- g_attrib_send(device->attrib, 0, rsp, len, NULL, NULL, NULL);
- return 0;
- }
-
- if (!data.error)
+ if (queue_isempty(device->pending_requests))
+ data.error = ATT_ECODE_ATTR_NOT_FOUND;
+ else
process_dev_pending_requests(device, ATT_OP_FIND_BY_TYPE_REQ);

return data.error;


Regards,
Jakub Tyszkowski

2015-02-02 11:40:48

by Lukasz Rymanowski

Subject: Re: [PATCH 1/2] android/gatt: Fix Find By Type request

Hi,

On 30 January 2015 at 06:46, Lukasz Rymanowski
<[email protected]> wrote:
> With this patch GATT will correctly response with attribute not found if
> there is no attributes matching.
>
> Issue found and fix tested on UPF50
> ---
> android/gatt.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/android/gatt.c b/android/gatt.c
> index 466c1ea..3b0b81d 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -6258,6 +6258,7 @@ struct find_by_type_request_data {
> uint8_t *search_value;
> size_t search_vlen;
> uint8_t error;
> + bool found;
> };
>
> static void find_by_type_request_cb(struct gatt_db_attribute *attrib,
> @@ -6289,6 +6290,9 @@ static void find_by_type_request_cb(struct gatt_db_attribute *attrib,
> find_data->search_vlen);
>
> queue_push_tail(find_data->device->pending_requests, request_data);
> +
> + if (!find_data->found)
> + find_data->found = true;
> }
>
> static uint8_t find_by_type_request(const uint8_t *cmd, uint16_t cmd_len,
> @@ -6315,11 +6319,12 @@ static uint8_t find_by_type_request(const uint8_t *cmd, uint16_t cmd_len,
> data.search_vlen = search_vlen;
> data.search_value = search_value;
> data.device = device;
> + data.found = false;
>
> gatt_db_find_by_type(gatt_db, start, end, &uuid,
> find_by_type_request_cb, &data);
>
> - if (data.error == ATT_ECODE_ATTR_NOT_FOUND) {
> + if (!data.found) {
> size_t mtu;
> uint8_t *rsp = g_attrib_get_buffer(device->attrib, &mtu);

Ehen think longer about it, maybe we should move the fix somehow to
gatt-db. E.x. find_by_type_request_cb would be called with *attrib ==
NULL when there is any matching attribute.

Any comments?

\Łukasz

>
> --
> 1.8.4
>