2012-02-08 20:35:45

by Andre Guedes

[permalink] [raw]
Subject: [PATCH BlueZ 0/4] Cleanup browse_req

Hi all,

This patch series does some cleanup in struct browse_req. This series
depends on the last series I just sent to the ML.

BR,

Andre

Andre Guedes (4):
device: Use att_io in device_browse_primary
device: Remove unused function parameter
device: Move GAttrib reference to device
device: Remove unused fields from browse_req

src/device.c | 54 +++++++++++++++++++++---------------------------------
1 files changed, 21 insertions(+), 33 deletions(-)

--
1.7.9



2012-02-09 10:54:27

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH BlueZ 0/4] Cleanup browse_req

Hi Andre,

On Wed, Feb 08, 2012, Andre Guedes wrote:
> Hi all,
>
> This patch series does some cleanup in struct browse_req. This series
> depends on the last series I just sent to the ML.
>
> BR,
>
> Andre
>
> Andre Guedes (4):
> device: Use att_io in device_browse_primary
> device: Remove unused function parameter
> device: Move GAttrib reference to device
> device: Remove unused fields from browse_req
>
> src/device.c | 54 +++++++++++++++++++++---------------------------------
> 1 files changed, 21 insertions(+), 33 deletions(-)

Applied. Thanks.

Johan

2012-02-08 20:35:49

by Andre Guedes

[permalink] [raw]
Subject: [PATCH BlueZ 4/4] device: Remove unused fields from browse_req

