2013-03-01 23:22:23

by Vinicius Costa Gomes

[permalink] [raw]
Subject: [RFC BlueZ v0 00/10] Service Discovery/GATT cleanup

Hi,

I know that this series is somewhat disorganized, but I am sending it
in its current form to get some input if the problems that it tries to
solve are really problems.

And suggestions about how to better organize it are very welcome.

This series started with the aim to solve the crashes that were
happening when a GATT Service Discovery was interrupted (device going
out of range, misbehaved device, etc). Those crashes were solved for
all the cases that I could test.

And it caused me to take a critical look at how Service Discovery
(including SDP) is done inside bluetoothd. It seems the code tries to
solve a few problems that aren't applicable anymore, one example is
PATCH 10/10. Can anyone remember anything else that can be trimmed?

Cheers,


Vinicius Costa Gomes (10):
device: Clean up the ATT Connection attempt when it is cancelled
device: Separate SDP browse from GATT discover primary services
device: Fix removing profiles
device: Remove misleading function
device: Use only one entry point for LE connections
gatt: Handle errors when g_attrib_send() in gatt_discover_primary()
gatt: Track the lifetime of Primary Discovery requests
gatt: Better lifetime tracking for Find Included services
gatt: Add lifetime tracking for Discover Characteristics requests
device: Remove detection of removed services during SDP browse

attrib/gatt.c | 234 ++++++++++++++++++++-----------
src/device.c | 434 +++++++++++++++++++++++++---------------------------------
2 files changed, 342 insertions(+), 326 deletions(-)

--
1.8.1.3



2013-03-02 01:50:19

by Anderson Lizardo

[permalink] [raw]
Subject: Re: [RFC BlueZ v0 04/10] device: Remove misleading function

Hi Vinicius,

On Fri, Mar 1, 2013 at 7:22 PM, Vinicius Costa Gomes
<[email protected]> wrote:
> @@ -3049,7 +3044,9 @@ static void register_all_services(struct btd_device *device, GSList *services)
> g_slist_free_full(device->primaries, g_free);
> device->primaries = NULL;

I think you can remove the NULL assignment above, as device->primaries
is updated just below.

>
> - device_register_primaries(device, g_slist_copy(services), -1);
> + /* And keep the new one */
> + device->primaries = g_slist_copy(services);
> +
> if (removed)
> device_remove_profiles(device, removed);

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

2013-03-01 23:22:33

by Vinicius Costa Gomes

[permalink] [raw]
Subject: [RFC BlueZ v0 10/10] device: Remove detection of removed services during SDP browse

Right now, the remote device services are discovered only once,
when it is created, and then this list is not expected to change,
neither the user has any way to update it.

The only[1] situation that a device may "lose" services is when it
supports GATT and sends the Service Changed notification, which
could mean that the device lost support for some services.

[1] In theory, it could also happen between doing a Device.Connect()
and Device.Pair(), because Pair() triggers another service discovery.
---
src/device.c | 33 +++------------------------------
1 file changed, 3 insertions(+), 30 deletions(-)

diff --git a/src/device.c b/src/device.c
index f2abcff..5ee84a3 100644
--- a/src/device.c
+++ b/src/device.c
@@ -109,7 +109,6 @@ struct browse_req {
struct btd_device *device;
GSList *match_uuids;
GSList *profiles_added;
- GSList *profiles_removed;
sdp_list_t *records;
int search_uuid;
int reconnect_attempt;
@@ -349,7 +348,6 @@ static void browse_request_free(struct browse_req *req)
if (req->device)
btd_device_unref(req->device);
g_slist_free_full(req->profiles_added, g_free);
- g_slist_free(req->profiles_removed);
if (req->records)
sdp_list_free(req->records, (sdp_free_func_t) sdp_record_free);

@@ -2667,16 +2665,12 @@ static void update_bredr_services(struct browse_req *req, sdp_list_t *recs)

l = g_slist_find_custom(device->uuids, profile_uuid,
(GCompareFunc) strcmp);
- if (!l)
+ if (l == NULL)
req->profiles_added =
g_slist_append(req->profiles_added,
profile_uuid);
- else {
- req->profiles_removed =
- g_slist_remove(req->profiles_removed,
- l->data);
+ else
g_free(profile_uuid);
- }

sdp_list_free(svcclass, free);
}
@@ -2805,7 +2799,7 @@ static void search_cb(sdp_list_t *recs, int err, gpointer user_data)
device->tmp_records = req->records;
req->records = NULL;

- if (!req->profiles_added && !req->profiles_removed) {
+ if (!req->profiles_added) {
DBG("%s: No service update", addr);
goto send_reply;
}
@@ -2822,10 +2816,6 @@ static void search_cb(sdp_list_t *recs, int err, gpointer user_data)
device_probe_profiles(device, req->profiles_added);
}

- /* Remove profiles for services removed */
- if (req->profiles_removed)
- device_remove_profiles(device, req->profiles_removed);
-
/* Propagate services changes */
g_dbus_emit_property_changed(dbus_conn, req->device->path,
DEVICE_INTERFACE, "UUIDs");
@@ -2878,22 +2868,6 @@ done:
search_cb(recs, err, user_data);
}

