2011-03-16 20:30:11

by Anderson Lizardo

Subject: [PATCH 0/5] Attribute server API fixes

Hi,

This series contains small fixes to the attribute server API. Some fixes are
required for a RFC that I will send soon.

Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

2011-03-17 12:58:32

by Johan Hedberg

Subject: Re: [PATCH 0/5] Attribute server API fixes

Hi Lizardo,

On Wed, Mar 16, 2011, Anderson Lizardo wrote:
> This series contains small fixes to the attribute server API. Some fixes are
> required for a RFC that I will send soon.

Thanks. All five patches have been pushed upstream.

Johan

2011-03-16 20:30:16

by Anderson Lizardo

Subject: [PATCH 5/5] Check for existing handle in attrib_db_add()

This check is necessary to avoid inserting attributes with same handle.
---
src/attrib-server.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index 597a635..74a1c8d 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -1213,10 +1213,12 @@ struct attribute *attrib_db_add(uint16_t handle, bt_uuid_t *uuid, int read_reqs,
int write_reqs, const uint8_t *value, int len)
{
struct attribute *a;
+ guint h = handle;

DBG("handle=0x%04x", handle);

- /* FIXME: handle conflicts */
+ if (g_slist_find_custom(database, GUINT_TO_POINTER(h), handle_cmp))
+ return NULL;

a = g_malloc0(sizeof(struct attribute) + len);
a->handle = handle;
--
1.7.0.4

2011-03-16 20:30:15

by Anderson Lizardo

Subject: [PATCH 4/5] Change attrib_db_update() to return reallocated data

attrib_db_update() uses g_try_realloc(), which means the memory address
of the updated attribute may change. Callers may need to update
references to the old address.

The new struct attribute pointer is returned to caller by the "attr"
paramater.
---
src/attrib-server.c | 9 ++++++---
src/attrib-server.h | 2 +-
2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index e705310..597a635 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -741,7 +741,7 @@ static uint16_t write_value(struct gatt_channel *channel, uint16_t handle,
if (client_attr)
a = client_attr;
else
- attrib_db_update(a->handle, NULL, value, vlen);
+ attrib_db_update(a->handle, NULL, value, vlen, &a);

if (a->write_cb) {
status = a->write_cb(a, a->cb_user_data);
@@ -1232,7 +1232,7 @@ struct attribute *attrib_db_add(uint16_t handle, bt_uuid_t *uuid, int read_reqs,
}

int attrib_db_update(uint16_t handle, bt_uuid_t *uuid, const uint8_t *value,
- int len)
+ int len, struct attribute **attr)
{
struct attribute *a;
GSList *l;
@@ -1256,6 +1256,9 @@ int attrib_db_update(uint16_t handle, bt_uuid_t *uuid, const uint8_t *value,

attrib_notify_clients(a);

+ if (attr)
+ *attr = a;
+
return 0;
}

@@ -1295,5 +1298,5 @@ int attrib_gap_set(uint16_t uuid, const uint8_t *value, int len)
return -ENOSYS;
}

- return attrib_db_update(handle, NULL, value, len);
+ return attrib_db_update(handle, NULL, value, len, NULL);
}
diff --git a/src/attrib-server.h b/src/attrib-server.h
index c03d3c5..38a1f05 100644
--- a/src/attrib-server.h
+++ b/src/attrib-server.h
@@ -28,7 +28,7 @@ void attrib_server_exit(void);
struct attribute *attrib_db_add(uint16_t handle, bt_uuid_t *uuid, int read_reqs,
int write_reqs, const uint8_t *value, int len);
int attrib_db_update(uint16_t handle, bt_uuid_t *uuid, const uint8_t *value,
- int len);
+ int len, struct attribute **attr);
int attrib_db_del(uint16_t handle);
int attrib_gap_set(uint16_t uuid, const uint8_t *value, int len);
uint32_t attrib_create_sdp(uint16_t handle, const char *name);
--
1.7.0.4

