* [PATCH BlueZ] Fix using the attribute struct for encoding ATT pdus
@ 2011-10-06 14:38 Vinicius Costa Gomes
2011-10-06 16:08 ` Johan Hedberg
2011-10-06 18:01 ` Vinicius Costa Gomes
0 siblings, 2 replies; 5+ messages in thread
From: Vinicius Costa Gomes @ 2011-10-06 14:38 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Vinicius Costa Gomes
The enconding and decoding of ATT pdus should be kept as much
free of dependences from other parts of the code as possible, so
it can be used in many contexts.
In this case, for encoding and decoding notifications and indications
we did have to pass an instance of an attribute instead of direct
values.
---
attrib/att.c | 45 +++++++++++++++++++++++++--------------------
attrib/att.h | 9 ++++++---
2 files changed, 31 insertions(+), 23 deletions(-)
diff --git a/attrib/att.c b/attrib/att.c
index c3cbf86..a3a8947 100644
--- a/attrib/att.c
+++ b/attrib/att.c
@@ -820,60 +820,65 @@ struct att_data_list *dec_find_info_resp(const uint8_t *pdu, int len,
return list;
}
-uint16_t enc_notification(struct attribute *a, uint8_t *pdu, int len)
+uint16_t enc_notification(uint16_t handle, uint8_t *value, int vlen,
+ uint8_t *pdu, int len)
{
const uint16_t min_len = sizeof(pdu[0]) + sizeof(uint16_t);
if (pdu == NULL)
return 0;
- if (len < (a->len + min_len))
+ if (len < (vlen + min_len))
return 0;
pdu[0] = ATT_OP_HANDLE_NOTIFY;
- att_put_u16(a->handle, &pdu[1]);
- memcpy(&pdu[3], a->data, a->len);
+ att_put_u16(handle, &pdu[1]);
+ memcpy(&pdu[3], value, vlen);
- return a->len + min_len;
+ return vlen + min_len;
}
-uint16_t enc_indication(struct attribute *a, uint8_t *pdu, int len)
+uint16_t enc_indication(uint16_t handle, uint8_t *value, int vlen,
+ uint8_t *pdu, int len)
{
const uint16_t min_len = sizeof(pdu[0]) + sizeof(uint16_t);
if (pdu == NULL)
return 0;
- if (len < (a->len + min_len))
+ if (len < (vlen + min_len))
return 0;
pdu[0] = ATT_OP_HANDLE_IND;
- att_put_u16(a->handle, &pdu[1]);
- memcpy(&pdu[3], a->data, a->len);
+ att_put_u16(handle, &pdu[1]);
+ memcpy(&pdu[3], value, vlen);
- return a->len + min_len;
+ return vlen + min_len;
}
-struct attribute *dec_indication(const uint8_t *pdu, int len)
+uint16_t dec_indication(const uint8_t *pdu, int len, uint16_t *handle,
+ uint8_t *value, int vlen)
{
const uint16_t min_len = sizeof(pdu[0]) + sizeof(uint16_t);
- struct attribute *a;
+ uint16_t dlen;
if (pdu == NULL)
- return NULL;
+ return 0;
if (pdu[0] != ATT_OP_HANDLE_IND)
- return NULL;
+ return 0;
if (len < min_len)
- return NULL;
+ return 0;
+
+ dlen = MIN(len - min_len, vlen);
+
+ if (handle)
+ *handle = att_get_u16(&pdu[1]);
- a = g_new0(struct attribute, 1);
- a->handle = att_get_u16(&pdu[1]);
- a->len = len - min_len;
- a->data = g_memdup(&pdu[3], a->len);
+ memcpy(value, &pdu[3], dlen);
- return a;
+ return dlen;
}
uint16_t enc_confirmation(uint8_t *pdu, int len)
diff --git a/attrib/att.h b/attrib/att.h
index 3de1726..851e6ba 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -299,9 +299,12 @@ uint16_t enc_find_info_resp(uint8_t format, struct att_data_list *list,
uint8_t *pdu, int len);
struct att_data_list *dec_find_info_resp(const uint8_t *pdu, int len,
uint8_t *format);
-uint16_t enc_notification(struct attribute *a, uint8_t *pdu, int len);
-uint16_t enc_indication(struct attribute *a, uint8_t *pdu, int len);
-struct attribute *dec_indication(const uint8_t *pdu, int len);
+uint16_t enc_notification(uint16_t handle, uint8_t *value, int vlen,
+ uint8_t *pdu, int len);
+uint16_t enc_indication(uint16_t handle, uint8_t *value, int vlen,
+ uint8_t *pdu, int len);
+uint16_t dec_indication(const uint8_t *pdu, int len, uint16_t *handle,
+ uint8_t *value, int vlen);
uint16_t enc_confirmation(uint8_t *pdu, int len);
uint16_t enc_mtu_req(uint16_t mtu, uint8_t *pdu, int len);
--
1.7.6.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH BlueZ] Fix using the attribute struct for encoding ATT pdus
2011-10-06 14:38 [PATCH BlueZ] Fix using the attribute struct for encoding ATT pdus Vinicius Costa Gomes
@ 2011-10-06 16:08 ` Johan Hedberg
2011-10-06 16:24 ` Vinicius Costa Gomes
2011-10-06 18:01 ` Vinicius Costa Gomes
1 sibling, 1 reply; 5+ messages in thread
From: Johan Hedberg @ 2011-10-06 16:08 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth
Hi Vinicius,
On Thu, Oct 06, 2011, Vinicius Costa Gomes wrote:
> The enconding and decoding of ATT pdus should be kept as much
> free of dependences from other parts of the code as possible, so
> it can be used in many contexts.
>
> In this case, for encoding and decoding notifications and indications
> we did have to pass an instance of an attribute instead of direct
> values.
> ---
> attrib/att.c | 45 +++++++++++++++++++++++++--------------------
> attrib/att.h | 9 ++++++---
> 2 files changed, 31 insertions(+), 23 deletions(-)
Did you forget to include the changes to the places where this API is
used? The source tree should remain compilable between each individual
commit (to maintain e.g. bisectability) and that's not happening with
your patch.
Johan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH BlueZ] Fix using the attribute struct for encoding ATT pdus
2011-10-06 16:08 ` Johan Hedberg
@ 2011-10-06 16:24 ` Vinicius Costa Gomes
0 siblings, 0 replies; 5+ messages in thread
From: Vinicius Costa Gomes @ 2011-10-06 16:24 UTC (permalink / raw)
To: linux-bluetooth
Hi Johan,
On 19:08 Thu 06 Oct, Johan Hedberg wrote:
> Hi Vinicius,
>
> On Thu, Oct 06, 2011, Vinicius Costa Gomes wrote:
> > The enconding and decoding of ATT pdus should be kept as much
> > free of dependences from other parts of the code as possible, so
> > it can be used in many contexts.
> >
> > In this case, for encoding and decoding notifications and indications
> > we did have to pass an instance of an attribute instead of direct
> > values.
> > ---
> > attrib/att.c | 45 +++++++++++++++++++++++++--------------------
> > attrib/att.h | 9 ++++++---
> > 2 files changed, 31 insertions(+), 23 deletions(-)
>
> Did you forget to include the changes to the places where this API is
> used? The source tree should remain compilable between each individual
> commit (to maintain e.g. bisectability) and that's not happening with
> your patch.
I messed up with my last rebase, sorry about that. Fixed patch coming
soon.
>
> Johan
Cheers,
--
Vinicius
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH BlueZ] Fix using the attribute struct for encoding ATT pdus
2011-10-06 14:38 [PATCH BlueZ] Fix using the attribute struct for encoding ATT pdus Vinicius Costa Gomes
2011-10-06 16:08 ` Johan Hedberg
@ 2011-10-06 18:01 ` Vinicius Costa Gomes
2011-10-07 20:25 ` Johan Hedberg
1 sibling, 1 reply; 5+ messages in thread
From: Vinicius Costa Gomes @ 2011-10-06 18:01 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Vinicius Costa Gomes
The enconding and decoding of ATT pdus should be kept as much
free of dependences from other parts of the code as possible, so
it can be used in many contexts.
In this case, for encoding and decoding notifications and indications
we did have to pass an instance of an attribute instead of direct
values.
---
attrib/att.c | 45 +++++++++++++++++++++++++--------------------
attrib/att.h | 9 ++++++---
src/attrib-server.c | 6 ++++--
3 files changed, 35 insertions(+), 25 deletions(-)
diff --git a/attrib/att.c b/attrib/att.c
index c3cbf86..a3a8947 100644
--- a/attrib/att.c
+++ b/attrib/att.c
@@ -820,60 +820,65 @@ struct att_data_list *dec_find_info_resp(const uint8_t *pdu, int len,
return list;
}
-uint16_t enc_notification(struct attribute *a, uint8_t *pdu, int len)
+uint16_t enc_notification(uint16_t handle, uint8_t *value, int vlen,
+ uint8_t *pdu, int len)
{
const uint16_t min_len = sizeof(pdu[0]) + sizeof(uint16_t);
if (pdu == NULL)
return 0;
- if (len < (a->len + min_len))
+ if (len < (vlen + min_len))
return 0;
pdu[0] = ATT_OP_HANDLE_NOTIFY;
- att_put_u16(a->handle, &pdu[1]);
- memcpy(&pdu[3], a->data, a->len);
+ att_put_u16(handle, &pdu[1]);
+ memcpy(&pdu[3], value, vlen);
- return a->len + min_len;
+ return vlen + min_len;
}
-uint16_t enc_indication(struct attribute *a, uint8_t *pdu, int len)
+uint16_t enc_indication(uint16_t handle, uint8_t *value, int vlen,
+ uint8_t *pdu, int len)
{
const uint16_t min_len = sizeof(pdu[0]) + sizeof(uint16_t);
if (pdu == NULL)
return 0;
- if (len < (a->len + min_len))
+ if (len < (vlen + min_len))
return 0;
pdu[0] = ATT_OP_HANDLE_IND;
- att_put_u16(a->handle, &pdu[1]);
- memcpy(&pdu[3], a->data, a->len);
+ att_put_u16(handle, &pdu[1]);
+ memcpy(&pdu[3], value, vlen);
- return a->len + min_len;
+ return vlen + min_len;
}
-struct attribute *dec_indication(const uint8_t *pdu, int len)
+uint16_t dec_indication(const uint8_t *pdu, int len, uint16_t *handle,
+ uint8_t *value, int vlen)
{
const uint16_t min_len = sizeof(pdu[0]) + sizeof(uint16_t);
- struct attribute *a;
+ uint16_t dlen;
if (pdu == NULL)
- return NULL;
+ return 0;
if (pdu[0] != ATT_OP_HANDLE_IND)
- return NULL;
+ return 0;
if (len < min_len)
- return NULL;
+ return 0;
+
+ dlen = MIN(len - min_len, vlen);
+
+ if (handle)
+ *handle = att_get_u16(&pdu[1]);
- a = g_new0(struct attribute, 1);
- a->handle = att_get_u16(&pdu[1]);
- a->len = len - min_len;
- a->data = g_memdup(&pdu[3], a->len);
+ memcpy(value, &pdu[3], dlen);
- return a;
+ return dlen;
}
uint16_t enc_confirmation(uint8_t *pdu, int len)
diff --git a/attrib/att.h b/attrib/att.h
index 3de1726..851e6ba 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -299,9 +299,12 @@ uint16_t enc_find_info_resp(uint8_t format, struct att_data_list *list,
uint8_t *pdu, int len);
struct att_data_list *dec_find_info_resp(const uint8_t *pdu, int len,
uint8_t *format);
-uint16_t enc_notification(struct attribute *a, uint8_t *pdu, int len);
-uint16_t enc_indication(struct attribute *a, uint8_t *pdu, int len);
-struct attribute *dec_indication(const uint8_t *pdu, int len);
+uint16_t enc_notification(uint16_t handle, uint8_t *value, int vlen,
+ uint8_t *pdu, int len);
+uint16_t enc_indication(uint16_t handle, uint8_t *value, int vlen,
+ uint8_t *pdu, int len);
+uint16_t dec_indication(const uint8_t *pdu, int len, uint16_t *handle,
+ uint8_t *value, int vlen);
uint16_t enc_confirmation(uint8_t *pdu, int len);
uint16_t enc_mtu_req(uint16_t mtu, uint8_t *pdu, int len);
diff --git a/src/attrib-server.c b/src/attrib-server.c
index 95efd9f..9ba5f74 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -1040,7 +1040,8 @@ static void attrib_notify_clients(struct attribute *attr)
uint8_t pdu[ATT_MAX_MTU];
uint16_t len;
- len = enc_notification(attr, pdu, channel->mtu);
+ len = enc_notification(attr->handle, attr->data,
+ attr->len, pdu, channel->mtu);
if (len == 0)
continue;
@@ -1054,7 +1055,8 @@ static void attrib_notify_clients(struct attribute *attr)
uint8_t pdu[ATT_MAX_MTU];
uint16_t len;
- len = enc_indication(attr, pdu, channel->mtu);
+ len = enc_indication(attr->handle, attr->data,
+ attr->len, pdu, channel->mtu);
if (len == 0)
return;
--
1.7.6.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH BlueZ] Fix using the attribute struct for encoding ATT pdus
2011-10-06 18:01 ` Vinicius Costa Gomes
@ 2011-10-07 20:25 ` Johan Hedberg
0 siblings, 0 replies; 5+ messages in thread
From: Johan Hedberg @ 2011-10-07 20:25 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth
Hi Vinicius,
On Thu, Oct 06, 2011, Vinicius Costa Gomes wrote:
> The enconding and decoding of ATT pdus should be kept as much
> free of dependences from other parts of the code as possible, so
> it can be used in many contexts.
>
> In this case, for encoding and decoding notifications and indications
> we did have to pass an instance of an attribute instead of direct
> values.
> ---
> attrib/att.c | 45 +++++++++++++++++++++++++--------------------
> attrib/att.h | 9 ++++++---
> src/attrib-server.c | 6 ++++--
> 3 files changed, 35 insertions(+), 25 deletions(-)
Applied. Thanks.
Johan
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-10-07 20:25 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-06 14:38 [PATCH BlueZ] Fix using the attribute struct for encoding ATT pdus Vinicius Costa Gomes
2011-10-06 16:08 ` Johan Hedberg
2011-10-06 16:24 ` Vinicius Costa Gomes
2011-10-06 18:01 ` Vinicius Costa Gomes
2011-10-07 20:25 ` Johan Hedberg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).