Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH BlueZ 4/4] gas: Fix not sending response to indication
From: Vinicius Costa Gomes @ 2013-01-28 23:48 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Vinicius Costa Gomes
In-Reply-To: <1359416891-12740-1-git-send-email-vinicius.gomes@openbossa.org>

Even if the remote device is not bonded, we should send the response to the
indication. If we don't the remote device may disconnect.
---
 profiles/gatt/gas.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/profiles/gatt/gas.c b/profiles/gatt/gas.c
index e0edbf3..8477dca 100644
--- a/profiles/gatt/gas.c
+++ b/profiles/gatt/gas.c
@@ -184,16 +184,16 @@ static void indication_cb(const uint8_t *pdu, uint16_t len, gpointer user_data)
 
 	DBG("Service Changed start: 0x%04X end: 0x%04X", start, end);
 
-	if (device_is_bonded(gas->device) == FALSE) {
-		DBG("Ignoring Service Changed: device is not bonded");
-		return;
-	}
-
 	/* Confirming indication received */
 	opdu = g_attrib_get_buffer(gas->attrib, &plen);
 	olen = enc_confirmation(opdu, plen);
 	g_attrib_send(gas->attrib, 0, opdu, olen, NULL, NULL, NULL);
 
+	if (device_is_bonded(gas->device) == FALSE) {
+		DBG("Ignoring Service Changed: device is not bonded");
+		return;
+	}
+
 	btd_device_gatt_set_service_changed(gas->device, start, end);
 }
 
-- 
1.8.1.1


^ permalink raw reply related

* [PATCH BlueZ 3/4] gas: Move all the code to only one file
From: Vinicius Costa Gomes @ 2013-01-28 23:48 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Vinicius Costa Gomes
In-Reply-To: <1359416891-12740-1-git-send-email-vinicius.gomes@openbossa.org>

Our Generic Attribute/Access Service plugin is small and simple enough
to be kept in only one file.
---
 Makefile.plugins        |  4 +--
 profiles/gatt/gas.c     | 46 ++++++++++++++++++++++++++++
 profiles/gatt/main.c    | 47 -----------------------------
 profiles/gatt/manager.c | 79 -------------------------------------------------
 profiles/gatt/manager.h | 24 ---------------
 5 files changed, 47 insertions(+), 153 deletions(-)
 delete mode 100644 profiles/gatt/main.c
 delete mode 100644 profiles/gatt/manager.c
 delete mode 100644 profiles/gatt/manager.h

diff --git a/Makefile.plugins b/Makefile.plugins
index faab011..f497782 100644
--- a/Makefile.plugins
+++ b/Makefile.plugins
@@ -69,9 +69,7 @@ builtin_sources += profiles/health/mcap_lib.h profiles/health/mcap_internal.h \
 endif
 
 builtin_modules += gatt
-builtin_sources += profiles/gatt/main.c profiles/gatt/manager.h \
-			profiles/gatt/manager.c profiles/gatt/gas.h \
-			profiles/gatt/gas.c
+builtin_sources += profiles/gatt/gas.c
 
 builtin_modules += scanparam
 builtin_sources += profiles/scanparam/scan.c
diff --git a/profiles/gatt/gas.c b/profiles/gatt/gas.c
index 429850b..e0edbf3 100644
--- a/profiles/gatt/gas.c
+++ b/profiles/gatt/gas.c
@@ -35,8 +35,10 @@
 #include <btio/btio.h>
 
 #include "lib/uuid.h"
+#include "plugin.h"
 #include "adapter.h"
 #include "device.h"
+#include "profile.h"
 #include "attrib/att.h"
 #include "attrib/gattrib.h"
 #include "attio.h"
@@ -405,3 +407,47 @@ void gas_unregister(struct btd_device *device)
 	devices = g_slist_remove(devices, gas);
 	gas_free(gas);
 }
+
+static int gatt_driver_probe(struct btd_profile *p, struct btd_device *device,
+								GSList *uuids)
+{
+	struct gatt_primary *gap, *gatt;
+
+	gap = btd_device_get_primary(device, GAP_UUID);
+	gatt = btd_device_get_primary(device, GATT_UUID);
+
+	if (gap == NULL || gatt == NULL) {
+		error("GAP and GATT are mandatory");
+		return -EINVAL;
+	}
+
+	return gas_register(device, &gap->range, &gatt->range);
+}
+
+static void gatt_driver_remove(struct btd_profile *p,
+						struct btd_device *device)
+{
+	gas_unregister(device);
+}
+
+static struct btd_profile gatt_profile = {
+	.name		= "gap-gatt-profile",
+	.remote_uuids	= BTD_UUIDS(GAP_UUID, GATT_UUID),
+	.device_probe	= gatt_driver_probe,
+	.device_remove	= gatt_driver_remove
+};
+
+static int gatt_init(void)
+{
+	btd_profile_register(&gatt_profile);
+
+	return 0;
+}
+
+static void gatt_exit(void)
+{
+	btd_profile_unregister(&gatt_profile);
+}
+
+BLUETOOTH_PLUGIN_DEFINE(gatt, VERSION, BLUETOOTH_PLUGIN_PRIORITY_DEFAULT,
+					gatt_init, gatt_exit)
diff --git a/profiles/gatt/main.c b/profiles/gatt/main.c
deleted file mode 100644
index ecd4455..0000000
--- a/profiles/gatt/main.c
+++ /dev/null
@@ -1,47 +0,0 @@
-/*
- *
- *  BlueZ - Bluetooth protocol stack for Linux
- *
- *  Copyright (C) 2012  Instituto Nokia de Tecnologia - INdT
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; either version 2 of the License, or
- *  (at your option) any later version.
- *
- *  This program is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with this program; if not, write to the Free Software
- *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
- *
- */
-
-#ifdef HAVE_CONFIG_H
-#include <config.h>
-#endif
-
-#include <stdint.h>
-#include <glib.h>
-#include <errno.h>
-
-#include "plugin.h"
-#include "manager.h"
-#include "hcid.h"
-#include "log.h"
-
-static int gatt_init(void)
-{
-	return gatt_manager_init();
-}
-
-static void gatt_exit(void)
-{
-	gatt_manager_exit();
-}
-
-BLUETOOTH_PLUGIN_DEFINE(gatt, VERSION, BLUETOOTH_PLUGIN_PRIORITY_DEFAULT,
-					gatt_init, gatt_exit)
diff --git a/profiles/gatt/manager.c b/profiles/gatt/manager.c
deleted file mode 100644
index 2f2bd14..0000000
--- a/profiles/gatt/manager.c
+++ /dev/null
@@ -1,79 +0,0 @@
-/*
- *
- *  BlueZ - Bluetooth protocol stack for Linux
- *
- *  Copyright (C) 2012  Instituto Nokia de Tecnologia - INdT
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; either version 2 of the License, or
- *  (at your option) any later version.
- *
- *  This program is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with this program; if not, write to the Free Software
- *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
- *
- */
-
-#ifdef HAVE_CONFIG_H
-#include "config.h"
-#endif
-
-#include <glib.h>
-#include <errno.h>
-#include <stdbool.h>
-
-#include "lib/uuid.h"
-#include "adapter.h"
-#include "device.h"
-#include "profile.h"
-#include "attrib/att.h"
-#include "attrib/gattrib.h"
-#include "attrib/gatt.h"
-#include "gas.h"
-#include "log.h"
-#include "manager.h"
-
-static int gatt_driver_probe(struct btd_profile *p, struct btd_device *device,
-								GSList *uuids)
-{
-	struct gatt_primary *gap, *gatt;
-
-	gap = btd_device_get_primary(device, GAP_UUID);
-	gatt = btd_device_get_primary(device, GATT_UUID);
-
-	if (gap == NULL || gatt == NULL) {
-		error("GAP and GATT are mandatory");
-		return -EINVAL;
-	}
-
-	return gas_register(device, &gap->range, &gatt->range);
-}
-
-static void gatt_driver_remove(struct btd_profile *p,
-						struct btd_device *device)
-{
-	gas_unregister(device);
-}
-
-static struct btd_profile gatt_profile = {
-	.name		= "gap-gatt-profile",
-	.remote_uuids	= BTD_UUIDS(GAP_UUID, GATT_UUID),
-	.device_probe	= gatt_driver_probe,
-	.device_remove	= gatt_driver_remove
-};
-
-int gatt_manager_init(void)
-{
-	return btd_profile_register(&gatt_profile);
-}
-
-void gatt_manager_exit(void)
-{
-	btd_profile_unregister(&gatt_profile);
-}
diff --git a/profiles/gatt/manager.h b/profiles/gatt/manager.h
deleted file mode 100644
index 502fceb..0000000
--- a/profiles/gatt/manager.h
+++ /dev/null
@@ -1,24 +0,0 @@
-/*
- *
- *  BlueZ - Bluetooth protocol stack for Linux
- *
- *  Copyright (C) 2012  Instituto Nokia de Tecnologia - INdT
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; either version 2 of the License, or
- *  (at your option) any later version.
- *
- *  This program is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with this program; if not, write to the Free Software
- *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
- *
- */
-
-int gatt_manager_init(void);
-void gatt_manager_exit(void);
-- 
1.8.1.1