2011-03-16 20:30:14

by Anderson Lizardo

Subject: [PATCH 3/5] Remove unnecessary assignment from attrib_db_update()

attrib_db_update() uses g_try_realloc() only to expand/shrink space for
the variable "data" field. Therefore existing fields (like handle) are
guaranteed to remain unchanged.
---
src/attrib-server.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index 98b3fa3..e705310 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -1249,7 +1249,6 @@ int attrib_db_update(uint16_t handle, bt_uuid_t *uuid, const uint8_t *value,
return -ENOMEM;

l->data = a;
- a->handle = handle;
if (uuid != NULL)
memcpy(&a->uuid, uuid, sizeof(bt_uuid_t));
a->len = len;
--
1.7.0.4

2011-03-16 20:30:13

by Anderson Lizardo

Subject: [PATCH 2/5] Allow NULL pointer as UUID for attrib_db_update()

A NULL uuid_t pointer means that the UUID should remain unchanged. This
simplifies most attrib_db_update() calls which do not change the
attribute UUID.
---
src/attrib-server.c | 9 +++------
1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index aab7829..98b3fa3 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -741,7 +741,7 @@ static uint16_t write_value(struct gatt_channel *channel, uint16_t handle,
if (client_attr)
a = client_attr;
else
- attrib_db_update(a->handle, &a->uuid, value, vlen);
+ attrib_db_update(a->handle, NULL, value, vlen);

if (a->write_cb) {
status = a->write_cb(a, a->cb_user_data);
@@ -1250,7 +1250,7 @@ int attrib_db_update(uint16_t handle, bt_uuid_t *uuid, const uint8_t *value,

l->data = a;
a->handle = handle;
- if (uuid != &a->uuid)
+ if (uuid != NULL)
memcpy(&a->uuid, uuid, sizeof(bt_uuid_t));
a->len = len;
memcpy(a->data, value, len);
@@ -1281,13 +1281,10 @@ int attrib_db_del(uint16_t handle)

int attrib_gap_set(uint16_t uuid, const uint8_t *value, int len)
{
- bt_uuid_t u16;
uint16_t handle;

/* FIXME: Missing Privacy and Reconnection Address */

- bt_uuid16_create(&u16, uuid);
-
switch (uuid) {
case GATT_CHARAC_DEVICE_NAME:
handle = name_handle;
@@ -1299,5 +1296,5 @@ int attrib_gap_set(uint16_t uuid, const uint8_t *value, int len)
return -ENOSYS;
}

- return attrib_db_update(handle, &u16, value, len);
+ return attrib_db_update(handle, NULL, value, len);
}
--
1.7.0.4

2011-03-16 20:30:12

by Anderson Lizardo

Subject: [PATCH 1/5] Add DBG() calls to attrib_db_* functions

This will help debugging issues with registration of GATT services.
---
src/attrib-server.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index b076d98..aab7829 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -1214,6 +1214,8 @@ struct attribute *attrib_db_add(uint16_t handle, bt_uuid_t *uuid, int read_reqs,
{
struct attribute *a;

+ DBG("handle=0x%04x", handle);
+
/* FIXME: handle conflicts */

a = g_malloc0(sizeof(struct attribute) + len);
@@ -1236,6 +1238,8 @@ int attrib_db_update(uint16_t handle, bt_uuid_t *uuid, const uint8_t *value,
GSList *l;
guint h = handle;

+ DBG("handle=0x%04x", handle);
+
l = g_slist_find_custom(database, GUINT_TO_POINTER(h), handle_cmp);
if (!l)
return -ENOENT;
@@ -1262,6 +1266,8 @@ int attrib_db_del(uint16_t handle)
GSList *l;
guint h = handle;

+ DBG("handle=0x%04x", handle);
+
l = g_slist_find_custom(database, GUINT_TO_POINTER(h), handle_cmp);
if (!l)
return -ENOENT;
--
1.7.0.4