[PATCH BlueZ 0/4] Cleanup browse_req

[PATCH BlueZ 0/4] Cleanup browse_req

[Andre Guedes]

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

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

[Andre Guedes]

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

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

[Andre Guedes]

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

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

[Andre Guedes]

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

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

[Andre Guedes]

---
 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

Re: [PATCH BlueZ 0/4] Cleanup browse_req

[Johan Hedberg]

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