-static void init_browse(struct browse_req *req, gboolean reverse)
-{
- GSList *l;
-
- /* If we are doing reverse-SDP don't try to detect removed profiles
- * since some devices hide their service records while they are
- * connected
- */
- if (reverse)
- return;
-
- for (l = req->device->uuids; l; l = l->next)
- req->profiles_removed = g_slist_append(req->profiles_removed,
- l->data);
-}
-
static void store_services(struct btd_device *device)
{
struct btd_adapter *adapter = device->adapter;
@@ -3357,7 +3331,6 @@ static int device_browse_sdp(struct btd_device *device, DBusMessage *msg,
req = g_new0(struct browse_req, 1);
req->device = btd_device_ref(device);
sdp_uuid16_create(&uuid, uuid_list[req->search_uuid++]);
- init_browse(req, reverse);

err = bt_search_service(adapter_get_address(adapter), &device->bdaddr,
&uuid, browse_cb, req, NULL);
--
1.8.1.3


2013-03-01 23:22:32

by Vinicius Costa Gomes

[permalink] [raw]
Subject: [RFC BlueZ v0 09/10] gatt: Add lifetime tracking for Discover Characteristics requests

Use the GDestroyNotify facility from g_attrib_send() to properly
end a procedure.
---
attrib/gatt.c | 64 ++++++++++++++++++++++++++++++++++++++++-------------------
1 file changed, 44 insertions(+), 20 deletions(-)

diff --git a/attrib/gatt.c b/attrib/gatt.c
index ef3ffff..7091d9f 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -65,9 +65,11 @@ struct included_uuid_query {

struct discover_char {
GAttrib *attrib;
+ int refs;
bt_uuid_t *uuid;
uint16_t end;
GSList *characteristics;
+ int err;
gatt_cb_t cb;
void *user_data;
};
@@ -121,8 +123,20 @@ static void isd_unref(struct included_discovery *isd)
g_free(isd);
}

-static void discover_char_free(struct discover_char *dc)
+static struct discover_char *discover_char_ref(struct discover_char *dc)
+{
+ g_atomic_int_inc(&dc->refs);
+
+ return dc;
+}
+
+static void discover_char_unref(struct discover_char *dc)
{
+ if (g_atomic_int_dec_and_test(&dc->refs) == FALSE)
+ return;
+
+ dc->cb(dc->characteristics, dc->err, dc->user_data);
+
g_slist_free_full(dc->characteristics, g_free);
g_attrib_unref(dc->attrib);
g_free(dc->uuid);
@@ -494,12 +508,19 @@ unsigned int gatt_find_included(GAttrib *attrib, uint16_t start, uint16_t end,
return find_included(isd, start);
}

+static void discover_char_drop(gpointer user_data)
+{
+ struct discover_char *dc = user_data;
+
+ discover_char_unref(dc);
+}
+
static void char_discovered_cb(guint8 status, const guint8 *ipdu, guint16 iplen,
gpointer user_data)
{
struct discover_char *dc = user_data;
struct att_data_list *list;
- unsigned int i, err = ATT_ECODE_ATTR_NOT_FOUND;
+ unsigned int i;
size_t buflen;
uint8_t *buf;
guint16 oplen;
@@ -507,14 +528,14 @@ static void char_discovered_cb(guint8 status, const guint8 *ipdu, guint16 iplen,
uint16_t last = 0;

if (status) {
- err = status;
- goto done;
+ set_error(&dc->err, status);
+ return;
}

list = dec_read_by_type_resp(ipdu, iplen);
if (list == NULL) {
- err = ATT_ECODE_IO;
- goto done;
+ set_error(&dc->err, ATT_ECODE_IO);
+ return;
}

for (i = 0; i < list->num; i++) {
@@ -535,8 +556,8 @@ static void char_discovered_cb(guint8 status, const guint8 *ipdu, guint16 iplen,

chars = g_try_new0(struct gatt_char, 1);
if (!chars) {
- err = ATT_ECODE_INSUFF_RESOURCES;
- goto done;
+ set_error(&dc->err, ATT_ECODE_INSUFF_RESOURCES);
+ return;
}

chars->handle = last;
@@ -560,17 +581,13 @@ static void char_discovered_cb(guint8 status, const guint8 *ipdu, guint16 iplen,
if (oplen == 0)
return;

- g_attrib_send(dc->attrib, 0, buf, oplen, char_discovered_cb,
- dc, NULL);
-
- return;
+ if (g_attrib_send(dc->attrib, 0, buf, oplen, char_discovered_cb,
+ discover_char_ref(dc),
+ discover_char_drop) == 0) {
+ set_error(&dc->err, ATT_ECODE_ABORTED);
+ discover_char_unref(dc);
+ }
}
-
-done:
- err = (dc->characteristics ? 0 : err);
-
- dc->cb(dc->characteristics, err, dc->user_data);
- discover_char_free(dc);
}

guint gatt_discover_char(GAttrib *attrib, uint16_t start, uint16_t end,
@@ -582,6 +599,7 @@ guint gatt_discover_char(GAttrib *attrib, uint16_t start, uint16_t end,
struct discover_char *dc;
bt_uuid_t type_uuid;
guint16 plen;
+ int id;

bt_uuid16_create(&type_uuid, GATT_CHARAC_UUID);

@@ -599,8 +617,14 @@ guint gatt_discover_char(GAttrib *attrib, uint16_t start, uint16_t end,
dc->end = end;
dc->uuid = g_memdup(uuid, sizeof(bt_uuid_t));

- return g_attrib_send(attrib, 0, buf, plen, char_discovered_cb,
- dc, NULL);
+ id = g_attrib_send(attrib, 0, buf, plen, char_discovered_cb,
+ discover_char_ref(dc), discover_char_drop);
+ if (id == 0) {
+ g_attrib_unref(dc->attrib);
+ g_free(dc->uuid);
+ }
+
+ return id;
}

guint gatt_read_char_by_uuid(GAttrib *attrib, uint16_t start, uint16_t end,
--
1.8.1.3


2013-03-01 23:22:30

by Vinicius Costa Gomes

[permalink] [raw]
Subject: [RFC BlueZ v0 07/10] gatt: Track the lifetime of Primary Discovery requests

Use the GDestroyNotify facility from g_attrib_send() to properly
end a procedure.
---
attrib/gatt.c | 73 ++++++++++++++++++++++++++++++++---------------------------
1 file changed, 40 insertions(+), 33 deletions(-)

diff --git a/attrib/gatt.c b/attrib/gatt.c
index 1f6360f..b451917 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -165,6 +165,13 @@ static guint16 encode_discover_primary(uint16_t start, uint16_t end,
return plen;
}

+static void discover_primary_drop(gpointer user_data)
+{
+ struct discover_primary *dp = user_data;
+
+ discover_primary_unref(dp);
+}
+
static void primary_by_uuid_cb(guint8 status, const guint8 *ipdu,
guint16 iplen, gpointer user_data)

@@ -179,12 +186,15 @@ static void primary_by_uuid_cb(guint8 status, const guint8 *ipdu,

if (status) {
err = status == ATT_ECODE_ATTR_NOT_FOUND ? 0 : status;
- goto done;
+ set_error(&dp->err, err);
+ return;
}

ranges = dec_find_by_type_resp(ipdu, iplen);
- if (ranges == NULL)
- goto done;
+ if (ranges == NULL) {
+ set_error(&dp->err, ATT_ECODE_IO);
+ return;
+ }

dp->primaries = g_slist_concat(dp->primaries, ranges);

@@ -192,26 +202,20 @@ static void primary_by_uuid_cb(guint8 status, const guint8 *ipdu,
range = last->data;

if (range->end == 0xffff)
- goto done;
+ return;

buf = g_attrib_get_buffer(dp->attrib, &buflen);
oplen = encode_discover_primary(range->end + 1, 0xffff, &dp->uuid,
buf, buflen);
-
if (oplen == 0)
- goto done;
+ return;

if (g_attrib_send(dp->attrib, 0, buf, oplen, primary_by_uuid_cb,
- discover_primary_ref(dp), NULL) == 0) {
- err = ATT_ECODE_ABORTED;
- goto done;
+ discover_primary_ref(dp),
+ discover_primary_drop) == 0) {
+ set_error(&dp->err, ATT_ECODE_ABORTED);
+ discover_primary_unref(dp);
}
-
- return;
-
-done:
- set_error(&dp->err, err);
- discover_primary_unref(dp);
}

static void primary_all_cb(guint8 status, const guint8 *ipdu, guint16 iplen,
@@ -219,18 +223,19 @@ static void primary_all_cb(guint8 status, const guint8 *ipdu, guint16 iplen,
{
struct discover_primary *dp = user_data;
struct att_data_list *list;
- unsigned int i, err;
+ unsigned int i;
uint16_t start, end;

if (status) {
- err = status == ATT_ECODE_ATTR_NOT_FOUND ? 0 : status;
- goto done;
+ int err = status == ATT_ECODE_ATTR_NOT_FOUND ? 0 : status;
+ set_error(&dp->err, err);
+ return;
}

list = dec_read_by_grp_resp(ipdu, iplen);
if (list == NULL) {
- err = ATT_ECODE_IO;
- goto done;
+ set_error(&dp->err, ATT_ECODE_IO);
+ return;
}

for (i = 0, end = 0; i < list->num; i++) {
@@ -253,8 +258,8 @@ static void primary_all_cb(guint8 status, const guint8 *ipdu, guint16 iplen,

primary = g_try_new0(struct gatt_primary, 1);
if (!primary) {
- err = ATT_ECODE_INSUFF_RESOURCES;
- goto done;
+ set_error(&dp->err, ATT_ECODE_INSUFF_RESOURCES);
+ return;
}
primary->range.start = start;
primary->range.end = end;
@@ -263,7 +268,6 @@ static void primary_all_cb(guint8 status, const guint8 *ipdu, guint16 iplen,
}

att_data_list_free(list);
- err = 0;

if (end != 0xffff) {
size_t buflen;
@@ -272,17 +276,12 @@ static void primary_all_cb(guint8 status, const guint8 *ipdu, guint16 iplen,
buf, buflen);

if (g_attrib_send(dp->attrib, 0, buf, oplen, primary_all_cb,
- discover_primary_ref(dp), NULL) == 0) {
- err = ATT_ECODE_ABORTED;
- goto done;
+ discover_primary_ref(dp),
+ discover_primary_drop) == 0) {
+ set_error(&dp->err, ATT_ECODE_ABORTED);
+ discover_primary_unref(dp);
}
-
- return;
}
-
-done:
- set_error(&dp->err, err);
- discover_primary_unref(dp);
}

guint gatt_discover_primary(GAttrib *attrib, bt_uuid_t *uuid, gatt_cb_t func,
@@ -293,6 +292,7 @@ guint gatt_discover_primary(GAttrib *attrib, bt_uuid_t *uuid, gatt_cb_t func,
uint8_t *buf = g_attrib_get_buffer(attrib, &buflen);
GAttribResultFunc cb;
guint16 plen;
+ int id;

plen = encode_discover_primary(0x0001, 0xffff, uuid, buf, buflen);
if (plen == 0)
@@ -312,7 +312,14 @@ guint gatt_discover_primary(GAttrib *attrib, bt_uuid_t *uuid, gatt_cb_t func,
} else
cb = primary_all_cb;

- return g_attrib_send(attrib, 0, buf, plen, cb, dp, NULL);
+ id = g_attrib_send(attrib, 0, buf, plen, cb, discover_primary_ref(dp),
+ discover_primary_drop);
+ if (id == 0) {
+ g_attrib_unref(dp->attrib);
+ g_free(dp);
+ }
+
+ return id;
}

static void resolve_included_uuid_cb(uint8_t status, const uint8_t *pdu,
--
1.8.1.3


2013-03-01 23:22:31

by Vinicius Costa Gomes

[permalink] [raw]
Subject: [RFC BlueZ v0 08/10] gatt: Better lifetime tracking for Find Included services

Use the GDestroyNotify facility from g_attrib_send() to properly
end a procedure.
---
attrib/gatt.c | 73 +++++++++++++++++++++++++++++++++++------------------------
1 file changed, 44 insertions(+), 29 deletions(-)

diff --git a/attrib/gatt.c b/attrib/gatt.c
index b451917..ef3ffff 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -328,30 +328,37 @@ static void resolve_included_uuid_cb(uint8_t status, const uint8_t *pdu,
struct included_uuid_query *query = user_data;
struct included_discovery *isd = query->isd;
struct gatt_included *incl = query->included;
- unsigned int err = status;
bt_uuid_t uuid;
size_t buflen;
uint8_t *buf;

- if (err)
- goto done;
+ if (status) {
+ set_error(&isd->err, status);
+ return;
+ }

buf = g_attrib_get_buffer(isd->attrib, &buflen);
if (dec_read_resp(pdu, len, buf, buflen) != 16) {
- err = ATT_ECODE_IO;
- goto done;
+ set_error(&isd->err, ATT_ECODE_IO);
+ return;
}

uuid = att_get_uuid128(buf);
bt_uuid_to_string(&uuid, incl->uuid, sizeof(incl->uuid));
isd->includes = g_slist_append(isd->includes, incl);
+}

-done:
- if (err)
- g_free(incl);
+static void included_query_free(gpointer user_data)
+{
+ struct included_uuid_query *query = user_data;
+ struct included_discovery *isd = query->isd;

- if (isd->err == 0)
- isd->err = err;
+ if (isd->err) {
+ /* This means that 'included' was not included in the
+ * 'includes' list
+ */
+ g_free(query->included);
+ }

isd_unref(isd);

@@ -365,13 +372,21 @@ static guint resolve_included_uuid(struct included_discovery *isd,
size_t buflen;
uint8_t *buf = g_attrib_get_buffer(isd->attrib, &buflen);
guint16 oplen = enc_read_req(incl->range.start, buf, buflen);
+ int id;

query = g_new0(struct included_uuid_query, 1);
query->isd = isd_ref(isd);
query->included = incl;

- return g_attrib_send(isd->attrib, 0, buf, oplen,
- resolve_included_uuid_cb, query, NULL);
+ id = g_attrib_send(isd->attrib, 0, buf, oplen,
+ resolve_included_uuid_cb, query,
+ included_query_free);
+ if (id == 0) {
+ isd_unref(query->isd);
+ g_free(query);
+ }
+
+ return id;
}

static struct gatt_included *included_from_buf(const uint8_t *buf, gsize len)
@@ -396,6 +411,13 @@ static struct gatt_included *included_from_buf(const uint8_t *buf, gsize len)
static void find_included_cb(uint8_t status, const uint8_t *pdu, uint16_t len,
gpointer user_data);

+static void find_included_drop(gpointer user_data)
+{
+ struct included_discovery *isd = user_data;
+
+ isd_unref(isd);
+}
+
static guint find_included(struct included_discovery *isd, uint16_t start)
{
bt_uuid_t uuid;
@@ -408,7 +430,7 @@ static guint find_included(struct included_discovery *isd, uint16_t start)
buf, buflen);

return g_attrib_send(isd->attrib, 0, buf, oplen, find_included_cb,
- isd_ref(isd), NULL);
+ isd_ref(isd), find_included_drop);
}

static void find_included_cb(uint8_t status, const uint8_t *pdu, uint16_t len,
@@ -416,26 +438,25 @@ static void find_included_cb(uint8_t status, const uint8_t *pdu, uint16_t len,
{
struct included_discovery *isd = user_data;
uint16_t last_handle = isd->end_handle;
- unsigned int err = status;
struct att_data_list *list;
int i;

- if (err == ATT_ECODE_ATTR_NOT_FOUND)
- err = 0;
-
- if (status)
- goto done;
+ if (status) {
+ int err = status == ATT_ECODE_ATTR_NOT_FOUND ? 0 : status;
+ set_error(&isd->err, err);
+ return;
+ }

list = dec_read_by_type_resp(pdu, len);
if (list == NULL) {
- err = ATT_ECODE_IO;
- goto done;
+ set_error(&isd->err, ATT_ECODE_IO);
+ return;
}

if (list->len != 6 && list->len != 8) {
- err = ATT_ECODE_IO;
+ set_error(&isd->err, ATT_ECODE_IO);
att_data_list_free(list);
- goto done;
+ return;
}

for (i = 0; i < list->num; i++) {
@@ -457,12 +478,6 @@ static void find_included_cb(uint8_t status, const uint8_t *pdu, uint16_t len,

if (last_handle < isd->end_handle)
find_included(isd, last_handle + 1);
-
-done:
- if (isd->err == 0)
- isd->err = err;
-
- isd_unref(isd);
}

unsigned int gatt_find_included(GAttrib *attrib, uint16_t start, uint16_t end,
--
1.8.1.3


2013-03-01 23:22:29

by Vinicius Costa Gomes

[permalink] [raw]
Subject: [RFC BlueZ v0 06/10] gatt: Handle errors when g_attrib_send() in gatt_discover_primary()

To make the error handling clearer, it was introduced a reference
counting mechanism for the Discover Primary services procedure.
---
attrib/gatt.c | 46 ++++++++++++++++++++++++++++++++++++++--------
1 file changed, 38 insertions(+), 8 deletions(-)

diff --git a/attrib/gatt.c b/attrib/gatt.c
index 44d3eb6..1f6360f 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -39,8 +39,10 @@

struct discover_primary {
GAttrib *attrib;
+ int refs;
bt_uuid_t uuid;
GSList *primaries;
+ int err;
gatt_cb_t cb;
void *user_data;
};
@@ -70,13 +72,33 @@ struct discover_char {
void *user_data;
};

-static void discover_primary_free(struct discover_primary *dp)
+static struct discover_primary *discover_primary_ref(
+ struct discover_primary *dp)
{
+ g_atomic_int_inc(&dp->refs);
+
+ return dp;
+}
+
+static void discover_primary_unref(struct discover_primary *dp)
+{
+ if (g_atomic_int_dec_and_test(&dp->refs) == FALSE)
+ return;
+
+ dp->cb(dp->primaries, dp->err, dp->user_data);
+
g_slist_free(dp->primaries);
g_attrib_unref(dp->attrib);
g_free(dp);
}

+static void set_error(int *result, const int err)
+{
+ /* Do not overwrite errors */
+ if (*result == 0)
+ *result = err;
+}
+
static struct included_discovery *isd_ref(struct included_discovery *isd)
{
g_atomic_int_inc(&isd->refs);
@@ -179,12 +201,17 @@ static void primary_by_uuid_cb(guint8 status, const guint8 *ipdu,
if (oplen == 0)
goto done;

- g_attrib_send(dp->attrib, 0, buf, oplen, primary_by_uuid_cb, dp, NULL);
+ if (g_attrib_send(dp->attrib, 0, buf, oplen, primary_by_uuid_cb,
+ discover_primary_ref(dp), NULL) == 0) {
+ err = ATT_ECODE_ABORTED;
+ goto done;
+ }
+
return;

done:
- dp->cb(dp->primaries, err, dp->user_data);
- discover_primary_free(dp);
+ set_error(&dp->err, err);
+ discover_primary_unref(dp);
}

static void primary_all_cb(guint8 status, const guint8 *ipdu, guint16 iplen,
@@ -244,15 +271,18 @@ static void primary_all_cb(guint8 status, const guint8 *ipdu, guint16 iplen,
guint16 oplen = encode_discover_primary(end + 1, 0xffff, NULL,
buf, buflen);

- g_attrib_send(dp->attrib, 0, buf, oplen, primary_all_cb,
- dp, NULL);
+ if (g_attrib_send(dp->attrib, 0, buf, oplen, primary_all_cb,
+ discover_primary_ref(dp), NULL) == 0) {
+ err = ATT_ECODE_ABORTED;
+ goto done;
+ }

return;
}

done:
- dp->cb(dp->primaries, err, dp->user_data);
- discover_primary_free(dp);
+ set_error(&dp->err, err);
+ discover_primary_unref(dp);
}

guint gatt_discover_primary(GAttrib *attrib, bt_uuid_t *uuid, gatt_cb_t func,
--
1.8.1.3


2013-03-01 23:22:28

by Vinicius Costa Gomes

[permalink] [raw]
Subject: [RFC BlueZ v0 05/10] device: Use only one entry point for LE connections

Until now, two functions were used for establishing LE connections,
device_connect_le() and device_browse_primary().

This commit changes the behaviour of device_browse_primary() to expect the
conenction to be already established, and if not to call device_connect_le().
This simplification allows for the 'att_callbacks' mechanism to be removed.
---
src/device.c | 178 +++++++++++++++++------------------------------------------
1 file changed, 50 insertions(+), 128 deletions(-)

diff --git a/src/device.c b/src/device.c
index bfc60fa..f2abcff 100644
--- a/src/device.c
+++ b/src/device.c
@@ -141,12 +141,6 @@ struct svc_callback {
typedef void (*attio_error_cb) (const GError *gerr, gpointer user_data);
typedef void (*attio_success_cb) (gpointer user_data);

-struct att_callbacks {
- attio_error_cb error; /* Callback for error */
- attio_success_cb success; /* Callback for success */
- gpointer user_data;
-};
-
struct btd_device {
int ref_count;

@@ -214,8 +208,7 @@ static const uint16_t uuid_list[] = {
0
};

-static int device_browse_primary(struct btd_device *device, DBusMessage *msg,
- gboolean secure);
+static int device_browse_primary(struct btd_device *device, DBusMessage *msg);
static int device_browse_sdp(struct btd_device *device, DBusMessage *msg,
gboolean reverse);
static void device_svc_resolved(struct btd_device *dev, DBusMessage *msg,
@@ -1185,7 +1178,7 @@ static int device_resolve_svc(struct btd_device *dev, DBusMessage *msg)
if (device_is_bredr(dev))
return device_browse_sdp(dev, msg, FALSE);
else
- return device_browse_primary(dev, msg, FALSE);
+ return device_browse_primary(dev, msg);
}

static struct btd_profile *find_connectable_profile(struct btd_device *dev,
@@ -2838,14 +2831,14 @@ static void search_cb(sdp_list_t *recs, int err, gpointer user_data)
DEVICE_INTERFACE, "UUIDs");

send_reply:
- /* Browse has finished, some profiles may use device->browse to check
- * if browsing is still ongoing, so we mark it as finished, and free
+ /* Browse has finished, some code may use device->browse to check if
+ * browsing is still ongoing, so we mark it as finished, and free
* it later
*/
device->browse = NULL;

device_svc_resolved(device, req->msg, err);
- req->msg = NULL; /* The reply was sent above */
+ req->msg = NULL; /* The reply was just sent */

if (!device->temporary)
store_device_info(device);
@@ -3173,10 +3166,35 @@ static void primary_cb(GSList *services, guint8 status, gpointer user_data)
find_included_services(device, services);
}

+static void att_connect_error(struct btd_device *device, const GError *gerr)
+{
+ if (g_error_matches(gerr, BT_IO_ERROR, ECONNABORTED))
+ return;
+
+ if (device_get_auto_connect(device)) {
+ DBG("Enabling automatic connections");
+ adapter_connect_list_add(device->adapter, device);
+ }
+}
+
+static void att_connect_success(struct btd_device *device)
+{
+ if (device->attios == NULL)
+ return;
+
+ /*
+ * Remove the device from the connect_list and give the passive
+ * scanning another chance to be restarted in case there are
+ * other devices in the connect_list.
+ */
+ adapter_connect_list_remove(device->adapter, device);
+
+ g_slist_foreach(device->attios, attio_connected, device->attrib);
+}
+
static void att_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
{
- struct att_callbacks *attcb = user_data;
- struct btd_device *device = attcb->user_data;
+ struct btd_device *device = user_data;
DBusMessage *reply;
uint8_t io_cap;
GAttrib *attrib;
@@ -3188,8 +3206,7 @@ static void att_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
if (gerr) {
DBG("%s", gerr->message);

- if (attcb->error)
- attcb->error(gerr, attcb->user_data);
+ att_connect_error(device, gerr);

err = -ECONNABORTED;
goto done;
@@ -3204,8 +3221,7 @@ static void att_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
device->cleanup_id = g_io_add_watch(io, G_IO_HUP,
attrib_disconnected_cb, device);

- if (attcb->success)
- attcb->success(user_data);
+ att_connect_success(device);

if (!device->bonding)
goto done;
@@ -3225,10 +3241,12 @@ done:
bonding_request_free(device->bonding);
}

- if (device->connect) {
- if (!device->svc_resolved)
- device_browse_primary(device, NULL, FALSE);
+ /* The connection was caused by a device_browse_primary() */
+ if (device->gatt_discov)
+ gatt_discover_primary(device->attrib, NULL, primary_cb,
+ btd_device_ref(device));

+ if (device->connect) {
if (err < 0)
reply = btd_error_failed(device->connect,
strerror(-err));
@@ -3239,45 +3257,11 @@ done:
dbus_message_unref(device->connect);
device->connect = NULL;
}
-
- g_free(attcb);
-}
-
-static void att_error_cb(const GError *gerr, gpointer user_data)
-{
- struct btd_device *device = user_data;
-
- if (g_error_matches(gerr, BT_IO_ERROR, ECONNABORTED))
- return;
-
- if (device_get_auto_connect(device)) {
- DBG("Enabling automatic connections");
- adapter_connect_list_add(device->adapter, device);
- }
-}
-
-static void att_success_cb(gpointer user_data)
-{
- struct att_callbacks *attcb = user_data;
- struct btd_device *device = attcb->user_data;
-
- if (device->attios == NULL)
- return;
-
- /*
- * Remove the device from the connect_list and give the passive
- * scanning another chance to be restarted in case there are
- * other devices in the connect_list.
- */
- adapter_connect_list_remove(device->adapter, device);
-
- g_slist_foreach(device->attios, attio_connected, device->attrib);
}

int device_connect_le(struct btd_device *dev)
{
struct btd_adapter *adapter = dev->adapter;
- struct att_callbacks *attcb;
BtIOSecLevel sec_level;
GIOChannel *io;
GError *gerr = NULL;
@@ -3291,11 +3275,6 @@ int device_connect_le(struct btd_device *dev)

DBG("Connection attempt to: %s", addr);

- attcb = g_new0(struct att_callbacks, 1);
- attcb->error = att_error_cb;
- attcb->success = att_success_cb;
- attcb->user_data = dev;
-
if (dev->paired)
sec_level = BT_IO_SEC_MEDIUM;
else
@@ -3305,7 +3284,7 @@ int device_connect_le(struct btd_device *dev)
* This connection will help us catch any PDUs that comes before
* pairing finishes
*/
- io = bt_io_connect(att_connect_cb, attcb, NULL, &gerr,
+ io = bt_io_connect(att_connect_cb, dev, NULL, &gerr,
BT_IO_OPT_SOURCE_BDADDR, adapter_get_address(adapter),
BT_IO_OPT_DEST_BDADDR, &dev->bdaddr,
BT_IO_OPT_DEST_TYPE, dev->bdaddr_type,
@@ -3325,7 +3304,6 @@ int device_connect_le(struct btd_device *dev)

error("ATT bt_io_connect(%s): %s", addr, gerr->message);
g_error_free(gerr);
- g_free(attcb);
return -EIO;
}

@@ -3335,78 +3313,16 @@ int device_connect_le(struct btd_device *dev)
return 0;
}

-static void att_browse_error_cb(const GError *gerr, gpointer user_data)
-{
- struct btd_device *device = user_data;
- struct gatt_discovery *disc = device->gatt_discov;
-
- if (disc->msg) {
- DBusMessage *reply;
-
- reply = btd_error_failed(disc->msg, gerr->message);
- g_dbus_send_message(dbus_conn, reply);
- }
-
- device->gatt_discov = NULL;
- gatt_discovery_free(disc);
-}
-
-static void att_browse_cb(gpointer user_data)
-{
- struct att_callbacks *attcb = user_data;
- struct btd_device *device = attcb->user_data;
-
- gatt_discover_primary(device->attrib, NULL, primary_cb,
- device->browse);
-}
-
-static int device_browse_primary(struct btd_device *device, DBusMessage *msg,
- gboolean secure)
+static int device_browse_primary(struct btd_device *device, DBusMessage *msg)
{
- struct btd_adapter *adapter = device->adapter;
- struct att_callbacks *attcb;
struct gatt_discovery *disc;
- BtIOSecLevel sec_level;

if (device->gatt_discov)
return -EBUSY;

disc = g_new0(struct gatt_discovery, 1);
-
device->gatt_discov = disc;

- if (device->attrib) {
- gatt_discover_primary(device->attrib, NULL, primary_cb,
- btd_device_ref(device));
- goto done;
- }
-
- sec_level = secure ? BT_IO_SEC_HIGH : BT_IO_SEC_LOW;
-
- attcb = g_new0(struct att_callbacks, 1);
- attcb->error = att_browse_error_cb;
- attcb->success = att_browse_cb;
- attcb->user_data = device;
-
- device->att_io = bt_io_connect(att_connect_cb,
- attcb, NULL, NULL,
- BT_IO_OPT_SOURCE_BDADDR,
- adapter_get_address(adapter),
- BT_IO_OPT_DEST_BDADDR, &device->bdaddr,
- BT_IO_OPT_DEST_TYPE, device->bdaddr_type,
- BT_IO_OPT_CID, ATT_CID,
- BT_IO_OPT_SEC_LEVEL, sec_level,
- BT_IO_OPT_INVALID);
-
- if (device->att_io == NULL) {
- gatt_discovery_free(disc);
- device->gatt_discov = NULL;
- g_free(attcb);
- return -EIO;
- }
-
-done:
-
if (msg) {
const char *sender = dbus_message_get_sender(msg);

@@ -3418,6 +3334,12 @@ done:
device, NULL);
}

+ if (device->attrib == NULL)
+ return device_connect_le(device);
+
+ gatt_discover_primary(device->attrib, NULL, primary_cb,
+ btd_device_ref(device));
+
return 0;
}

@@ -3615,7 +3537,7 @@ static gboolean start_discovery(gpointer user_data)
if (device_is_bredr(device))
device_browse_sdp(device, NULL, TRUE);
else
- device_browse_primary(device, NULL, FALSE);
+ device_browse_primary(device, NULL);

device->discov_timer = 0;

@@ -3685,7 +3607,7 @@ void device_bonding_complete(struct btd_device *device, uint8_t status)
if (device_is_bredr(device))
device_browse_sdp(device, bonding->msg, FALSE);
else
- device_browse_primary(device, bonding->msg, FALSE);
+ device_browse_primary(device, bonding->msg);

bonding_request_free(bonding);
} else if (!device->svc_resolved) {
@@ -4109,7 +4031,7 @@ void btd_device_gatt_set_service_changed(struct btd_device *device,
prim->changed = TRUE;
}

- device_browse_primary(device, NULL, FALSE);
+ device_browse_primary(device, NULL);
}

void btd_device_add_uuid(struct btd_device *device, const char *uuid)
--
1.8.1.3


2013-03-01 23:22:27

by Vinicius Costa Gomes

[permalink] [raw]
Subject: [RFC BlueZ v0 04/10] device: Remove misleading function

'device_register_primaries()' only appended a list of primary services
to the current list.
---
src/device.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/src/device.c b/src/device.c
index 435a9b8..bfc60fa 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2789,12 +2789,6 @@ static GSList *device_services_from_record(struct btd_device *device,
return prim_list;
}

-static void device_register_primaries(struct btd_device *device,
- GSList *prim_list, int psm)
-{
- device->primaries = g_slist_concat(device->primaries, prim_list);
-}
-
static void search_cb(sdp_list_t *recs, int err, gpointer user_data)
{
struct browse_req *req = user_data;
@@ -2829,7 +2823,8 @@ static void search_cb(sdp_list_t *recs, int err, gpointer user_data)

list = device_services_from_record(device, req->profiles_added);
if (list)
- device_register_primaries(device, list, ATT_PSM);
+ device->primaries = g_slist_concat(device->primaries,
+ list);

device_probe_profiles(device, req->profiles_added);
}
@@ -3049,7 +3044,9 @@ static void register_all_services(struct btd_device *device, GSList *services)
g_slist_free_full(device->primaries, g_free);
device->primaries = NULL;

- device_register_primaries(device, g_slist_copy(services), -1);
+ /* And keep the new one */
+ device->primaries = g_slist_copy(services);
+
if (removed)
device_remove_profiles(device, removed);

--
1.8.1.3


2013-03-01 23:22:26

by Vinicius Costa Gomes

[permalink] [raw]
Subject: [RFC BlueZ v0 03/10] device: Fix removing profiles

The check against removed profile were always made using an empty
list, device->uuids was just cleared. And we should store the device
information after the profiles are removed.
---
src/device.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/src/device.c b/src/device.c
index b7f30d8..435a9b8 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2474,25 +2474,19 @@ add_uuids:

static void device_remove_profiles(struct btd_device *device, GSList *uuids)
{
- char srcaddr[18], dstaddr[18];
+ char dstaddr[18];
GSList *l, *next;

- ba2str(adapter_get_address(device->adapter), srcaddr);
ba2str(&device->bdaddr, dstaddr);

DBG("Removing profiles for %s", dstaddr);

- g_slist_free(device->uuids);
- device->uuids = NULL;
- store_device_info(device);
-
for (l = device->profiles; l != NULL; l = next) {
struct btd_profile *profile = l->data;
GSList *probe_uuids;

next = l->next;
- probe_uuids = device_match_profile(device, profile,
- device->uuids);
+ probe_uuids = device_match_profile(device, profile, uuids);
if (probe_uuids != NULL) {
g_slist_free(probe_uuids);
continue;
@@ -2500,6 +2494,7 @@ static void device_remove_profiles(struct btd_device *device, GSList *uuids)

profile->device_remove(profile, device);
device->profiles = g_slist_remove(device->profiles, profile);
+ g_slist_free(probe_uuids);
}
}

@@ -3069,6 +3064,8 @@ static void register_all_services(struct btd_device *device, GSList *services)
device_svc_resolved(device, disc->msg, 0);
disc->msg = NULL; /* The reply was sent just above */

+ store_device_info(device);
+
store_services(device);
}

--
1.8.1.3


2013-03-01 23:22:25

by Vinicius Costa Gomes

[permalink] [raw]
Subject: [RFC BlueZ v0 02/10] device: Separate SDP browse from GATT discover primary services

Both procedures have the same objective, which is discovering the
services that a remote device supports. But the similarities stop
there.

This commit splits the structures used for both structures, so
it's clear that the procedures are different. As the structures are
different some simplification in the service discovery logic can
be made.

The presence of a valid pointer on device->gatt_discov indicates
that there's a discovery going on, if at any point that pointer
isn't valid, the discovery should be stopped, for example, the
sender of the request has exited.
---
src/device.c | 225 ++++++++++++++++++++++++++++++++++++-----------------------
1 file changed, 137 insertions(+), 88 deletions(-)

diff --git a/src/device.c b/src/device.c
index 8c1bd73..b7f30d8 100644
--- a/src/device.c
+++ b/src/device.c
@@ -116,10 +116,11 @@ struct browse_req {
guint listener_id;
};

-struct included_search {
- struct browse_req *req;
- GSList *services;
- GSList *current;
+struct gatt_discovery {
+ DBusMessage *msg;
+ guint sender_watch;
+ GSList *services; /* Found GATT services */
+ GSList *current; /* Current service checked for included services */
};

struct attio_data {
@@ -174,6 +175,7 @@ struct btd_device {
guint disconn_timer;
guint discov_timer;
struct browse_req *browse; /* service discover request */
+ struct gatt_discovery *gatt_discov; /* GATT service discovery */
struct bonding_req *bonding;
struct authentication_req *authr; /* authentication request */
GSList *disconnects; /* disconnects message */
@@ -216,6 +218,8 @@ static int device_browse_primary(struct btd_device *device, DBusMessage *msg,
gboolean secure);
static int device_browse_sdp(struct btd_device *device, DBusMessage *msg,
gboolean reverse);
+static void device_svc_resolved(struct btd_device *dev, DBusMessage *msg,
+ int err);

static gboolean store_device_info_cb(gpointer user_data)
{
@@ -359,6 +363,20 @@ static void browse_request_free(struct browse_req *req)
g_free(req);
}

+static void gatt_discovery_free(struct gatt_discovery *disc)
+{
+ if (disc == NULL)
+ return;
+
+ if (disc->msg)
+ dbus_message_unref(disc->msg);
+
+ if (disc->sender_watch)
+ g_dbus_remove_watch(dbus_conn, disc->sender_watch);
+
+ g_free(disc);
+}
+
static void attio_cleanup(struct btd_device *device)
{
if (device->attachid) {
@@ -377,6 +395,16 @@ static void attio_cleanup(struct btd_device *device)
device->att_io = NULL;
}

+ if (device->gatt_discov) {
+ struct gatt_discovery *disc = device->gatt_discov;
+
+ device_svc_resolved(device, disc->msg, EIO);
+ disc->msg = NULL;
+
+ gatt_discovery_free(disc);
+ device->gatt_discov = NULL;
+ }
+
if (device->attrib) {
g_attrib_unref(device->attrib);
device->attrib = NULL;
@@ -960,6 +988,13 @@ static void discover_services_req_exit(DBusConnection *conn, void *user_data)
browse_request_cancel(req);
}

+static void gatt_discovery_sender_exit(DBusConnection *conn, void *user_data)
+{
+ struct btd_device *device = user_data;
+
+ attio_cleanup(device);
+}
+
static void bonding_request_cancel(struct bonding_req *bonding)
{
struct btd_device *device = bonding->device;
@@ -1344,13 +1379,12 @@ static DBusMessage *disconnect_profile(DBusConnection *conn, DBusMessage *msg,
return NULL;
}

-static void device_svc_resolved(struct btd_device *dev, int err)
+static void device_svc_resolved(struct btd_device *dev, DBusMessage *msg,
+ int err)
{
DBusMessage *reply;
- struct browse_req *req = dev->browse;

dev->svc_resolved = true;
- dev->browse = NULL;

g_slist_free_full(dev->eir_uuids, g_free);
dev->eir_uuids = NULL;
@@ -1368,32 +1402,30 @@ static void device_svc_resolved(struct btd_device *dev, int err)
g_free(cb);
}

- if (!req || !req->msg)
+ if (msg == NULL)
return;

- if (dbus_message_is_method_call(req->msg, DEVICE_INTERFACE,
- "Pair")) {
- reply = dbus_message_new_method_return(req->msg);
+ if (dbus_message_is_method_call(msg, DEVICE_INTERFACE, "Pair")) {
+ reply = dbus_message_new_method_return(msg);
g_dbus_send_message(dbus_conn, reply);
return;
}

if (err) {
- reply = btd_error_failed(req->msg, strerror(-err));
+ reply = btd_error_failed(msg, strerror(-err));
g_dbus_send_message(dbus_conn, reply);
return;
}

- if (dbus_message_is_method_call(req->msg, DEVICE_INTERFACE, "Connect"))
- reply = dev_connect(dbus_conn, req->msg, dev);
- else if (dbus_message_is_method_call(req->msg, DEVICE_INTERFACE,
+ if (dbus_message_is_method_call(msg, DEVICE_INTERFACE, "Connect"))
+ reply = dev_connect(dbus_conn, msg, dev);
+ else if (dbus_message_is_method_call(msg, DEVICE_INTERFACE,
"ConnectProfile"))
- reply = connect_profile(dbus_conn, req->msg, dev);
+ reply = connect_profile(dbus_conn, msg, dev);
else
return;

- dbus_message_unref(req->msg);
- req->msg = NULL;
+ dbus_message_unref(msg);

if (reply)
g_dbus_send_message(dbus_conn, reply);
@@ -2689,8 +2721,8 @@ static gint primary_cmp(gconstpointer a, gconstpointer b)
return memcmp(a, b, sizeof(struct gatt_primary));
}

-static void update_gatt_services(struct browse_req *req, GSList *current,
- GSList *found)
+static void update_gatt_services(GSList *current, GSList *found,
+ GSList **added, GSList **removed)
{
GSList *l, *lmatch, *left = g_slist_copy(current);

@@ -2706,8 +2738,7 @@ static void update_gatt_services(struct browse_req *req, GSList *current,
}

/* New entry */
- req->profiles_added = g_slist_append(req->profiles_added,
- g_strdup(prim->uuid));
+ *added = g_slist_append(*added, g_strdup(prim->uuid));

DBG("UUID Added: %s", prim->uuid);
}
@@ -2715,8 +2746,7 @@ static void update_gatt_services(struct browse_req *req, GSList *current,
/* Removed Profiles */
for (l = left; l; l = g_slist_next(l)) {
struct gatt_primary *prim = l->data;
- req->profiles_removed = g_slist_append(req->profiles_removed,
- g_strdup(prim->uuid));
+ *removed = g_slist_append(*removed, g_strdup(prim->uuid));

DBG("UUID Removed: %s", prim->uuid);
}
@@ -2818,7 +2848,14 @@ static void search_cb(sdp_list_t *recs, int err, gpointer user_data)
DEVICE_INTERFACE, "UUIDs");

send_reply:
- device_svc_resolved(device, err);
+ /* Browse has finished, some profiles may use device->browse to check
+ * if browsing is still ongoing, so we mark it as finished, and free
+ * it later
+ */
+ device->browse = NULL;
+
+ device_svc_resolved(device, req->msg, err);
+ req->msg = NULL; /* The reply was sent above */

if (!device->temporary)
store_device_info(device);
@@ -2972,10 +3009,11 @@ static gboolean attrib_disconnected_cb(GIOChannel *io, GIOCondition cond,
gpointer user_data)
{
struct btd_device *device = user_data;
+ struct gatt_discovery *disc = device->gatt_discov;
int sock, err = 0;
socklen_t len;

- if (device->browse)
+ if (disc)
goto done;

sock = g_io_channel_unix_get_fd(io);
@@ -3003,21 +3041,24 @@ done:
return FALSE;
}

-static void register_all_services(struct browse_req *req, GSList *services)
+static void register_all_services(struct btd_device *device, GSList *services)
{
- struct btd_device *device = req->device;
+ struct gatt_discovery *disc = device->gatt_discov;
+ GSList *added = NULL, *removed = NULL;

device_set_temporary(device, FALSE);

- update_gatt_services(req, device->primaries, services);
+ update_gatt_services(device->primaries, services, &added, &removed);
+
+ /* 'services' is the new 'primaries', clear the old one */
g_slist_free_full(device->primaries, g_free);
device->primaries = NULL;

device_register_primaries(device, g_slist_copy(services), -1);
- if (req->profiles_removed)
- device_remove_profiles(device, req->profiles_removed);
+ if (removed)
+ device_remove_profiles(device, removed);

- device_probe_profiles(device, req->profiles_added);
+ device_probe_profiles(device, added);

if (device->attios == NULL && device->attios_offline == NULL)
attio_cleanup(device);
@@ -3025,11 +3066,10 @@ static void register_all_services(struct browse_req *req, GSList *services)
g_dbus_emit_property_changed(dbus_conn, device->path,
DEVICE_INTERFACE, "UUIDs");

- device_svc_resolved(device, 0);
+ device_svc_resolved(device, disc->msg, 0);
+ disc->msg = NULL; /* The reply was sent just above */

store_services(device);
-
- browse_request_free(req);
}

static int service_by_range_cmp(gconstpointer a, gconstpointer b)
@@ -3043,11 +3083,16 @@ static int service_by_range_cmp(gconstpointer a, gconstpointer b)
static void find_included_cb(GSList *includes, uint8_t status,
gpointer user_data)
{
- struct included_search *search = user_data;
- struct btd_device *device = search->req->device;
+ struct btd_device *device = user_data;
+ struct gatt_discovery *disc = device->gatt_discov;
struct gatt_primary *prim;
GSList *l;

+ if (disc == NULL) {
+ btd_device_unref(device);
+ return;
+ }
+
if (status != 0) {
error("Find included services failed: %s (%d)",
att_ecode2str(status), status);
@@ -3060,7 +3105,7 @@ static void find_included_cb(GSList *includes, uint8_t status,
for (l = includes; l; l = l->next) {
struct gatt_included *incl = l->data;

- if (g_slist_find_custom(search->services, &incl->range,
+ if (g_slist_find_custom(disc->services, &incl->range,
service_by_range_cmp))
continue;

@@ -3068,63 +3113,70 @@ static void find_included_cb(GSList *includes, uint8_t status,
memcpy(prim->uuid, incl->uuid, sizeof(prim->uuid));
memcpy(&prim->range, &incl->range, sizeof(prim->range));

- search->services = g_slist_append(search->services, prim);
+ disc->services = g_slist_append(disc->services, prim);
}

done:
- search->current = search->current->next;
- if (search->current == NULL) {
- register_all_services(search->req, search->services);
- g_slist_free(search->services);
- g_free(search);
+ disc->current = disc->current->next;
+ if (disc->current == NULL) {
+ register_all_services(device, disc->services);
+ g_slist_free(disc->services);
+ btd_device_unref(device);
return;
}

- prim = search->current->data;
+ prim = disc->current->data;
gatt_find_included(device->attrib, prim->range.start, prim->range.end,
- find_included_cb, search);
+ find_included_cb, device);
}

-static void find_included_services(struct browse_req *req, GSList *services)
+static void find_included_services(struct btd_device *device, GSList *services)
{
- struct btd_device *device = req->device;
- struct included_search *search;
struct gatt_primary *prim;
+ struct gatt_discovery *disc;
+
+ disc = device->gatt_discov;

- if (services == NULL)
+ if (services == NULL) {
+ gatt_discovery_free(disc);
+ device->gatt_discov = NULL;
+ btd_device_unref(device);
return;
+ }

- search = g_new0(struct included_search, 1);
- search->req = req;
- search->services = g_slist_copy(services);
- search->current = search->services;
+ disc->services = g_slist_copy(services);
+ disc->current = disc->services;

- prim = search->current->data;
+ prim = disc->current->data;
gatt_find_included(device->attrib, prim->range.start, prim->range.end,
- find_included_cb, search);
-
+ find_included_cb, device);
}

static void primary_cb(GSList *services, guint8 status, gpointer user_data)
{
- struct browse_req *req = user_data;
+ struct btd_device *device = user_data;
+ struct gatt_discovery *disc = device->gatt_discov;

- if (status) {
- struct btd_device *device = req->device;
+ if (disc == NULL) {
+ btd_device_unref(device);
+ return;
+ }

- if (req->msg) {
+ if (status) {
+ if (disc->msg) {
DBusMessage *reply;
- reply = btd_error_failed(req->msg,
+ reply = btd_error_failed(disc->msg,
att_ecode2str(status));
g_dbus_send_message(dbus_conn, reply);
}

- device->browse = NULL;
- browse_request_free(req);
+ gatt_discovery_free(disc);
+ device->gatt_discov = NULL;
+ btd_device_unref(device);
return;
}

- find_included_services(req, services);
+ find_included_services(device, services);
}

static void att_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
@@ -3143,7 +3195,7 @@ static void att_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
DBG("%s", gerr->message);

if (attcb->error)
- attcb->error(gerr, user_data);
+ attcb->error(gerr, attcb->user_data);

err = -ECONNABORTED;
goto done;
@@ -3199,8 +3251,7 @@ done:

static void att_error_cb(const GError *gerr, gpointer user_data)
{
- struct att_callbacks *attcb = user_data;
- struct btd_device *device = attcb->user_data;
+ struct btd_device *device = user_data;

if (g_error_matches(gerr, BT_IO_ERROR, ECONNABORTED))
return;
@@ -3292,19 +3343,18 @@ int device_connect_le(struct btd_device *dev)

static void att_browse_error_cb(const GError *gerr, gpointer user_data)
{
- struct att_callbacks *attcb = user_data;
- struct btd_device *device = attcb->user_data;
- struct browse_req *req = device->browse;
+ struct btd_device *device = user_data;
+ struct gatt_discovery *disc = device->gatt_discov;

- if (req->msg) {
+ if (disc->msg) {
DBusMessage *reply;

- reply = btd_error_failed(req->msg, gerr->message);
+ reply = btd_error_failed(disc->msg, gerr->message);
g_dbus_send_message(dbus_conn, reply);
}

- device->browse = NULL;
- browse_request_free(req);
+ device->gatt_discov = NULL;
+ gatt_discovery_free(disc);
}

static void att_browse_cb(gpointer user_data)
@@ -3321,19 +3371,19 @@ static int device_browse_primary(struct btd_device *device, DBusMessage *msg,
{
struct btd_adapter *adapter = device->adapter;
struct att_callbacks *attcb;
- struct browse_req *req;
+ struct gatt_discovery *disc;
BtIOSecLevel sec_level;

- if (device->browse)
+ if (device->gatt_discov)
return -EBUSY;

- req = g_new0(struct browse_req, 1);
- req->device = btd_device_ref(device);
+ disc = g_new0(struct gatt_discovery, 1);

- device->browse = req;
+ device->gatt_discov = disc;

if (device->attrib) {
- gatt_discover_primary(device->attrib, NULL, primary_cb, req);
+ gatt_discover_primary(device->attrib, NULL, primary_cb,
+ btd_device_ref(device));
goto done;
}

@@ -3355,8 +3405,8 @@ static int device_browse_primary(struct btd_device *device, DBusMessage *msg,
BT_IO_OPT_INVALID);

if (device->att_io == NULL) {
- device->browse = NULL;
- browse_request_free(req);
+ gatt_discovery_free(disc);
+ device->gatt_discov = NULL;
g_free(attcb);
return -EIO;
}
@@ -3366,13 +3416,12 @@ done:
if (msg) {
const char *sender = dbus_message_get_sender(msg);

- req->msg = dbus_message_ref(msg);
+ disc->msg = dbus_message_ref(msg);
/* Track the request owner to cancel it
* automatically if the owner exits */
- req->listener_id = g_dbus_add_disconnect_watch(dbus_conn,
- sender,
- discover_services_req_exit,
- req, NULL);
+ disc->sender_watch = g_dbus_add_disconnect_watch(dbus_conn,
+ sender, gatt_discovery_sender_exit,
+ device, NULL);
}

return 0;
--
1.8.1.3


2013-03-01 23:22:24

by Vinicius Costa Gomes

[permalink] [raw]
Subject: [RFC BlueZ v0 01/10] device: Clean up the ATT Connection attempt when it is cancelled

---
src/device.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/src/device.c b/src/device.c
index 4320234..8c1bd73 100644
--- a/src/device.c
+++ b/src/device.c
@@ -987,6 +987,8 @@ void device_request_disconnect(struct btd_device *device, DBusMessage *msg)
if (device->browse)
browse_request_cancel(device->browse);

+ attio_cleanup(device);
+
if (device->connect) {
DBusMessage *reply = btd_error_failed(device->connect,
"Cancelled");
--
1.8.1.3