---
src/device.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/src/device.c b/src/device.c
index a85dd57..cc7d11a 100644
--- a/src/device.c
+++ b/src/device.c
@@ -100,8 +100,6 @@ struct authentication_req {
struct browse_req {
DBusConnection *conn;
DBusMessage *msg;
- GIOChannel *io;
- GAttrib *attrib;
struct btd_device *device;
GSList *match_uuids;
GSList *profiles_added;
--
1.7.9


2012-02-08 20:35:48

by Andre Guedes

[permalink] [raw]
Subject: [PATCH BlueZ 3/4] device: Move GAttrib reference to device

Save GAttrib reference directly in struct btd_device instead of
the temporary browse_req. GAttrib instance scope should not be
restricted to primary service browsing context only, it needs to
be shared to upper layers.
---
src/device.c | 17 +++--------------
1 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/src/device.c b/src/device.c
index 5bc2d3b..a85dd57 100644
--- a/src/device.c
+++ b/src/device.c
@@ -177,8 +177,6 @@ static void browse_request_free(struct browse_req *req)
{
if (req->listener_id)
g_dbus_remove_watch(req->conn, req->listener_id);
- if (req->attrib)
- g_attrib_unref(req->attrib);
if (req->msg)
dbus_message_unref(req->msg);
if (req->conn)
@@ -1789,13 +1787,6 @@ static void primary_cb(GSList *services, guint8 status, gpointer user_data)
uuids = g_slist_append(uuids, prim->uuid);
}

- /*
- * Profiles may register attio callbacks in the probing callback.
- * GAttrib reference can be released if the registered callbacks
- * list is emtpy.
- */
- device->attrib = g_attrib_ref(req->attrib);
-
device_register_services(req->conn, device, g_slist_copy(services), -1);
device_probe_drivers(device, uuids);

@@ -1903,7 +1894,6 @@ static void browse_primary_connect_cb(GIOChannel *io, GError *gerr,
{
struct btd_device *device = user_data;
struct browse_req *req = device->browse;
- GAttrib *attrib;

g_io_channel_unref(device->att_io);
device->att_io = NULL;
@@ -1922,13 +1912,12 @@ static void browse_primary_connect_cb(GIOChannel *io, GError *gerr,
return;
}

- attrib = g_attrib_new(io);
- device->attachid = attrib_channel_attach(attrib, TRUE);
+ device->attrib = g_attrib_new(io);
+ device->attachid = attrib_channel_attach(device->attrib, TRUE);
if (device->attachid == 0)
error("Attribute server attach failure!");

- req->attrib = attrib;
- gatt_discover_primary(req->attrib, NULL, primary_cb, req);
+ gatt_discover_primary(device->attrib, NULL, primary_cb, req);
}

int device_browse_primary(struct btd_device *device, DBusConnection *conn,
--
1.7.9


2012-02-08 20:35:47

by Andre Guedes

[permalink] [raw]
Subject: [PATCH BlueZ 2/4] device: Remove unused function parameter

This patch removes the unused shutdown parameter from
browse_request_free function.
---
src/device.c | 17 +++++++----------
1 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/src/device.c b/src/device.c
index 23ca67b..5bc2d3b 100644
--- a/src/device.c
+++ b/src/device.c
@@ -173,7 +173,7 @@ static uint16_t uuid_list[] = {

static GSList *device_drivers = NULL;

-static void browse_request_free(struct browse_req *req, gboolean shutdown)
+static void browse_request_free(struct browse_req *req)
{
if (req->listener_id)
g_dbus_remove_watch(req->conn, req->listener_id);
@@ -213,7 +213,7 @@ static void browse_request_cancel(struct browse_req *req)
}

device->browse = NULL;
- browse_request_free(req, TRUE);
+ browse_request_free(req);
}

static void device_free(gpointer user_data)
@@ -1623,7 +1623,7 @@ cleanup:
}

device->browse = NULL;
- browse_request_free(req, FALSE);
+ browse_request_free(req);
}

static void browse_cb(sdp_list_t *recs, int err, gpointer user_data)
@@ -1774,13 +1774,11 @@ static void primary_cb(GSList *services, guint8 status, gpointer user_data)
struct browse_req *req = user_data;
struct btd_device *device = req->device;
GSList *l, *uuids = NULL;
- gboolean shutdown;

if (status) {
DBusMessage *reply;
reply = btd_error_failed(req->msg, att_ecode2str(status));
g_dbus_send_message(req->conn, reply);
- shutdown = TRUE;
goto done;
}

@@ -1816,11 +1814,10 @@ static void primary_cb(GSList *services, guint8 status, gpointer user_data)
create_device_reply(device, req);

store_services(device);
- shutdown = FALSE;

done:
device->browse = NULL;
- browse_request_free(req, shutdown);
+ browse_request_free(req);
}

static void att_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
@@ -1920,7 +1917,7 @@ static void browse_primary_connect_cb(GIOChannel *io, GError *gerr,
g_dbus_send_message(req->conn, reply);

device->browse = NULL;
- browse_request_free(req, TRUE);
+ browse_request_free(req);

return;
}
@@ -1961,7 +1958,7 @@ int device_browse_primary(struct btd_device *device, DBusConnection *conn,
BT_IO_OPT_INVALID);

if (device->att_io == NULL) {
- browse_request_free(req, FALSE);
+ browse_request_free(req);
return -EIO;
}

@@ -2013,7 +2010,7 @@ int device_browse_sdp(struct btd_device *device, DBusConnection *conn,

err = bt_search_service(&src, &device->bdaddr, &uuid, cb, req, NULL);
if (err < 0) {
- browse_request_free(req, FALSE);
+ browse_request_free(req);
return err;
}

--
1.7.9


2012-02-08 20:35:46

by Andre Guedes

[permalink] [raw]
Subject: [PATCH BlueZ 1/4] device: Use att_io in device_browse_primary

In order to have the same approach as att_connect function,
device_browse_primary function should save the reference to
GIOChannel at device->att_io. This way, we can properly shut
it down if the browse request is canceled during the connection
attempt.
---
src/device.c | 18 +++++++++++-------
1 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/src/device.c b/src/device.c
index bd25a27..23ca67b 100644
--- a/src/device.c
+++ b/src/device.c
@@ -179,11 +179,6 @@ static void browse_request_free(struct browse_req *req, gboolean shutdown)
g_dbus_remove_watch(req->conn, req->listener_id);
if (req->attrib)
g_attrib_unref(req->attrib);
- if (req->io) {
- if (shutdown)
- g_io_channel_shutdown(req->io, FALSE, NULL);
- g_io_channel_unref(req->io);
- }
if (req->msg)
dbus_message_unref(req->msg);
if (req->conn)
@@ -211,6 +206,12 @@ static void browse_request_cancel(struct browse_req *req)

bt_cancel_discovery(&src, &device->bdaddr);

+ if (device->att_io) {
+ g_io_channel_shutdown(device->att_io, FALSE, NULL);
+ g_io_channel_unref(device->att_io);
+ device->att_io = NULL;
+ }
+
device->browse = NULL;
browse_request_free(req, TRUE);
}
@@ -1907,6 +1908,9 @@ static void browse_primary_connect_cb(GIOChannel *io, GError *gerr,
struct browse_req *req = device->browse;
GAttrib *attrib;

+ g_io_channel_unref(device->att_io);
+ device->att_io = NULL;
+
if (gerr) {
DBusMessage *reply;

@@ -1948,7 +1952,7 @@ int device_browse_primary(struct btd_device *device, DBusConnection *conn,

sec_level = secure ? BT_IO_SEC_HIGH : BT_IO_SEC_LOW;

- req->io = bt_io_connect(BT_IO_L2CAP, browse_primary_connect_cb,
+ device->att_io = bt_io_connect(BT_IO_L2CAP, browse_primary_connect_cb,
device, NULL, NULL,
BT_IO_OPT_SOURCE_BDADDR, &src,
BT_IO_OPT_DEST_BDADDR, &device->bdaddr,
@@ -1956,7 +1960,7 @@ int device_browse_primary(struct btd_device *device, DBusConnection *conn,
BT_IO_OPT_SEC_LEVEL, sec_level,
BT_IO_OPT_INVALID);

- if (req->io == NULL) {
+ if (device->att_io == NULL) {
browse_request_free(req, FALSE);
return -EIO;
}
--
1.7.9