^ permalink raw reply related

* [PATCH BlueZ 2/4] device: Fix missing PDUs during encryption procedure
From: Vinicius Costa Gomes @ 2013-01-28 23:48 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Vinicius Costa Gomes
In-Reply-To: <1359416891-12740-1-git-send-email-vinicius.gomes@openbossa.org>

In case the remote device sends an ATT PDU while encryption is going
on, we may lose it because the ATT socket (with security level medium),
would only be attached when encryption finishes.
---
 src/device.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/device.c b/src/device.c
index ceaa575..0d2d3ee 100644
--- a/src/device.c
+++ b/src/device.c
@@ -3167,7 +3167,6 @@ 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;
 	char addr[18];
@@ -3185,21 +3184,18 @@ int device_connect_le(struct btd_device *dev)
 	attcb->success = att_success_cb;
 	attcb->user_data = dev;
 
-	if (dev->paired)
-		sec_level = BT_IO_SEC_MEDIUM;
-	else
-		sec_level = BT_IO_SEC_LOW;
-
 	/*
 	 * This connection will help us catch any PDUs that comes before
-	 * pairing finishes
+	 * pairing finishes. Its security level is low, because we don't
+	 * want to miss any PDU that may come before the encryption
+	 * procedure finishes
 	 */
 	io = bt_io_connect(att_connect_cb, attcb, 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,
 			BT_IO_OPT_CID, ATT_CID,
-			BT_IO_OPT_SEC_LEVEL, sec_level,
+			BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
 			BT_IO_OPT_INVALID);
 
 	if (io == NULL) {
-- 
1.8.1.1


^ permalink raw reply related

* [PATCH BlueZ 1/4] device: Fix invalid memory access during Find Included
From: Vinicius Costa Gomes @ 2013-01-28 23:48 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Vinicius Costa Gomes

When doing the Find Included Services GATT procedure, the status of the ATT
procedure was being ignored, and in the case of a timeout it is possible to
crash bluetooth with an invalid memory access.

Valgrind log:

==1755== Invalid read of size 8
==1755==    at 0x46971A: find_included_cb (device.c:2964)
==1755==    by 0x4465AE: isd_unref (gatt.c:92)
==1755==    by 0x446885: find_included_cb (gatt.c:425)
==1755==    by 0x448266: disconnect_timeout (gattrib.c:269)
==1755==    by 0x4E76BCA: g_timeout_dispatch (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755==    by 0x4E76044: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755==    by 0x4E76377: g_main_context_iterate.isra.24 (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755==    by 0x4E76771: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755==    by 0x40A2EE: main (main.c:583)
==1755==  Address 0x69530a8 is 8 bytes inside a block of size 64 free'd
==1755==    at 0x4C2874F: free (vg_replace_malloc.c:446)
==1755==    by 0x40BFA6: service_filter (watch.c:486)
==1755==    by 0x40BC6A: message_filter (watch.c:554)
==1755==    by 0x5160A1D: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.7.2)
==1755==    by 0x40AAB7: message_dispatch (mainloop.c:76)
==1755==    by 0x4E76BCA: g_timeout_dispatch (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755==    by 0x4E76044: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755==    by 0x4E76377: g_main_context_iterate.isra.24 (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755==    by 0x4E76771: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755==    by 0x40A2EE: main (main.c:583)
==1755==
==1755== Invalid read of size 8
==1755==    at 0x4486D5: g_attrib_get_buffer (gattrib.c:657)
==1755==    by 0x4467C5: find_included (gatt.c:363)
==1755==    by 0x4465AE: isd_unref (gatt.c:92)
==1755==    by 0x446885: find_included_cb (gatt.c:425)
==1755==    by 0x448266: disconnect_timeout (gattrib.c:269)
==1755==    by 0x4E76BCA: g_timeout_dispatch (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755==    by 0x4E76044: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755==    by 0x4E76377: g_main_context_iterate.isra.24 (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755==    by 0x4E76771: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755==    by 0x40A2EE: main (main.c:583)
==1755==  Address 0x18 is not stack'd, malloc'd or (recently) free'd
==1755==
==1755==
==1755== Process terminating with default action of signal 11 (SIGSEGV)
==1755==  Access not within mapped region at address 0x18
==1755==    at 0x4486D5: g_attrib_get_buffer (gattrib.c:657)
==1755==    by 0x4467C5: find_included (gatt.c:363)
==1755==    by 0x4465AE: isd_unref (gatt.c:92)
==1755==    by 0x446885: find_included_cb (gatt.c:425)
==1755==    by 0x448266: disconnect_timeout (gattrib.c:269)
==1755==    by 0x4E76BCA: g_timeout_dispatch (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755==    by 0x4E76044: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755==    by 0x4E76377: g_main_context_iterate.isra.24 (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755==    by 0x4E76771: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3400.2)
==1755==    by 0x40A2EE: main (main.c:583)
---
 attrib/gatt.c | 5 ++++-
 src/device.c  | 6 ++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/attrib/gatt.c b/attrib/gatt.c
index d54feac..44d3eb6 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -89,7 +89,10 @@ static void isd_unref(struct included_discovery *isd)
 	if (g_atomic_int_dec_and_test(&isd->refs) == FALSE)
 		return;
 
-	isd->cb(isd->includes, isd->err, isd->user_data);
+	if (isd->err)
+		isd->cb(NULL, isd->err, isd->user_data);
+	else
+		isd->cb(isd->includes, isd->err, isd->user_data);
 
 	g_slist_free_full(isd->includes, g_free);
 	g_attrib_unref(isd->attrib);
diff --git a/src/device.c b/src/device.c
index 34902b3..ceaa575 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2988,6 +2988,12 @@ static void find_included_cb(GSList *includes, uint8_t status,
 	struct gatt_primary *prim;
 	GSList *l;
 
+	if (status != 0) {
+		error("Find included services failed: %s (%d)",
+					att_ecode2str(status), status);
+		goto done;
+	}
+
 	if (includes == NULL)
 		goto done;
 
-- 
1.8.1.1


^ permalink raw reply related

* Re: [PATCH BlueZ 1/2 v2] thermometer: Fix crash while unregistering adapter
From: Johan Hedberg @ 2013-01-28 22:39 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1359409441-21066-1-git-send-email-luiz.dentz@gmail.com>

Hi Luiz,

On Mon, Jan 28, 2013, Luiz Augusto von Dentz wrote:
> Invalid read of size 8
>    at 0x448200: g_attrib_unregister (gattrib.c:722)
>    by 0x440476: destroy_thermometer (thermometer.c:167)
>    by 0x40D849: remove_interface (object.c:656)
>    by 0x40DAA9: g_dbus_unregister_interface (object.c:1413)
>    by 0x3DF7A63C9C: g_slist_foreach (gslist.c:894)
>    by 0x469656: device_remove (device.c:2200)
>    by 0x45CDC1: adapter_remove (adapter.c:3884)
>    by 0x45F146: index_removed (adapter.c:5442)
>    by 0x46BC17: received_data (mgmt.c:252)
>    by 0x3DF7A47A74: g_main_context_dispatch (gmain.c:2715)
>    by 0x3DF7A47DA7: g_main_context_iterate.isra.24 (gmain.c:3290)
>    by 0x3DF7A481A1: g_main_loop_run (gmain.c:3484)
>  Address 0x40 is not stack'd, malloc'd or (recently) free'd
> ---
> v2: Print a warning if invalid id is passed to g_attrib_unregister
> 
>  profiles/thermometer/thermometer.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

Both patches have been applied. Thanks.

Johan

^ permalink raw reply

* [PATCH BlueZ 2/2 v2] attrib: Don't attempt to unregister event id 0
From: Luiz Augusto von Dentz @ 2013-01-28 21:44 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1359409441-21066-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Id 0 is considered invalid so the code should not even try to lookup for
it in the event list instead print a warning and return FALSE
immediatelly.
---
 attrib/gattrib.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index 58f19d0..01c19f9 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -719,6 +719,11 @@ gboolean g_attrib_unregister(GAttrib *attrib, guint id)
 	struct event *evt;
 	GSList *l;
 
+	if (id == 0) {
+		warn("%s: invalid id", __FUNCTION__);
+		return FALSE;
+	}
+
 	l = g_slist_find_custom(attrib->events, GUINT_TO_POINTER(id),
 							event_cmp_by_id);
 	if (l == NULL)
-- 
1.8.1


^ permalink raw reply related

* [PATCH BlueZ 1/2 v2] thermometer: Fix crash while unregistering adapter
From: Luiz Augusto von Dentz @ 2013-01-28 21:44 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Invalid read of size 8
   at 0x448200: g_attrib_unregister (gattrib.c:722)
   by 0x440476: destroy_thermometer (thermometer.c:167)
   by 0x40D849: remove_interface (object.c:656)
   by 0x40DAA9: g_dbus_unregister_interface (object.c:1413)
   by 0x3DF7A63C9C: g_slist_foreach (gslist.c:894)
   by 0x469656: device_remove (device.c:2200)
   by 0x45CDC1: adapter_remove (adapter.c:3884)
   by 0x45F146: index_removed (adapter.c:5442)
   by 0x46BC17: received_data (mgmt.c:252)
   by 0x3DF7A47A74: g_main_context_dispatch (gmain.c:2715)
   by 0x3DF7A47DA7: g_main_context_iterate.isra.24 (gmain.c:3290)
   by 0x3DF7A481A1: g_main_loop_run (gmain.c:3484)
 Address 0x40 is not stack'd, malloc'd or (recently) free'd
---
v2: Print a warning if invalid id is passed to g_attrib_unregister

 profiles/thermometer/thermometer.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/profiles/thermometer/thermometer.c b/profiles/thermometer/thermometer.c
index 0cf14e6..1b299e7 100644
--- a/profiles/thermometer/thermometer.c
+++ b/profiles/thermometer/thermometer.c
@@ -164,12 +164,12 @@ static void destroy_thermometer(gpointer user_data)
 	if (t->attioid > 0)
 		btd_device_remove_attio_callback(t->dev, t->attioid);
 
-	g_attrib_unregister(t->attrib, t->attio_measurement_id);
-	g_attrib_unregister(t->attrib, t->attio_intermediate_id);
-	g_attrib_unregister(t->attrib, t->attio_interval_id);
-
-	if (t->attrib != NULL)
+	if (t->attrib != NULL) {
+		g_attrib_unregister(t->attrib, t->attio_measurement_id);
+		g_attrib_unregister(t->attrib, t->attio_intermediate_id);
+		g_attrib_unregister(t->attrib, t->attio_interval_id);
 		g_attrib_unref(t->attrib);
+	}
 
 	btd_device_unref(t->dev);
 	g_free(t->svc_range);
-- 
1.8.1


^ permalink raw reply related

* RE: LE auto connect
From: Damjan Cvetko @ 2013-01-28 21:43 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <20130128182332.GA23272@x220.ger.corp.intel.com>

Hey Johan.

> Thanks. I pushed a cleaned up version of the adapter.c changes since that needs to be in a separate patch anyway. Could you create a proper git patch (git format-patch & git send-email) from the first part of the patch and resend it?

Sent a patch to the list, hope it's ok now.

BR.
Damjan

^ permalink raw reply

* [PATCH BlueZ 2/2] attrib: Don't attempt to unregister event id 0
From: Luiz Augusto von Dentz @ 2013-01-28 21:30 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1359408630-20210-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Id 0 is considered invalid so the code should not even try to lookup for
it in the event list.
---
 attrib/gattrib.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index 58f19d0..ca73491 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -719,6 +719,9 @@ gboolean g_attrib_unregister(GAttrib *attrib, guint id)
 	struct event *evt;
 	GSList *l;
 
+	if (id == 0)
+		return FALSE;
+
 	l = g_slist_find_custom(attrib->events, GUINT_TO_POINTER(id),
 							event_cmp_by_id);
 	if (l == NULL)
-- 
1.8.1


^ permalink raw reply related

* [PATCH BlueZ 1/2] thermometer: Fix crash while unregistering adapter
From: Luiz Augusto von Dentz @ 2013-01-28 21:30 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Invalid read of size 8
   at 0x448200: g_attrib_unregister (gattrib.c:722)
   by 0x440476: destroy_thermometer (thermometer.c:167)
   by 0x40D849: remove_interface (object.c:656)
   by 0x40DAA9: g_dbus_unregister_interface (object.c:1413)
   by 0x3DF7A63C9C: g_slist_foreach (gslist.c:894)
   by 0x469656: device_remove (device.c:2200)
   by 0x45CDC1: adapter_remove (adapter.c:3884)
   by 0x45F146: index_removed (adapter.c:5442)
   by 0x46BC17: received_data (mgmt.c:252)
   by 0x3DF7A47A74: g_main_context_dispatch (gmain.c:2715)
   by 0x3DF7A47DA7: g_main_context_iterate.isra.24 (gmain.c:3290)
   by 0x3DF7A481A1: g_main_loop_run (gmain.c:3484)
 Address 0x40 is not stack'd, malloc'd or (recently) free'd
---
 profiles/thermometer/thermometer.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/profiles/thermometer/thermometer.c b/profiles/thermometer/thermometer.c
index 0cf14e6..1b299e7 100644
--- a/profiles/thermometer/thermometer.c
+++ b/profiles/thermometer/thermometer.c
@@ -164,12 +164,12 @@ static void destroy_thermometer(gpointer user_data)
 	if (t->attioid > 0)
 		btd_device_remove_attio_callback(t->dev, t->attioid);
 
-	g_attrib_unregister(t->attrib, t->attio_measurement_id);
-	g_attrib_unregister(t->attrib, t->attio_intermediate_id);
-	g_attrib_unregister(t->attrib, t->attio_interval_id);
-
-	if (t->attrib != NULL)
+	if (t->attrib != NULL) {
+		g_attrib_unregister(t->attrib, t->attio_measurement_id);
+		g_attrib_unregister(t->attrib, t->attio_intermediate_id);
+		g_attrib_unregister(t->attrib, t->attio_interval_id);
 		g_attrib_unref(t->attrib);
+	}
 
 	btd_device_unref(t->dev);
 	g_free(t->svc_range);
-- 
1.8.1


^ permalink raw reply related

* [PATCH] Add heartrate monitoring LE device to auto connect list.
From: Damjan Cvetko @ 2013-01-28 21:27 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Damjan Cvetko

---
 profiles/heartrate/heartrate.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/profiles/heartrate/heartrate.c b/profiles/heartrate/heartrate.c
index 5c56d3f..1788d4f 100644
--- a/profiles/heartrate/heartrate.c
+++ b/profiles/heartrate/heartrate.c
@@ -801,6 +801,8 @@ static int heartrate_device_register(struct btd_device *device,
 	hr->attioid = btd_device_add_attio_callback(device, attio_connected_cb,
 						attio_disconnected_cb, hr);
 
+	device_set_auto_connect(device, TRUE);
+
 	return 0;
 }
 
-- 
1.7.10.4


^ permalink raw reply related

* Re: [PATCH BlueZ] media: Fix not finding endpoints with different case UUIDs
From: Luiz Augusto von Dentz @ 2013-01-28 20:20 UTC (permalink / raw)
  To: Vinicius Costa Gomes; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1359401256-14512-1-git-send-email-vinicius.gomes@openbossa.org>

Hi Vinicius,

On Mon, Jan 28, 2013 at 1:27 PM, Vinicius Costa Gomes
<vinicius.gomes@openbossa.org> wrote:
> When multiple Endpoints are registered with different case UUIDs,
> the only UUIDs found were those with matching case.
> ---
>  profiles/audio/media.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/profiles/audio/media.c b/profiles/audio/media.c
> index 0ae9932..59fc840 100644
> --- a/profiles/audio/media.c
> +++ b/profiles/audio/media.c
> @@ -170,7 +170,7 @@ static struct media_endpoint *media_adapter_find_endpoint(
>                 if (path && g_strcmp0(endpoint->path, path) != 0)
>                         continue;
>
> -               if (uuid && g_strcmp0(endpoint->uuid, uuid) != 0)
> +               if (uuid && strcasecmp(endpoint->uuid, uuid) != 0)
>                         continue;
>
>                 return endpoint;
> @@ -692,7 +692,7 @@ static bool endpoint_properties_get(const char *uuid,
>         for (l = adapter->endpoints; l; l = l->next) {
>                 struct media_endpoint *endpoint = l->data;
>
> -               if (g_strcmp0(endpoint->uuid, uuid) != 0)
> +               if (strcasecmp(endpoint->uuid, uuid) != 0)
>                         continue;
>
>                 append_endpoint(endpoint, &dict);
> --
> 1.8.1.1

Applied, thanks


--
Luiz Augusto von Dentz

^ permalink raw reply

* [PATCH BlueZ] media: Fix not finding endpoints with different case UUIDs
From: Vinicius Costa Gomes @ 2013-01-28 19:27 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Vinicius Costa Gomes

When multiple Endpoints are registered with different case UUIDs,
the only UUIDs found were those with matching case.
---
 profiles/audio/media.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index 0ae9932..59fc840 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -170,7 +170,7 @@ static struct media_endpoint *media_adapter_find_endpoint(
 		if (path && g_strcmp0(endpoint->path, path) != 0)
 			continue;
 
-		if (uuid && g_strcmp0(endpoint->uuid, uuid) != 0)
+		if (uuid && strcasecmp(endpoint->uuid, uuid) != 0)
 			continue;
 
 		return endpoint;
@@ -692,7 +692,7 @@ static bool endpoint_properties_get(const char *uuid,
 	for (l = adapter->endpoints; l; l = l->next) {
 		struct media_endpoint *endpoint = l->data;
 
-		if (g_strcmp0(endpoint->uuid, uuid) != 0)
+		if (strcasecmp(endpoint->uuid, uuid) != 0)
 			continue;
 
 		append_endpoint(endpoint, &dict);
-- 
1.8.1.1


^ permalink raw reply related

* Re: [PATCH 2/2 v2] Bluetooth: Increment Management interface revision
From: Gustavo Padovan @ 2013-01-28 18:33 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1359297121-9022-2-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

* Johan Hedberg <johan.hedberg@gmail.com> [2013-01-27 08:32:01 -0600]:

> From: Johan Hedberg <johan.hedberg@intel.com>
> 
> This patch increments the management interface revision due to the
> various fixes, improvements and other changes that have gone in lately.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  net/bluetooth/mgmt.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Both patches have been applied to bluetooth-next. Thanks.

	Gustavo

^ permalink raw reply

* Re: [PATCH 9/9 v4] Bluetooth: Add support for 128-bit UUIDs in EIR data
From: Gustavo Padovan @ 2013-01-28 18:28 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1359239495-3444-10-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

* Johan Hedberg <johan.hedberg@gmail.com> [2013-01-27 00:31:35 +0200]:

> From: Johan Hedberg <johan.hedberg@intel.com>
> 
> This patch adds the necessary code for encoding a list of 128-bit UUIDs
> into the EIR data.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> Acked-by: Marcel Holtmann <marcel@holtmann.org>
> ---
>  net/bluetooth/mgmt.c |   34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)

All 9 patches have been applied. Thanks.

	Gustavo

^ permalink raw reply

* Re: LE auto connect
From: Johan Hedberg @ 2013-01-28 18:23 UTC (permalink / raw)
  To: Damjan Cvetko; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <CEE0DF8E7B2727428D202DA3F3FBC4DD8C753ABB@PULWAR.mtk.lan>

Hi Damjan,

On Mon, Jan 28, 2013, Damjan Cvetko wrote:
> I checked out latest code, and found that LE auto connection does not
> work at all. Dug a bit in the code to see there is no relevant
> implementation (related to connect_list).
>
> Here a few lines that make it work.

Thanks. I pushed a cleaned up version of the adapter.c changes since
that needs to be in a separate patch anyway. Could you create a proper
git patch (git format-patch & git send-email) from the first part of the
patch and resend it?

Johan

^ permalink raw reply

* Re: [PATCH 1/2] Bluetooth: Fix L2CAP socket shutdown for LE connections
From: Andre Guedes @ 2013-01-28 18:17 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1359170902.16748.6.camel@aeonflux>

Hi Marcel,

On Sat, Jan 26, 2013 at 12:28 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Andre,
>
>> During the L2CAP socket shutdown, the LE connection is not terminated
>> as expected. This bug can be reproduced using l2test tool. Once the
>> LE connection is established, kill l2test and the LE connection will
>> not terminate.
>>
>> This patch fixes hci_conn_disconnect function so it is able to
>> terminate LE connections.
>>
>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>> ---
>>  net/bluetooth/hci_conn.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
>> index 25bfce0..0492949 100644
>> --- a/net/bluetooth/hci_conn.c
>> +++ b/net/bluetooth/hci_conn.c
>> @@ -250,6 +250,7 @@ static void hci_conn_disconnect(struct hci_conn *conn)
>>
>>       switch (conn->type) {
>>       case ACL_LINK:
>> +     case LE_LINK:
>>               hci_acl_disconn(conn, reason);
>>               break;
>>       case AMP_LINK:
>
> I am wondering if we are not missing SCO_LINK here either.

Yes, we have the same problem with SCO socket shutdown.

I'll fix it and add the patch to this patchset.

Regards,

Andre

^ permalink raw reply

* LE auto connect
From: Damjan Cvetko @ 2013-01-28 12:09 UTC (permalink / raw)
  To: linux-bluetooth@vger.kernel.org

I checked out latest code, and found that LE auto connection does not work at all. Dug a bit in the code to see there is no relevant implementation (related to connect_list).
Here a few lines that make it work.

Best
Damjan

diff --git a/profiles/heartrate/heartrate.c b/profiles/heartrate/heartrate.c
index 5c56d3f..1788d4f 100644
--- a/profiles/heartrate/heartrate.c
+++ b/profiles/heartrate/heartrate.c
@@ -801,6 +801,8 @@ static int heartrate_device_register(struct btd_device *device,
        hr->attioid = btd_device_add_attio_callback(device, attio_connected_cb,
                                                attio_disconnected_cb, hr);

+       device_set_auto_connect(device, TRUE);
+
        return 0;
}

diff --git a/src/adapter.c b/src/adapter.c
index 43a9a3a..20667c8 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -4071,6 +4071,12 @@ static void update_found_devices(struct btd_adapter *adapter,

done:
        if (device_is_le(dev) && g_slist_find(adapter->connect_list, dev)) {
+               if (!device_is_connected(dev)) {
+                       err = device_connect_le(dev);
+                       if (err < 0) {
+                               DBG("LE auto connection failed %s (%d)", strerror(-err), -err);
+                       }
+               }
        }
}


^ permalink raw reply related

* Re: What is involved in supporting SCO packets in HCI_RAW mode?
From: Eponymous - @ 2013-01-28  9:45 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1359170626.16748.4.camel@aeonflux>

Thanks for getting back to me so quickly Marcel.

Yes that sounds logical. I think it sounds like we are wanting connect
using USB another way in this case rather than trying to re-engineer
BlueZ to work in a way which it wasn't intended.

Cheers.

On Sat, Jan 26, 2013 at 3:23 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi,
>
>> After the patch was added to btusb.c to allow ACL packets to be
>> sent/received in HCI_RAW mode I was wondering what would be involved
>> in allowing raw SCO packets to be sent/received?
>>
>> This would be a useful feature for someone like me who works with
>> Bluetooth firmware at this level on a daily basis.
>>
>> However, I can understand there may be some limitations.
>>
>> Is this something that can be realistically done?
>
> I do not see a way to do this properly. The main problem here is the USB
> transport. It requires special settings and active URBs.
>
> Once you switch it to HCI_RAW mode, the Bluetooth host stack does no
> command/event tracking and with that no connection tracking anymore.
> This means it can not inform the driver about any changes. And for USB
> it is required to change its alternate setting based on the number of
> connections and start ISOC URBs. Having ISOC URBs submitted all the time
> is not feasible either since that consumes way too much power.
>
> For other transports like UART, the SCO packets in HCI_RAW mode should
> still work.
>
> Regards
>
> Marcel
>
>

^ permalink raw reply

* Re: [PATCH BlueZ] media: Fix custom property registration for multiple adapters
From: Johan Hedberg @ 2013-01-28  0:03 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1358775253-29202-1-git-send-email-luiz.dentz@gmail.com>

Hi Luiz,

On Mon, Jan 21, 2013, Luiz Augusto von Dentz wrote:
> The function btd_profile_add_custom_prop register a custom property for
> an UUID which is valid for every adapter so its user_data cannot be tied
> to a single adapter.
> 
> To fix this now NULL is passed as user_data and the callbacks checks if
> the adapter passed has been registered and if there is an endpoint for
> the UUID requested.
> ---
>  profiles/audio/media.c | 27 +++++++++++++++++++++++----
>  1 file changed, 23 insertions(+), 4 deletions(-)

Applied. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH BlueZ 1/3 v3] AVRCP: Fix not handling commands while browsing is connecting
From: Johan Hedberg @ 2013-01-28  0:00 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1359330736-9190-1-git-send-email-luiz.dentz@gmail.com>

Hi Luiz,

On Sun, Jan 27, 2013, Luiz Augusto von Dentz wrote:
> With introdution of browsing channel the .init callback is called when
> browsing channel connection completes, but in the meantime the remote
> device can send commands over control channel.
> 
> To fix this the init callaback of control and browsing channel are now
> separated into .init_control and .init_browsing so the handler can be
> register as soon the respective channel connection completes.
> ---
> v2: Separate control and browsing inits and use handler id to track if channel
> has been initialized or not.
> v3: Adds patches 2 and 3
> 
>  profiles/audio/avrcp.c | 73 +++++++++++++++++++++++++++++---------------------
>  1 file changed, 42 insertions(+), 31 deletions(-)

All three patches have been applied. Thanks.

Johan

^ permalink raw reply

* [PATCH BlueZ 3/3 v3] AVCTP: Add destroy callback to avctp_register_browsing_pdu_handler
From: Luiz Augusto von Dentz @ 2013-01-27 23:52 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1359330736-9190-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds a destroy callback which is called when the PDU handler is
destroyed.
---
 profiles/audio/avctp.c | 44 +++++++++++++++++++++++++++++++++++++-------
 profiles/audio/avctp.h |  3 ++-
 profiles/audio/avrcp.c | 21 ++++++++++++---------
 3 files changed, 51 insertions(+), 17 deletions(-)

diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index 6cac718..41e2f46 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -168,6 +168,7 @@ struct avctp_channel {
 	GQueue *queue;
 	GSList *processed;
 	guint process_id;
+	GDestroyNotify destroy;
 };
 
 struct key_pressed {
@@ -206,6 +207,7 @@ struct avctp_browsing_pdu_handler {
 	avctp_browsing_pdu_cb cb;
 	void *user_data;
 	unsigned int id;
+	GDestroyNotify destroy;
 };
 
 static struct {
@@ -406,6 +408,9 @@ static void avctp_channel_destroy(struct avctp_channel *chan)
 	if (chan->process_id > 0)
 		g_source_remove(chan->process_id);
 
+	if (chan->destroy)
+		chan->destroy(chan);
+
 	g_free(chan->buffer);
 	g_queue_foreach(chan->queue, pending_destroy, NULL);
 	g_queue_free(chan->queue);
@@ -985,7 +990,8 @@ static void init_uinput(struct avctp *session)
 }
 
 static struct avctp_channel *avctp_channel_create(struct avctp *session,
-							GIOChannel *io)
+							GIOChannel *io,
+							GDestroyNotify destroy)
 {
 	struct avctp_channel *chan;
 
@@ -993,10 +999,30 @@ static struct avctp_channel *avctp_channel_create(struct avctp *session,
 	chan->session = session;
 	chan->io = g_io_channel_ref(io);
 	chan->queue = g_queue_new();
+	chan->destroy = destroy;
 
 	return chan;
 }
 
+static void handler_free(void *data)
+{
+	struct avctp_browsing_pdu_handler *handler = data;
+
+	if (handler->destroy)
+		handler->destroy(handler->user_data);
+
+	g_free(data);
+}
+
+static void avctp_destroy_browsing(void *data)
+{
+	struct avctp_channel *chan = data;
+
+	g_slist_free_full(chan->handlers, handler_free);
+
+	chan->handlers = NULL;
+}
+
 static void avctp_connect_browsing_cb(GIOChannel *chan, GError *err,
 							gpointer data)
 {
@@ -1026,7 +1052,8 @@ static void avctp_connect_browsing_cb(GIOChannel *chan, GError *err,
 	DBG("AVCTP Browsing: connected to %s", address);
 
 	if (session->browsing == NULL)
-		session->browsing = avctp_channel_create(session, chan);
+		session->browsing = avctp_channel_create(session, chan,
+						avctp_destroy_browsing);
 
 	session->browsing->imtu = imtu;
 	session->browsing->omtu = omtu;
@@ -1075,7 +1102,7 @@ static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
 	DBG("AVCTP: connected to %s", address);
 
 	if (session->control == NULL)
-		session->control = avctp_channel_create(session, chan);
+		session->control = avctp_channel_create(session, chan, NULL);
 
 	session->control->imtu = imtu;
 	session->control->omtu = omtu;
@@ -1189,7 +1216,7 @@ static void avctp_control_confirm(struct avctp *session, GIOChannel *chan,
 	}
 
 	avctp_set_state(session, AVCTP_STATE_CONNECTING);
-	session->control = avctp_channel_create(session, chan);
+	session->control = avctp_channel_create(session, chan, NULL);
 
 	src = adapter_get_address(device_get_adapter(dev->btd_dev));
 	dst = device_get_address(dev->btd_dev);
@@ -1666,7 +1693,8 @@ unsigned int avctp_register_pdu_handler(struct avctp *session, uint8_t opcode,
 
 unsigned int avctp_register_browsing_pdu_handler(struct avctp *session,
 						avctp_browsing_pdu_cb cb,
-						void *user_data)
+						void *user_data,
+						GDestroyNotify destroy)
 {
 	struct avctp_channel *browsing = session->browsing;
 	struct avctp_browsing_pdu_handler *handler;
@@ -1682,6 +1710,7 @@ unsigned int avctp_register_browsing_pdu_handler(struct avctp *session,
 	handler->cb = cb;
 	handler->user_data = user_data;
 	handler->id = ++id;
+	handler->destroy = destroy;
 
 	browsing->handlers = g_slist_append(browsing->handlers, handler);
 
@@ -1787,7 +1816,7 @@ struct avctp *avctp_connect(struct audio_device *device)
 		return NULL;
 	}
 
-	session->control = avctp_channel_create(session, io);
+	session->control = avctp_channel_create(session, io, NULL);
 	g_io_channel_unref(io);
 
 	return session;
@@ -1821,7 +1850,8 @@ int avctp_connect_browsing(struct avctp *session)
 		return -EIO;
 	}
 
-	session->browsing = avctp_channel_create(session, io);
+	session->browsing = avctp_channel_create(session, io,
+						avctp_destroy_browsing);
 	g_io_channel_unref(io);
 
 	return 0;
diff --git a/profiles/audio/avctp.h b/profiles/audio/avctp.h
index 6cbda92..411b093 100644
--- a/profiles/audio/avctp.h
+++ b/profiles/audio/avctp.h
@@ -110,7 +110,8 @@ gboolean avctp_unregister_pdu_handler(unsigned int id);
 
 unsigned int avctp_register_browsing_pdu_handler(struct avctp *session,
 						avctp_browsing_pdu_cb cb,
-						void *user_data);
+						void *user_data,
+						GDestroyNotify destroy);
 gboolean avctp_unregister_browsing_pdu_handler(unsigned int id);
 
 int avctp_send_passthrough(struct avctp *session, uint8_t op);
diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 6b61664..f83c3db 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -2165,12 +2165,20 @@ static struct avrcp *find_session(GSList *list, struct audio_device *dev)
 	return NULL;
 }
 
+static void destroy_browsing(void *data)
+{
+	struct avrcp *session = data;
+
+	session->browsing_id = 0;
+}
+
 static void session_tg_init_browsing(struct avrcp *session)
 {
 	session->browsing_id = avctp_register_browsing_pdu_handler(
 							session->conn,
 							handle_browsing_pdu,
-							session);
+							session,
+							destroy_browsing);
 }
 
 static void session_tg_init_control(struct avrcp *session)
@@ -2210,7 +2218,8 @@ static void session_ct_init_browsing(struct avrcp *session)
 	session->browsing_id = avctp_register_browsing_pdu_handler(
 							session->conn,
 							handle_browsing_pdu,
-							session);
+							session,
+							destroy_browsing);
 }
 
 static void session_ct_init_control(struct avrcp *session)
@@ -2378,15 +2387,9 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
 
 		break;
 	case AVCTP_STATE_CONNECTED:
-		if (session == NULL)
+		if (session == NULL || session->control_id > 0)
 			break;
 
-		if (session->browsing_id > 0)
-			session->browsing_id = 0;
-
-		if (session->control_id > 0)
-			return;
-
 		session->init_control(session);
 
 		if (session->version >= 0x0104 &&
-- 
1.8.1


^ permalink raw reply related

* [PATCH BlueZ 2/3 v3] AVCTP: Fix not destroying browsing channel if disconnected
From: Luiz Augusto von Dentz @ 2013-01-27 23:52 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1359330736-9190-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

If the browsing channel is disconnected it should be destroyed
immediatelly and set to NULL otherwise it will point to invalid channel.
---
 profiles/audio/avctp.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index 6ad2b66..6cac718 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -485,6 +485,10 @@ static void avctp_set_state(struct avctp *session, avctp_state_t new_state)
 		break;
 	case AVCTP_STATE_CONNECTED:
 		DBG("AVCTP Connected");
+		if (session->browsing) {
+			avctp_channel_destroy(session->browsing);
+			session->browsing = NULL;
+		}
 		break;
 	case AVCTP_STATE_BROWSING_CONNECTING:
 		DBG("AVCTP Browsing Connecting");
-- 
1.8.1


^ permalink raw reply related

* [PATCH BlueZ 1/3 v3] AVRCP: Fix not handling commands while browsing is connecting
From: Luiz Augusto von Dentz @ 2013-01-27 23:52 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

With introdution of browsing channel the .init callback is called when
browsing channel connection completes, but in the meantime the remote
device can send commands over control channel.

To fix this the init callaback of control and browsing channel are now
separated into .init_control and .init_browsing so the handler can be
register as soon the respective channel connection completes.
---
v2: Separate control and browsing inits and use handler id to track if channel
has been initialized or not.
v3: Adds patches 2 and 3

 profiles/audio/avrcp.c | 73 +++++++++++++++++++++++++++++---------------------
 1 file changed, 42 insertions(+), 31 deletions(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 4965b0c..6b61664 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -190,9 +190,9 @@ struct avrcp {
 	gboolean target;
 	uint16_t version;
 	int features;
-	bool initialized;
 
-	void (*init) (struct avrcp *session);
+	void (*init_control) (struct avrcp *session);
+	void (*init_browsing) (struct avrcp *session);
 	void (*destroy) (struct avrcp *sesion);
 
 	const struct control_pdu_handler *control_handlers;
@@ -2165,14 +2165,23 @@ static struct avrcp *find_session(GSList *list, struct audio_device *dev)
 	return NULL;
 }
 
-static void session_tg_init(struct avrcp *session)
+static void session_tg_init_browsing(struct avrcp *session)
+{
+	session->browsing_id = avctp_register_browsing_pdu_handler(
+							session->conn,
+							handle_browsing_pdu,
+							session);
+}
+
+static void session_tg_init_control(struct avrcp *session)
 {
 	struct avrcp_server *server = session->server;
 	struct avrcp_player *player;
 
-	DBG("%p version 0x%04x", session, session->version);
+	if (session->version < 0x0103)
+		return;
 
-	session->initialized = true;
+	DBG("%p version 0x%04x", session, session->version);
 
 	player = g_slist_nth_data(server->players, 0);
 	if (player != NULL) {
@@ -2180,6 +2189,10 @@ static void session_tg_init(struct avrcp *session)
 		player->sessions = g_slist_prepend(player->sessions, session);
 	}
 
+	session->control_id = avctp_register_pdu_handler(session->conn,
+							AVC_OP_VENDORDEP,
+							handle_vendordep_pdu,
+							session);
 	session->control_handlers = tg_control_handlers;
 	session->supported_events = (1 << AVRCP_EVENT_STATUS_CHANGED) |
 				(1 << AVRCP_EVENT_TRACK_CHANGED) |
@@ -2190,36 +2203,31 @@ static void session_tg_init(struct avrcp *session)
 	if (session->version >= 0x0104)
 		avrcp_register_notification(session,
 						AVRCP_EVENT_VOLUME_CHANGED);
+}
 
-	session->control_id = avctp_register_pdu_handler(session->conn,
-							AVC_OP_VENDORDEP,
-							handle_vendordep_pdu,
-							session);
+static void session_ct_init_browsing(struct avrcp *session)
+{
 	session->browsing_id = avctp_register_browsing_pdu_handler(
 							session->conn,
 							handle_browsing_pdu,
 							session);
 }
 
-static void session_ct_init(struct avrcp *session)
+static void session_ct_init_control(struct avrcp *session)
 {
 	struct avrcp_player *player;
 	struct media_player *mp;
 	const char *path;
 
-	session->control_handlers = ct_control_handlers;
-
-	if (session->version >= 0x0104)
-		session->supported_events = (1 << AVRCP_EVENT_VOLUME_CHANGED);
-
 	DBG("%p version 0x%04x", session, session->version);
 
-	session->initialized = true;
-
 	session->control_id = avctp_register_pdu_handler(session->conn,
 							AVC_OP_VENDORDEP,
 							handle_vendordep_pdu,
 							session);
+	session->control_handlers = ct_control_handlers;
+	if (session->version >= 0x0104)
+		session->supported_events = (1 << AVRCP_EVENT_VOLUME_CHANGED);
 
 	player = g_new0(struct avrcp_player, 1);
 	player->sessions = g_slist_prepend(player->sessions, session);
@@ -2316,11 +2324,13 @@ static struct avrcp *session_create(struct avrcp_server *server,
 		session->target = FALSE;
 
 	if (session->target) {
-		session->init = session_tg_init;
+		session->init_control = session_tg_init_control;
+		session->init_browsing = session_tg_init_browsing;
 		session->destroy = session_tg_destroy;
 		rec = btd_device_get_record(dev->btd_dev, AVRCP_REMOTE_UUID);
 	} else {
-		session->init = session_ct_init;
+		session->init_control = session_ct_init_control;
+		session->init_browsing = session_ct_init_browsing;
 		session->destroy = session_ct_destroy;
 		rec = btd_device_get_record(dev->btd_dev, AVRCP_TARGET_UUID);
 	}
@@ -2368,26 +2378,27 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
 
 		break;
 	case AVCTP_STATE_CONNECTED:
-		if (session == NULL || session->initialized)
+		if (session == NULL)
 			break;
 
-		/* Initialize session if browsing cannot be used */
-		if (session->version <= 0x0103 ||
-				old_state == AVCTP_STATE_BROWSING_CONNECTING ||
-				!(session->features & AVRCP_FEATURE_BROWSING)) {
-			session->init(session);
-			break;
-		}
+		if (session->browsing_id > 0)
+			session->browsing_id = 0;
+
+		if (session->control_id > 0)
+			return;
+
+		session->init_control(session);
 
-		if (avctp_connect_browsing(session->conn) != 0)
-			session->init(session);
+		if (session->version >= 0x0104 &&
+				session->features & AVRCP_FEATURE_BROWSING)
+			avctp_connect_browsing(session->conn);
 
 		break;
 	case AVCTP_STATE_BROWSING_CONNECTED:
-		if (session == NULL || session->initialized)
+		if (session == NULL || session->browsing_id > 0)
 			break;
 
-		session->init(session);
+		session->init_browsing(session);
 		break;
 	default:
 		return;
-- 
1.8.1


^ permalink raw reply related

* [PATCH BlueZ] AVRCP: Fix not handling commands while browsing is connecting
From: Luiz Augusto von Dentz @ 2013-01-27 22:37 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

With introdution of browsing channel the .init callback is called when
browsing channel connection completes, but in the meantime the remote
device can send commands over control channel.

To fix this the init callaback of control and browsing channel are now
separated into .init_control and .init_browsing so the handler can be
register as soon the respective channel connection completes.
---
v2: Separate control and browsing inits and use handler id to track if channel
has been initialized or not.

 profiles/audio/avrcp.c | 73 +++++++++++++++++++++++++++++---------------------
 1 file changed, 42 insertions(+), 31 deletions(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 4965b0c..6b61664 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -190,9 +190,9 @@ struct avrcp {
 	gboolean target;
 	uint16_t version;
 	int features;
-	bool initialized;
 
-	void (*init) (struct avrcp *session);
+	void (*init_control) (struct avrcp *session);
+	void (*init_browsing) (struct avrcp *session);
 	void (*destroy) (struct avrcp *sesion);
 
 	const struct control_pdu_handler *control_handlers;
@@ -2165,14 +2165,23 @@ static struct avrcp *find_session(GSList *list, struct audio_device *dev)
 	return NULL;
 }
 
-static void session_tg_init(struct avrcp *session)
+static void session_tg_init_browsing(struct avrcp *session)
+{
+	session->browsing_id = avctp_register_browsing_pdu_handler(
+							session->conn,
+							handle_browsing_pdu,
+							session);
+}
+
+static void session_tg_init_control(struct avrcp *session)
 {
 	struct avrcp_server *server = session->server;
 	struct avrcp_player *player;
 
-	DBG("%p version 0x%04x", session, session->version);
+	if (session->version < 0x0103)
+		return;
 
-	session->initialized = true;
+	DBG("%p version 0x%04x", session, session->version);
 
 	player = g_slist_nth_data(server->players, 0);
 	if (player != NULL) {
@@ -2180,6 +2189,10 @@ static void session_tg_init(struct avrcp *session)
 		player->sessions = g_slist_prepend(player->sessions, session);
 	}
 
+	session->control_id = avctp_register_pdu_handler(session->conn,
+							AVC_OP_VENDORDEP,
+							handle_vendordep_pdu,
+							session);
 	session->control_handlers = tg_control_handlers;
 	session->supported_events = (1 << AVRCP_EVENT_STATUS_CHANGED) |
 				(1 << AVRCP_EVENT_TRACK_CHANGED) |
@@ -2190,36 +2203,31 @@ static void session_tg_init(struct avrcp *session)
 	if (session->version >= 0x0104)
 		avrcp_register_notification(session,
 						AVRCP_EVENT_VOLUME_CHANGED);
+}
 
-	session->control_id = avctp_register_pdu_handler(session->conn,
-							AVC_OP_VENDORDEP,
-							handle_vendordep_pdu,
-							session);
+static void session_ct_init_browsing(struct avrcp *session)
+{
 	session->browsing_id = avctp_register_browsing_pdu_handler(
 							session->conn,
 							handle_browsing_pdu,
 							session);
 }
 
-static void session_ct_init(struct avrcp *session)
+static void session_ct_init_control(struct avrcp *session)
 {
 	struct avrcp_player *player;
 	struct media_player *mp;
 	const char *path;
 
-	session->control_handlers = ct_control_handlers;
-
-	if (session->version >= 0x0104)
-		session->supported_events = (1 << AVRCP_EVENT_VOLUME_CHANGED);
-
 	DBG("%p version 0x%04x", session, session->version);
 
-	session->initialized = true;
-
 	session->control_id = avctp_register_pdu_handler(session->conn,
 							AVC_OP_VENDORDEP,
 							handle_vendordep_pdu,
 							session);
+	session->control_handlers = ct_control_handlers;
+	if (session->version >= 0x0104)
+		session->supported_events = (1 << AVRCP_EVENT_VOLUME_CHANGED);
 
 	player = g_new0(struct avrcp_player, 1);
 	player->sessions = g_slist_prepend(player->sessions, session);
@@ -2316,11 +2324,13 @@ static struct avrcp *session_create(struct avrcp_server *server,
 		session->target = FALSE;
 
 	if (session->target) {
-		session->init = session_tg_init;
+		session->init_control = session_tg_init_control;
+		session->init_browsing = session_tg_init_browsing;
 		session->destroy = session_tg_destroy;
 		rec = btd_device_get_record(dev->btd_dev, AVRCP_REMOTE_UUID);
 	} else {
-		session->init = session_ct_init;
+		session->init_control = session_ct_init_control;
+		session->init_browsing = session_ct_init_browsing;
 		session->destroy = session_ct_destroy;
 		rec = btd_device_get_record(dev->btd_dev, AVRCP_TARGET_UUID);
 	}
@@ -2368,26 +2378,27 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
 
 		break;
 	case AVCTP_STATE_CONNECTED:
-		if (session == NULL || session->initialized)
+		if (session == NULL)
 			break;
 
-		/* Initialize session if browsing cannot be used */
-		if (session->version <= 0x0103 ||
-				old_state == AVCTP_STATE_BROWSING_CONNECTING ||
-				!(session->features & AVRCP_FEATURE_BROWSING)) {
-			session->init(session);
-			break;
-		}
+		if (session->browsing_id > 0)
+			session->browsing_id = 0;
+
+		if (session->control_id > 0)
+			return;
+
+		session->init_control(session);
 
-		if (avctp_connect_browsing(session->conn) != 0)
-			session->init(session);
+		if (session->version >= 0x0104 &&
+				session->features & AVRCP_FEATURE_BROWSING)
+			avctp_connect_browsing(session->conn);
 
 		break;
 	case AVCTP_STATE_BROWSING_CONNECTED:
-		if (session == NULL || session->initialized)
+		if (session == NULL || session->browsing_id > 0)
 			break;
 
-		session->init(session);
+		session->init_browsing(session);
 		break;
 	default:
 		return;
-- 
1.8.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox