Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH BlueZ 1/3] lib: Fix buffer overflow when processing SDP response
From: Anderson Lizardo @ 2013-02-04  1:20 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1359940845-14451-1-git-send-email-anderson.lizardo@openbossa.org>

rsp_count is either read or calculated from untrusted input, and
therefore needs to be checked before being used as offset. The "plen"
variable is appropriate because it is calculated as the sum of fixed and
variable length fields, excluding the continuation state field, which
has at least 1 byte for its own length field.
---
 lib/sdp.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/lib/sdp.c b/lib/sdp.c
index b87f392..f3a0c17 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -4189,6 +4189,17 @@ int sdp_process(sdp_session_t *session)
 		goto end;
 	}
 
+	/*
+	 * Out of bound check before using rsp_count as offset for continuation
+	 * state, which has at least a one byte size field.
+	 */
+	if ((n - (int) sizeof(sdp_pdu_hdr_t)) < plen + 1) {
+		t->err = EPROTO;
+		SDPERR("Protocol error: invalid PDU size");
+		status = SDP_INVALID_PDU_SIZE;
+		goto end;
+	}
+
 	pcstate = (sdp_cstate_t *) (pdata + rsp_count);
 
 	SDPDBG("Cstate length : %d\n", pcstate->length);
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH BlueZ 0/3] SDP library invalid memory access fixes
From: Anderson Lizardo @ 2013-02-04  1:20 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Anderson Lizardo

Hi,

This small set of patches fixes a couple of invalid memory reads/writes
detected by code inspection and confirmed by emulating invalid PDUs.

BTW, I have been silently working for some time on a tool now called "Blueish"
(variant of "bluish", meaning "somewhat blue"). It is fully written in Python
and allows to "easily" generate automated standalone test scripts (that also
only require Python + D-Bus/GLib bindings) for testing scenarios difficult on
real hardware. It uses VHCI for emulation.

For documentation and code, see: https://github.com/lizardo/blueish

The repository contains example data files for the latest patches I sent a
while ago (and these ones).

I tried to make it easy to use by adopting YAML for HCI packet construction.
Still, I'm aware that constructing HCI packets by hand is error prone, so I
plan (someday) to have a nice GUI and even some sort of visualization for the
packets (message sequence charts, maybe?).

That said, I'm still interested on helping with improving unit tests for BlueZ
(specially code not touched for a while). I just could not come up with a nice
way to integrate SDP client unit tests with the current server ones without too
much code duplication.

Best Regards,

Anderson Lizardo (3):
  lib: Fix buffer overflow when processing SDP response
  lib: Add range check for SDP_SVC_ATTR_RSP/SDP_SVC_SEARCH_ATTR_RSP
  lib: Check if SDP buffer has enough data on partial responses

 lib/sdp.c |   26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

-- 
1.7.9.5


^ permalink raw reply

* [PATCH BlueZ] AVRCP: Handler player features as a byte array
From: Luiz Augusto von Dentz @ 2013-02-03 20:55 UTC (permalink / raw)
  To: linux-bluetooth

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

This way we can direct match by byte which is how the spec represent
them.
---
 profiles/audio/avrcp.c  |  5 ++---
 profiles/audio/player.c | 16 ++++++++++++----
 profiles/audio/player.h |  2 +-
 3 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 00eeea1..20088e5 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -1983,7 +1983,7 @@ static void avrcp_parse_media_player_item(struct avrcp *session,
 	uint16_t id;
 	uint32_t subtype;
 	const char *curval, *strval;
-	uint64_t features[2];
+	uint8_t features[16];
 	char name[255];
 
 	if (len < 28)
@@ -2008,8 +2008,7 @@ static void avrcp_parse_media_player_item(struct avrcp *session,
 		avrcp_get_play_status(session);
 	}
 
-	features[0] = bt_get_be64(&operands[8]);
-	features[1] = bt_get_be64(&operands[16]);
+	memcpy(features, &operands[8], sizeof(features));
 
 	media_player_set_features(mp, features);
 
diff --git a/profiles/audio/player.c b/profiles/audio/player.c
index f875ee7..f1024ba 100644
--- a/profiles/audio/player.c
+++ b/profiles/audio/player.c
@@ -67,7 +67,7 @@ struct media_player {
 	char			*name;		/* Player name */
 	char			*type;		/* Player type */
 	char			*subtype;	/* Player subtype */
-	uint64_t		features[2];	/* Player features */
+	uint8_t			*features;	/* Player features */
 	struct media_folder	*folder;	/* Player currenct folder */
 	char			*path;		/* Player object path */
 	GHashTable		*settings;	/* Player settings */
@@ -644,6 +644,7 @@ void media_player_destroy(struct media_player *mp)
 	g_free(mp->subtype);
 	g_free(mp->type);
 	g_free(mp->name);
+	g_free(mp->features);
 	g_free(mp);
 }
 
@@ -870,11 +871,18 @@ void media_player_set_folder(struct media_player *mp, const char *path,
 	}
 }
 
-void media_player_set_features(struct media_player *mp, uint64_t *features)
+void media_player_set_features(struct media_player *mp, uint8_t *features)
 {
-	DBG("0x%016" PRIx64 "%016" PRIx64, features[0], features[1]);
+	DBG("0x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x"
+		"%02x", features[0], features[1], features[2], features[3],
+		features[4], features[5], features[6], features[7],
+		features[8], features[9], features[10], features[11],
+		features[12], features[13], features[14], features[15]);
 
-	memcpy(features, mp->features, sizeof(mp->features));
+	if (mp->features != NULL)
+		return;
+
+	mp->features = g_memdup(features, 16);
 }
 
 void media_player_set_callbacks(struct media_player *mp,
diff --git a/profiles/audio/player.h b/profiles/audio/player.h
index 1ac9800..efdae31 100644
--- a/profiles/audio/player.h
+++ b/profiles/audio/player.h
@@ -49,7 +49,7 @@ void media_player_set_metadata(struct media_player *mp, const char *key,
 						void *data, size_t len);
 void media_player_set_type(struct media_player *mp, const char *type);
 void media_player_set_subtype(struct media_player *mp, const char *subtype);
-void media_player_set_features(struct media_player *mp, uint64_t *features);
+void media_player_set_features(struct media_player *mp, uint8_t *features);
 void media_player_set_name(struct media_player *mp, const char *name);
 void media_player_set_folder(struct media_player *mp, const char *path,
 								uint32_t items);
-- 
1.8.1


^ permalink raw reply related

* Re: [PATCH BlueZ 2/2] device: use device_set_trusted() in set_trust()
From: Anderson Lizardo @ 2013-02-03 18:27 UTC (permalink / raw)
  To: Antonio Ospite; +Cc: linux-bluetooth
In-Reply-To: <20130203185107.0dcdfc4271d63c9e5403d697@studenti.unina.it>

Hi Antonio,

On Sun, Feb 3, 2013 at 1:51 PM, Antonio Ospite <ospite@studenti.unina.it> wrote:
> The logic is equivalent to the previous one, as device_set_trusted()
> returns without doing anything when (device->trusted == trusted), and
> the previous implementation was already calling
> g_dbus_pending_property_success() just before returning in any exit
> path. Or was it meant to be g_dbus_pending_property_error() in the
> first exit path? I don't know about that.

My mistake. I incorrectly read the diff. And I think the _success()
function is correct as it is.

>
> The previous set_trust() code didn't consider the !device case so we
> should be OK if this function assumes the device is always defined.
>
> If the code is OK I can improve the commit message, yes.

Having more text on the commit message is always better than less text
:). But in this case, the commit is quite simple, so it is up to
you/Johan/Marcel.

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

^ permalink raw reply

* Re: [PATCH BlueZ 2/2] device: use device_set_trusted() in set_trust()
From: Antonio Ospite @ 2013-02-03 17:51 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <CAJdJm_OFBu_gtgBz8+RFSS8cDVbtVH9DfuRtnimOd8auh8zDgw@mail.gmail.com>

On Sun, 3 Feb 2013 13:32:12 -0400
Anderson Lizardo <anderson.lizardo@openbossa.org> wrote:

> Hi Antonio,
> 
> On Sun, Feb 3, 2013 at 12:14 PM, Antonio Ospite
> <ospite@studenti.unina.it> wrote:
> > @@ -724,17 +724,7 @@ static void set_trust(GDBusPendingPropertySet id, gboolean value, void *data)
> >  {
> >         struct btd_device *device = data;
> >
> > -       if (device->trusted == value) {
> > -               g_dbus_pending_property_success(id);
> > -               return;
> > -       }
> > -
> > -       device->trusted = value;
> > -
> > -       store_device_info(device);
> > -
> > -       g_dbus_emit_property_changed(dbus_conn, device->path,
> > -                                               DEVICE_INTERFACE, "Trusted");
> > +       device_set_trusted(device, value);
> >
> >         g_dbus_pending_property_success(id);
> 
> g_dbus_pending_property_success() is now called always (whether the
> property changed or not). Not sure if this is an issue, but it is a
> change that needs at least a clarification on the commit message.
> 

The logic is equivalent to the previous one, as device_set_trusted()
returns without doing anything when (device->trusted == trusted), and
the previous implementation was already calling
g_dbus_pending_property_success() just before returning in any exit
path. Or was it meant to be g_dbus_pending_property_error() in the
first exit path? I don't know about that.

The previous set_trust() code didn't consider the !device case so we
should be OK if this function assumes the device is always defined.

If the code is OK I can improve the commit message, yes.

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

^ permalink raw reply

* Re: [PATCH BlueZ 2/2] device: use device_set_trusted() in set_trust()
From: Anderson Lizardo @ 2013-02-03 17:32 UTC (permalink / raw)
  To: Antonio Ospite; +Cc: linux-bluetooth
In-Reply-To: <1359908071-15024-3-git-send-email-ospite@studenti.unina.it>

Hi Antonio,

On Sun, Feb 3, 2013 at 12:14 PM, Antonio Ospite
<ospite@studenti.unina.it> wrote:
> @@ -724,17 +724,7 @@ static void set_trust(GDBusPendingPropertySet id, gboolean value, void *data)
>  {
>         struct btd_device *device = data;
>
> -       if (device->trusted == value) {
> -               g_dbus_pending_property_success(id);
> -               return;
> -       }
> -
> -       device->trusted = value;
> -
> -       store_device_info(device);
> -
> -       g_dbus_emit_property_changed(dbus_conn, device->path,
> -                                               DEVICE_INTERFACE, "Trusted");
> +       device_set_trusted(device, value);
>
>         g_dbus_pending_property_success(id);

g_dbus_pending_property_success() is now called always (whether the
property changed or not). Not sure if this is an issue, but it is a
change that needs at least a clarification on the commit message.

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

^ permalink raw reply

* [PATCH BlueZ 2/2] device: use device_set_trusted() in set_trust()
From: Antonio Ospite @ 2013-02-03 16:14 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Antonio Ospite
In-Reply-To: <1359908071-15024-1-git-send-email-ospite@studenti.unina.it>

---
 src/device.c |   12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/src/device.c b/src/device.c
index 87e5eff..9e4c41f 100644
--- a/src/device.c
+++ b/src/device.c
@@ -724,17 +724,7 @@ static void set_trust(GDBusPendingPropertySet id, gboolean value, void *data)
 {
 	struct btd_device *device = data;
 
-	if (device->trusted == value) {
-		g_dbus_pending_property_success(id);
-		return;
-	}
-
-	device->trusted = value;
-
-	store_device_info(device);
-
-	g_dbus_emit_property_changed(dbus_conn, device->path,
-						DEVICE_INTERFACE, "Trusted");
+	device_set_trusted(device, value);
 
 	g_dbus_pending_property_success(id);
 }
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH BlueZ 1/2] device: add a device_set_trusted() function
From: Antonio Ospite @ 2013-02-03 16:14 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Antonio Ospite
In-Reply-To: <1359908071-15024-1-git-send-email-ospite@studenti.unina.it>

---
 src/device.c |   18 ++++++++++++++++++
 src/device.h |    1 +
 2 files changed, 19 insertions(+)

diff --git a/src/device.c b/src/device.c
index 49f8957..87e5eff 100644
--- a/src/device.c
+++ b/src/device.c
@@ -3462,6 +3462,24 @@ void device_set_temporary(struct btd_device *device, gboolean temporary)
 	device->temporary = temporary;
 }
 
+void device_set_trusted(struct btd_device *device, gboolean trusted)
+{
+	if (!device)
+		return;
+
+	if (device->trusted == trusted)
+		return;
+
+	DBG("trusted %d", trusted);
+
+	device->trusted = trusted;
+
+	store_device_info(device);
+
+	g_dbus_emit_property_changed(dbus_conn, device->path,
+					DEVICE_INTERFACE, "Trusted");
+}
+
 void device_set_bonded(struct btd_device *device, gboolean bonded)
 {
 	if (!device)
diff --git a/src/device.h b/src/device.h
index dc11e2c..d072015 100644
--- a/src/device.h
+++ b/src/device.h
@@ -68,6 +68,7 @@ gboolean device_is_bonded(struct btd_device *device);
 gboolean device_is_trusted(struct btd_device *device);
 void device_set_paired(struct btd_device *device, gboolean paired);
 void device_set_temporary(struct btd_device *device, gboolean temporary);
+void device_set_trusted(struct btd_device *device, gboolean trusted);
 void device_set_bonded(struct btd_device *device, gboolean bonded);
 void device_set_legacy(struct btd_device *device, bool legacy);
 void device_set_rssi(struct btd_device *device, int8_t rssi);
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH BlueZ 0/2] Add device_set_trusted()
From: Antonio Ospite @ 2013-02-03 16:14 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Antonio Ospite

Hi,

In patch 1 a device_set_trusted() function is proposed, which I plan to
use for the playstation-peripheral plugin; I am in the process of
rebasing the plugin on top of BlueZ 5.2.

In patch 2 the newly introduced function is used in order to avoid some
duplication.

device_set_trusted() looks a lot like device_set_temporary() and
device_set_legacy(), I hope it makes sense to you too.

Thanks,
   Antonio

Antonio Ospite (2):
  device: add a device_set_trusted() function
  device: use device_set_trusted() in set_trust()

 src/device.c |   30 +++++++++++++++++++-----------
 src/device.h |    1 +
 2 files changed, 20 insertions(+), 11 deletions(-)

-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

^ permalink raw reply

* [PATCH BlueZ] configure.ac: call AC_SUBST unconditionally
From: Antonio Ospite @ 2013-02-03 15:46 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Antonio Ospite

Call AC_SUBST unconditionally, otherwise options like
--with-dbusconfdir=DIR or --with-udevdir=DIR have no effect.

Before this change, configuring with:

  $ mkdir build
  $ ./configure --disable-systemd \
                --prefix=$(pwd)/build \
                --with-dbusconfdir=$(pwd)/build/etc

resulted in the option value to be ignored at "make install" time, with
this error:

  /bin/mkdir: cannot create directory '/dbus-1/system.d': Permission denied

After the patch the option value is respected.
---

Hi,

the issue was highlighted by the use "--prefix=" and running "make install" as
a restricted user, maybe the are still other issues with this use case.
Anyone willing to take a deeper look?

For instance, is "--prefix=DIR" supposed to be prepended to manually specified
paths too?

Thanks,
   Antonio

 configure.ac |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/configure.ac b/configure.ac
index 070acea..fe2893a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -71,8 +71,8 @@ if (test -z "${path_dbusconfdir}"); then
 		AC_MSG_ERROR([D-Bus configuration directory is required])
 	fi
 	AC_MSG_RESULT([${path_dbusconfdir}])
-	AC_SUBST(DBUS_CONFDIR, [${path_dbusconfdir}])
 fi
+AC_SUBST(DBUS_CONFDIR, [${path_dbusconfdir}])
 
 AC_ARG_WITH([dbussystembusdir], AC_HELP_STRING([--with-dbussystembusdir=DIR],
 				[path to D-Bus system bus services directory]),
@@ -84,8 +84,8 @@ if (test -z "${path_dbussystembusdir}"); then
 		AC_MSG_ERROR([D-Bus system bus services directory is required])
 	fi
 	AC_MSG_RESULT([${path_dbussystembusdir}])
-	AC_SUBST(DBUS_SYSTEMBUSDIR, [${path_dbussystembusdir}])
 fi
+AC_SUBST(DBUS_SYSTEMBUSDIR, [${path_dbussystembusdir}])
 
 AC_ARG_WITH([dbussessionbusdir], AC_HELP_STRING([--with-dbussessionbusdir=DIR],
 				[path to D-Bus session bus services directory]),
@@ -97,8 +97,8 @@ if (test -z "${path_dbussessionbusdir}"); then
 		AC_MSG_ERROR([D-Bus session bus services directory is required])
 	fi
 	AC_MSG_RESULT([${path_dbussessionbusdir}])
-	AC_SUBST(DBUS_SESSIONBUSDIR, [${path_dbussessionbusdir}])
 fi
+AC_SUBST(DBUS_SESSIONBUSDIR, [${path_dbussessionbusdir}])
 
 AC_ARG_ENABLE(library, AC_HELP_STRING([--enable-library],
 		[install Bluetooth library]), [enable_library=${enableval}])
@@ -121,8 +121,6 @@ AC_ARG_ENABLE(usb, AC_HELP_STRING([--disable-usb],
 if (test "${enable_tools}" != "no" && test "${enable_usb}" != "no"  ); then
 	PKG_CHECK_MODULES(USB, libusb, dummy=yes,
 			AC_MSG_ERROR(USB library support is required))
-	AC_SUBST(USB_CFLAGS)
-	AC_SUBST(USB_LIBS)
 	AC_CHECK_LIB(usb, usb_get_busses, dummy=yes,
 		AC_DEFINE(NEED_USB_GET_BUSSES, 1,
 			[Define to 1 if you need the usb_get_busses() function.]
@@ -133,6 +131,8 @@ if (test "${enable_tools}" != "no" && test "${enable_usb}" != "no"  ); then
 on.]))
 	AC_DEFINE(HAVE_LIBUSB, 1, [Define to 1 if you have USB library.])
 fi
+AC_SUBST(USB_CFLAGS)
+AC_SUBST(USB_LIBS)
 AM_CONDITIONAL(USB, test "${enable_usb}" != "no")
 
 AC_ARG_ENABLE(udev, AC_HELP_STRING([--disable-udev],
@@ -157,8 +157,8 @@ if (test "${enable_udev}" != "no" && test -z "${path_udevdir}"); then
 		AC_MSG_ERROR([udev directory is required])
 	fi
 	AC_MSG_RESULT([${path_udevdir}])
-	AC_SUBST(UDEV_DIR, [${path_udevdir}])
 fi
+AC_SUBST(UDEV_DIR, [${path_udevdir}])
 
 AM_CONDITIONAL(HID2HCI, test "${enable_tools}" != "no" &&
 		test "${enable_udev}" != "no" && test "${enable_usb}" != "no")
@@ -202,8 +202,8 @@ if (test "${enable_systemd}" != "no" && test -z "${path_systemunitdir}"); then
 		AC_MSG_ERROR([systemd system unit directory is required])
 	fi
 	AC_MSG_RESULT([${path_systemunitdir}])
-	AC_SUBST(SYSTEMD_SYSTEMUNITDIR, [${path_systemunitdir}])
 fi
+AC_SUBST(SYSTEMD_SYSTEMUNITDIR, [${path_systemunitdir}])
 
 AC_ARG_WITH([systemduserunitdir],
 			AC_HELP_STRING([--with-systemduserunitdir=DIR],
@@ -216,8 +216,8 @@ if (test "${enable_systemd}" != "no" && test -z "${path_userunitdir}"); then
 		AC_MSG_ERROR([systemd user unit directory is required])
 	fi
 	AC_MSG_RESULT([${path_userunitdir}])
-	AC_SUBST(SYSTEMD_USERUNITDIR, [${path_userunitdir}])
 fi
+AC_SUBST(SYSTEMD_USERUNITDIR, [${path_userunitdir}])
 
 AC_ARG_ENABLE(datafiles, AC_HELP_STRING([--disable-datafiles],
 			[do not install configuration and data files]),
-- 
1.7.10.4


^ permalink raw reply related

* Re: [PATCH] shared: Fix use after free in read_watch_destroy
From: Marcel Holtmann @ 2013-02-03 11:47 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1359830398-23165-1-git-send-email-szymon@janc.net.pl>

Hi Szymon,

> read_watch_destroy is called when received_data returns FALSE.
> free mgmt in read_watch_destroy instead of received_data to avoid
> use after free.
> 
> Invalid write of size 4
>    at 0x8051604: read_watch_destroy (mgmt.c:271)
>    by 0x48C7468E: g_source_callback_unref (gmain.c:1457)
>    by 0x48C77287: g_main_context_dispatch (gmain.c:2723)
>    by 0x48C774FF: g_main_context_iterate.isra.22 (gmain.c:3290)
>    by 0x48C77962: g_main_loop_run (gmain.c:3484)
>    by 0x805393E: tester_run (tester.c:784)
>    by 0x804D1C7: main (mgmt-tester.c:2558)
>  Address 0x4039b80 is 16 bytes inside a block of size 76 free'd
>    at 0x4007F0F: free (vg_replace_malloc.c:446)
>    by 0x48C7D44B: standard_free (gmem.c:98)
>    by 0x48C7D607: g_free (gmem.c:252)
>    by 0x8051BB6: received_data (mgmt.c:337)
>    by 0x48CBA72E: g_io_unix_dispatch (giounix.c:167)
>    by 0x48C7715A: g_main_context_dispatch (gmain.c:2715)
>    by 0x48C774FF: g_main_context_iterate.isra.22 (gmain.c:3290)
>    by 0x48C77962: g_main_loop_run (gmain.c:3484)
>    by 0x805393E: tester_run (tester.c:784)
>    by 0x804D1C7: main (mgmt-tester.c:2558)
> ---
>  src/shared/mgmt.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

excellent catch here. I totally overlooked this code path when fixing
unregister from event callback. Patch has been applied.

Regards

Marcel



^ permalink raw reply

* [PATCH] shared: Fix use after free in read_watch_destroy
From: Szymon Janc @ 2013-02-02 18:39 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

read_watch_destroy is called when received_data returns FALSE.
free mgmt in read_watch_destroy instead of received_data to avoid
use after free.

Invalid write of size 4
   at 0x8051604: read_watch_destroy (mgmt.c:271)
   by 0x48C7468E: g_source_callback_unref (gmain.c:1457)
   by 0x48C77287: g_main_context_dispatch (gmain.c:2723)
   by 0x48C774FF: g_main_context_iterate.isra.22 (gmain.c:3290)
   by 0x48C77962: g_main_loop_run (gmain.c:3484)
   by 0x805393E: tester_run (tester.c:784)
   by 0x804D1C7: main (mgmt-tester.c:2558)
 Address 0x4039b80 is 16 bytes inside a block of size 76 free'd
   at 0x4007F0F: free (vg_replace_malloc.c:446)
   by 0x48C7D44B: standard_free (gmem.c:98)
   by 0x48C7D607: g_free (gmem.c:252)
   by 0x8051BB6: received_data (mgmt.c:337)
   by 0x48CBA72E: g_io_unix_dispatch (giounix.c:167)
   by 0x48C7715A: g_main_context_dispatch (gmain.c:2715)
   by 0x48C774FF: g_main_context_iterate.isra.22 (gmain.c:3290)
   by 0x48C77962: g_main_loop_run (gmain.c:3484)
   by 0x805393E: tester_run (tester.c:784)
   by 0x804D1C7: main (mgmt-tester.c:2558)
---
 src/shared/mgmt.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/shared/mgmt.c b/src/shared/mgmt.c
index cf7fdcf..ca4b05f 100644
--- a/src/shared/mgmt.c
+++ b/src/shared/mgmt.c
@@ -268,6 +268,11 @@ static void read_watch_destroy(gpointer user_data)
 {
 	struct mgmt *mgmt = user_data;
 
+	if (mgmt->destroyed) {
+		g_free(mgmt);
+		return;
+	}
+
 	mgmt->read_watch = 0;
 }
 
@@ -333,10 +338,8 @@ static gboolean received_data(GIOChannel *channel, GIOCondition cond,
 		break;
 	}
 
-	if (mgmt->destroyed) {
-		g_free(mgmt);
+	if (mgmt->destroyed)
 		return FALSE;
-	}
 
 	return TRUE;
 }
-- 
1.8.1.2


^ permalink raw reply related

* Re: [RFC 1/7] Bluetooth: Add new connection states
From: Marcel Holtmann @ 2013-02-02  0:51 UTC (permalink / raw)
  To: Andre Guedes; +Cc: linux-bluetooth
In-Reply-To: <1359765878-31409-2-git-send-email-andre.guedes@openbossa.org>

Hi Andre,

> This patch adds two new connection states which will be used by
> hci_conn to establish LE connections.
> 
> BT_SCAN state means the controller is scanning for LE devices. Once
> the target device is found, the connection goes to BT_DEV_FOUND
> state, and then to BT_CONNECT state.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  include/net/bluetooth/bluetooth.h | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index 9531bee..add5721 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -126,7 +126,9 @@ enum {
>  	BT_CONNECT2,
>  	BT_CONFIG,
>  	BT_DISCONN,
> -	BT_CLOSED
> +	BT_CLOSED,
> +	BT_SCAN,
> +	BT_DEV_FOUND
>  };

I am actually against this. These states where originally socket states
and not general states that we just pile on top of.

You need to create separate state handling for LE connection handling. I
am really not happy that we always try to shoe-horn this onto something
else.

Regards

Marcel



^ permalink raw reply

* [RFC 7/7] Bluetooth: Track the number of hci_conn in BT_SCAN
From: Andre Guedes @ 2013-02-02  0:44 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1359765878-31409-1-git-send-email-andre.guedes@openbossa.org>

In order to avoid traversing the hci_conn list every time we get an
advertising report, we should keep a counter of connections in
BT_SCAN state. This way, we only traverse the list if the counter
is greater from zero.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h |  5 +++++
 net/bluetooth/hci_conn.c         |  6 +++++-
 net/bluetooth/hci_core.c         |  2 ++
 net/bluetooth/hci_event.c        | 13 +++++++++----
 4 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index b089968..b371434 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -290,6 +290,11 @@ struct hci_dev {
 	__u8			adv_data[HCI_MAX_AD_LENGTH];
 	__u8			adv_data_len;
 
+	/* This counter tracks the number of LE connections in scanning
+	 * state (BT_SCAN).
+	 */
+	atomic_t		le_conn_scan_cnt;
+
 	int (*open)(struct hci_dev *hdev);
 	int (*close)(struct hci_dev *hdev);
 	int (*flush)(struct hci_dev *hdev);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index ac45725..58fd681 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -262,6 +262,7 @@ static void hci_conn_timeout(struct work_struct *work)
 {
 	struct hci_conn *conn = container_of(work, struct hci_conn,
 					     disc_work.work);
+	struct hci_dev *hdev = conn->hdev;
 
 	BT_DBG("hcon %p state %s", conn, state_to_string(conn->state));
 
@@ -272,7 +273,8 @@ static void hci_conn_timeout(struct work_struct *work)
 	case BT_SCAN:
 		if (conn->type == LE_LINK) {
 			conn->state = BT_CLOSED;
-			hci_cancel_le_scan(conn->hdev);
+			atomic_dec(&hdev->le_conn_scan_cnt);
+			hci_cancel_le_scan(hdev);
 		}
 		break;
 	case BT_CONNECT:
@@ -524,6 +526,8 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
 		le->dst_type = bdaddr_to_le(dst_type);
 		le->state = BT_SCAN;
 
+		atomic_inc(&hdev->le_conn_scan_cnt);
+
 		hci_le_scan(hdev, LE_SCAN_PASSIVE, 0x60, 0x30, 0);
 	}
 
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 3aa0345..f5c2b01 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1742,6 +1742,8 @@ struct hci_dev *hci_alloc_dev(void)
 	hci_init_sysfs(hdev);
 	discovery_init(hdev);
 
+	atomic_set(&hdev->le_conn_scan_cnt, 0);
+
 	return hdev;
 }
 EXPORT_SYMBOL(hci_alloc_dev);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 63c5d10..1464e5e 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -4016,10 +4016,15 @@ static void hci_le_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb)
 	while (num_reports--) {
 		struct hci_ev_le_advertising_info *ev = ptr;
 
-		hcon = hci_conn_hash_lookup_ba(hdev, LE_LINK, &ev->bdaddr);
-		if (hcon && hcon->dst_type == ev->bdaddr_type) {
-			hcon->state = BT_DEV_FOUND;
-			hci_cancel_le_scan(hdev);
+		if (atomic_read(&hdev->le_conn_scan_cnt) != 0) {
+			hcon = hci_conn_hash_lookup_ba(hdev, LE_LINK,
+						       &ev->bdaddr);
+
+			if (hcon && hcon->dst_type == ev->bdaddr_type) {
+				hcon->state = BT_DEV_FOUND;
+				atomic_dec(&hdev->le_conn_scan_cnt);
+				hci_cancel_le_scan(hdev);
+			}
 		}
 
 		rssi = ev->data[ev->length];
-- 
1.8.1.1


^ permalink raw reply related

* [RFC 6/7] Bluetooth: Handle hci_conn timeout in BT_SCAN state
From: Andre Guedes @ 2013-02-02  0:44 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1359765878-31409-1-git-send-email-andre.guedes@openbossa.org>

If occurs a hci_conn timeout in BT_SCAN state we should stop the
ongoing LE scan.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/hci_conn.c  |  6 ++++++
 net/bluetooth/hci_event.c | 16 ++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index fa2caf2..ac45725 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -269,6 +269,12 @@ static void hci_conn_timeout(struct work_struct *work)
 		return;
 
 	switch (conn->state) {
+	case BT_SCAN:
+		if (conn->type == LE_LINK) {
+			conn->state = BT_CLOSED;
+			hci_cancel_le_scan(conn->hdev);
+		}
+		break;
 	case BT_CONNECT:
 	case BT_CONNECT2:
 		if (conn->out) {
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index c9d2b71..63c5d10 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1296,6 +1296,22 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 
 		clear_bit(HCI_LE_SCAN, &hdev->dev_flags);
 
+		hcon = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CLOSED);
+		if (hcon) {
+			mgmt_connect_failed(hdev, &hcon->dst, hcon->type,
+					    hcon->dst_type,
+					    HCI_ERROR_LOCAL_HOST_TERM);
+
+			hci_proto_connect_cfm(hcon, HCI_ERROR_LOCAL_HOST_TERM);
+			hci_conn_del(hcon);
+
+			hci_dev_lock(hdev);
+			hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
+			hci_dev_unlock(hdev);
+
+			return;
+		}
+
 		hcon = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_DEV_FOUND);
 		if (hcon) {
 			hci_dev_lock(hdev);
-- 
1.8.1.1


^ permalink raw reply related

* [RFC 5/7] Bluetooth: Change LE connection routine
From: Andre Guedes @ 2013-02-02  0:44 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1359765878-31409-1-git-send-email-andre.guedes@openbossa.org>

In order to better support LE connection requirements, such as
multiple connections and re-connections, we should support the
general connection establishment procedure described in Core
spec. Today, we support only the direct connection establishment
procedure which has some limitations and, therefore, requires
extra connection management at user-space in order to support
LE connection requirements.

According to the spec, the general procedure is described as
follows: The host starts scanning for LE devices, once the
device we want to connect to is in-range, the host stops scanning
and initiates a connection. The procedure is terminated when the
connection is established or when the host terminates the procedure.

This patch changes the LE connection routine so we carry out the
general procedure instead of the direct procedure.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/hci_conn.c  |  4 +++-
 net/bluetooth/hci_event.c | 19 +++++++++++++++++--
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index bb9a88d..fa2caf2 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -516,7 +516,9 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
 			return ERR_PTR(-ENOMEM);
 
 		le->dst_type = bdaddr_to_le(dst_type);
-		hci_le_create_connection(le);
+		le->state = BT_SCAN;
+
+		hci_le_scan(hdev, LE_SCAN_PASSIVE, 0x60, 0x30, 0);
 	}
 
 	le->pending_sec_level = sec_level;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 477726a..c9d2b71 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1260,6 +1260,7 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 {
 	struct hci_cp_le_set_scan_enable *cp;
 	__u8 status = *((__u8 *) skb->data);
+	struct hci_conn *hcon;
 
 	BT_DBG("%s status 0x%2.2x", hdev->name, status);
 
@@ -1295,8 +1296,15 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 
 		clear_bit(HCI_LE_SCAN, &hdev->dev_flags);
 
-		if (hdev->discovery.type == DISCOV_TYPE_INTERLEAVED &&
-		    hdev->discovery.state == DISCOVERY_FINDING) {
+		hcon = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_DEV_FOUND);
+		if (hcon) {
+			hci_dev_lock(hdev);
+			hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
+			hci_dev_unlock(hdev);
+
+			hci_le_create_connection(hcon);
+		} else if (hdev->discovery.type == DISCOV_TYPE_INTERLEAVED &&
+			   hdev->discovery.state == DISCOVERY_FINDING) {
 			mgmt_interleaved_discovery(hdev);
 		} else {
 			hci_dev_lock(hdev);
@@ -3987,10 +3995,17 @@ static void hci_le_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb)
 	u8 num_reports = skb->data[0];
 	void *ptr = &skb->data[1];
 	s8 rssi;
+	struct hci_conn *hcon;
 
 	while (num_reports--) {
 		struct hci_ev_le_advertising_info *ev = ptr;
 
+		hcon = hci_conn_hash_lookup_ba(hdev, LE_LINK, &ev->bdaddr);
+		if (hcon && hcon->dst_type == ev->bdaddr_type) {
+			hcon->state = BT_DEV_FOUND;
+			hci_cancel_le_scan(hdev);
+		}
+
 		rssi = ev->data[ev->length];
 		mgmt_device_found(hdev, &ev->bdaddr, LE_LINK, ev->bdaddr_type,
 				  NULL, rssi, 0, 1, ev->data, ev->length);
-- 
1.8.1.1


^ permalink raw reply related

* [RFC 4/7] Bluetooth: Add LE scan type macros
From: Andre Guedes @ 2013-02-02  0:44 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1359765878-31409-1-git-send-email-andre.guedes@openbossa.org>

This patch adds macros for active and passive LE scan type values. It
also removes the LE_SCAN_TYPE macro since it is not used anymore.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h | 3 +++
 net/bluetooth/mgmt.c             | 5 ++---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 00923ef..b089968 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -117,6 +117,9 @@ struct oob_data {
 	u8 randomizer[16];
 };
 
+#define LE_SCAN_PASSIVE		0x00
+#define LE_SCAN_ACTIVE		0x01
+
 struct le_scan_params {
 	u8 type;
 	u16 interval;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 39395c7..43db12e 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -106,7 +106,6 @@ static const u16 mgmt_events[] = {
  * These LE scan and inquiry parameters were chosen according to LE General
  * Discovery Procedure specification.
  */
-#define LE_SCAN_TYPE			0x01
 #define LE_SCAN_WIN			0x12
 #define LE_SCAN_INT			0x12
 #define LE_SCAN_TIMEOUT_LE_ONLY		10240	/* TGAP(gen_disc_scan_min) */
@@ -2485,7 +2484,7 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
 			goto failed;
 		}
 
-		err = hci_le_scan(hdev, LE_SCAN_TYPE, LE_SCAN_INT,
+		err = hci_le_scan(hdev, LE_SCAN_ACTIVE, LE_SCAN_INT,
 				  LE_SCAN_WIN, LE_SCAN_TIMEOUT_LE_ONLY);
 		break;
 
@@ -2497,7 +2496,7 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
 			goto failed;
 		}
 
-		err = hci_le_scan(hdev, LE_SCAN_TYPE, LE_SCAN_INT, LE_SCAN_WIN,
+		err = hci_le_scan(hdev, LE_SCAN_ACTIVE, LE_SCAN_INT, LE_SCAN_WIN,
 				  LE_SCAN_TIMEOUT_BREDR_LE);
 		break;
 
-- 
1.8.1.1


^ permalink raw reply related

* [RFC 3/7] Bluetooth: Setup LE scan with no timeout
From: Andre Guedes @ 2013-02-02  0:44 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1359765878-31409-1-git-send-email-andre.guedes@openbossa.org>

This patch modifies hci_do_le_scan and hci_cancel_le_scan helpers so
we are able to start and stop LE scan with no timeout. This feature
will be used by the LE connection routine.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/hci_core.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 22e77a7..3aa0345 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1618,26 +1618,27 @@ static int hci_do_le_scan(struct hci_dev *hdev, u8 type, u16 interval,
 	if (err < 0)
 		return err;
 
-	queue_delayed_work(hdev->workqueue, &hdev->le_scan_disable,
-			   msecs_to_jiffies(timeout));
+	if (timeout > 0)
+		queue_delayed_work(hdev->workqueue, &hdev->le_scan_disable,
+				   msecs_to_jiffies(timeout));
 
 	return 0;
 }
 
 int hci_cancel_le_scan(struct hci_dev *hdev)
 {
+	struct hci_cp_le_set_scan_enable cp;
+
 	BT_DBG("%s", hdev->name);
 
 	if (!test_bit(HCI_LE_SCAN, &hdev->dev_flags))
 		return -EALREADY;
 
-	if (cancel_delayed_work(&hdev->le_scan_disable)) {
-		struct hci_cp_le_set_scan_enable cp;
+	cancel_delayed_work(&hdev->le_scan_disable);
 
-		/* Send HCI command to disable LE Scan */
-		memset(&cp, 0, sizeof(cp));
-		hci_send_cmd(hdev, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
-	}
+	/* Send HCI command to disable LE Scan */
+	memset(&cp, 0, sizeof(cp));
+	hci_send_cmd(hdev, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
 
 	return 0;
 }
-- 
1.8.1.1


^ permalink raw reply related

* [RFC 2/7] Bluetooth: Make hci_le_create_connection non-static
From: Andre Guedes @ 2013-02-02  0:44 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1359765878-31409-1-git-send-email-andre.guedes@openbossa.org>

Make hci_le_create_connection helper non-static so it can be called
from outside hci_conn.c.

The helper will be used to create a LE connection once the target
device is in-range.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h | 1 +
 net/bluetooth/hci_conn.c         | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 90cf75a..00923ef 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -577,6 +577,7 @@ static inline struct hci_conn *hci_conn_hash_lookup_state(struct hci_dev *hdev,
 void hci_acl_disconn(struct hci_conn *conn, __u8 reason);
 void hci_setup_sync(struct hci_conn *conn, __u16 handle);
 void hci_sco_setup(struct hci_conn *conn, __u8 status);
+void hci_le_create_connection(struct hci_conn *conn);
 
 struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst);
 int hci_conn_del(struct hci_conn *conn);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 25bfce0..bb9a88d 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -31,7 +31,7 @@
 #include <net/bluetooth/a2mp.h>
 #include <net/bluetooth/smp.h>
 
-static void hci_le_create_connection(struct hci_conn *conn)
+void hci_le_create_connection(struct hci_conn *conn)
 {
 	struct hci_dev *hdev = conn->hdev;
 	struct hci_cp_le_create_conn cp;
-- 
1.8.1.1


^ permalink raw reply related

* [RFC 1/7] Bluetooth: Add new connection states
From: Andre Guedes @ 2013-02-02  0:44 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1359765878-31409-1-git-send-email-andre.guedes@openbossa.org>

This patch adds two new connection states which will be used by
hci_conn to establish LE connections.

BT_SCAN state means the controller is scanning for LE devices. Once
the target device is found, the connection goes to BT_DEV_FOUND
state, and then to BT_CONNECT state.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/bluetooth.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 9531bee..add5721 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -126,7 +126,9 @@ enum {
 	BT_CONNECT2,
 	BT_CONFIG,
 	BT_DISCONN,
-	BT_CLOSED
+	BT_CLOSED,
+	BT_SCAN,
+	BT_DEV_FOUND
 };
 
 /* If unused will be removed by compiler */
@@ -151,6 +153,10 @@ static inline const char *state_to_string(int state)
 		return "BT_DISCONN";
 	case BT_CLOSED:
 		return "BT_CLOSED";
+	case BT_SCAN:
+		return "BT_SCAN";
+	case BT_DEV_FOUND:
+		return "BT_DEV_FOUND";
 	}
 
 	return "invalid state";
-- 
1.8.1.1


^ permalink raw reply related

* [RFC 0/7] LE connection routine
From: Andre Guedes @ 2013-02-02  0:44 UTC (permalink / raw)
  To: linux-bluetooth

Hi all,

In order to better support LE connection requirements, such as multiple
connections and re-connections, we should support the general connection
establishment procedure in kernel side. Today, we support only the direct
connection establishment procedure which has some limitations and, therefore,
requires extra connection management at user-space.

According to the spec (Vol 3, Part C, section 9.3.6), the general procedure is
described as follows: The host starts scanning for LE devices, once the device
we want to connect to is in-range, the host stops scanning and initiates a
connection. The procedure is terminated when the connection is established or
when the host terminates the procedure.

This RFC series implements the basic support for general connection procedure.
The first patches do simple changes required to implement this new LE
connection routine. It has not been well tested, but the basic LE connection
and disconnection cases have been covered.

This is an initial work, so it doesn't support multiple LE connection attempts
at the same time (current kernel doesn't support too) and doesn't handle
concurrent device discovery properly.

The next steps are the following:
1. Cover some corner cases in this series.
2. Add support for multiple LE connection attempts.
3. Handle concurrent LE connections and device discovery
4. Add better support for controller which a capable of scanning and initiating
LE connection at the same time.
5. Add API so userspace is able to configure connection parameters.
6. Remove LE connection management code from bluetoothd.

Feedback is welcome.

Best regards,

Andre


Andre Guedes (7):
  Bluetooth: Add new connection states
  Bluetooth: Make hci_le_create_connection non-static
  Bluetooth: Setup LE scan with no timeout
  Bluetooth: Add LE scan type macros
  Bluetooth: Change LE connection routine
  Bluetooth: Handle hci_conn timeout in BT_SCAN state
  Bluetooth: Track the number of hci_conn in BT_SCAN

 include/net/bluetooth/bluetooth.h |  8 +++++++-
 include/net/bluetooth/hci_core.h  |  9 +++++++++
 net/bluetooth/hci_conn.c          | 16 ++++++++++++++--
 net/bluetooth/hci_core.c          | 19 +++++++++++--------
 net/bluetooth/hci_event.c         | 40 +++++++++++++++++++++++++++++++++++++--
 net/bluetooth/mgmt.c              |  5 ++---
 6 files changed, 81 insertions(+), 16 deletions(-)

-- 
1.8.1.1


^ permalink raw reply

* Re: [PATCH BlueZ 1/6 v2] media-api: Add org.bluez.MediaFolder1
From: Johan Hedberg @ 2013-02-01 21:45 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1359754105-8185-1-git-send-email-luiz.dentz@gmail.com>

Hi Luiz,

On Fri, Feb 01, 2013, Luiz Augusto von Dentz wrote:
> This interface adds support for browsing and searching in the player's
> storage using AVRCP 1.4/1.5.
> 
> Some remarks about the design:
> 
>   - Exposing UIDCounter and UIDs was considered, but the spec seems to have
>     missed to define the player's id persistency. There are also the fact that
>     UIDCounter alone does not guarantee persistency across sessions and do not
>     provide what exact items have changed, so in the end exposing these
>     details will bring almost no value.
>   - Indexing or caching the whole media library is not recommended, Bluetooth
>     is too slow for that and even vendors such as Apple do not recommend doing
>     it, so the only items keep in cache are the current listed ones.
>   - Addressed vs Browsed player is done implicitly when accessed, this was done
>     to simplify the API and avoid confusions between applications and players.
> ---
> v2: Remove Buttons property for now
> 
>  doc/media-api.txt | 171 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 171 insertions(+)

All patches in this set have been applied. Thanks.

Johan

^ permalink raw reply

* [PATCH BlueZ 6/6 v2] AVRCP: Set addressed player as browsed player
From: Luiz Augusto von Dentz @ 2013-02-01 21:28 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1359754105-8185-1-git-send-email-luiz.dentz@gmail.com>

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

This send SetBrowsedPlayer after getting the player details.
---
 profiles/audio/avrcp.c  |  73 +++++++++++++++++++++++--
 profiles/audio/player.c | 139 ++++++++++++++++++++++++++++++++++++++++++++++++
 profiles/audio/player.h |   2 +
 3 files changed, 209 insertions(+), 5 deletions(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index d1eb8ce..00eeea1 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -99,6 +99,7 @@
 #define AVRCP_REQUEST_CONTINUING	0x40
 #define AVRCP_ABORT_CONTINUING		0x41
 #define AVRCP_SET_ABSOLUTE_VOLUME	0x50
+#define AVRCP_SET_BROWSED_PLAYER	0x70
 #define AVRCP_GET_FOLDER_ITEMS		0x71
 #define AVRCP_GENERAL_REJECT		0xA0
 
@@ -1912,6 +1913,68 @@ static const char *subtype_to_string(uint32_t subtype)
 	return "None";
 }
 
+static gboolean avrcp_set_browsed_player_rsp(struct avctp *conn,
+						uint8_t *operands,
+						size_t operand_count,
+						void *user_data)
+{
+	struct avrcp *session = user_data;
+	struct avrcp_player *player = session->player;
+	struct media_player *mp = player->user_data;
+	uint32_t items;
+	char **folders, *path;
+	uint8_t depth, count;
+	int i;
+
+	if (operands[3] != AVRCP_STATUS_SUCCESS || operand_count < 13)
+		return FALSE;
+
+	player->uid_counter = bt_get_be16(&operands[4]);
+
+	items = bt_get_be32(&operands[6]);
+
+	depth = operands[13];
+
+	folders = g_new0(char *, depth + 1);
+
+	for (i = 14, count = 0; count < depth; count++) {
+		char *part;
+		uint8_t len;
+
+		len = operands[i++];
+		part = g_memdup(&operands[i], len);
+		i += len;
+		folders[count] = part;
+	}
+
+	path = g_build_pathv("/", folders);
+	g_strfreev(folders);
+
+	media_player_set_folder(mp, path, items);
+
+	g_free(path);
+
+	return FALSE;
+}
+
+static void avrcp_set_browsed_player(struct avrcp *session,
+						struct avrcp_player *player)
+{
+	uint8_t buf[AVRCP_BROWSING_HEADER_LENGTH + 2];
+	struct avrcp_browsing_header *pdu = (void *) buf;
+	uint16_t id;
+
+	memset(buf, 0, sizeof(buf));
+
+	pdu->pdu_id = AVRCP_SET_BROWSED_PLAYER;
+	id = htons(player->id);
+	memcpy(pdu->params, &id, 2);
+	pdu->param_len = htons(2);
+
+	avctp_send_browsing_req(session->conn, buf, sizeof(buf),
+				avrcp_set_browsed_player_rsp, session);
+}
+
 static void avrcp_parse_media_player_item(struct avrcp *session,
 					uint8_t *operands, uint16_t len)
 {
@@ -1950,12 +2013,12 @@ static void avrcp_parse_media_player_item(struct avrcp *session,
 
 	media_player_set_features(mp, features);
 
-	if (operands[26] == 0)
-		return;
-
-	memcpy(name, &operands[27], operands[26]);
+	if (operands[26] != 0) {
+		memcpy(name, &operands[27], operands[26]);
+		media_player_set_name(mp, name);
+	}
 
-	media_player_set_name(mp, name);
+	avrcp_set_browsed_player(session, player);
 }
 
 static gboolean avrcp_get_media_player_list_rsp(struct avctp *conn,
diff --git a/profiles/audio/player.c b/profiles/audio/player.c
index b4efa70..2f4ea83 100644
--- a/profiles/audio/player.c
+++ b/profiles/audio/player.c
@@ -43,6 +43,7 @@
 #include "error.h"
 
 #define MEDIA_PLAYER_INTERFACE "org.bluez.MediaPlayer1"
+#define MEDIA_FOLDER_INTERFACE "org.bluez.MediaFolder1"
 
 struct player_callback {
 	const struct media_player_callback *cbs;
@@ -55,12 +56,18 @@ struct pending_req {
 	const char *value;
 };
 
+struct media_folder {
+	char			*name;		/* Folder name */
+	uint32_t		items;		/* Number of items */
+};
+
 struct media_player {
 	char			*device;	/* Device path */
 	char			*name;		/* Player name */
 	char			*type;		/* Player type */
 	char			*subtype;	/* Player subtype */
 	uint64_t		features[2];	/* Player features */
+	struct media_folder	*folder;	/* Player currenct folder */
 	char			*path;		/* Player object path */
 	GHashTable		*settings;	/* Player settings */
 	GHashTable		*track;		/* Player current track */
@@ -505,6 +512,104 @@ static const GDBusPropertyTable media_player_properties[] = {
 	{ }
 };
 
+static DBusMessage *media_folder_search(DBusConnection *conn, DBusMessage *msg,
+								void *data)
+{
+	return btd_error_failed(msg, strerror(ENOTSUP));
+}
+
+static DBusMessage *media_folder_list_items(DBusConnection *conn,
+						DBusMessage *msg, void *data)
+{
+	return btd_error_failed(msg, strerror(ENOTSUP));
+}
+
+
+static DBusMessage *media_folder_change_folder(DBusConnection *conn,
+						DBusMessage *msg, void *data)
+{
+	return btd_error_failed(msg, strerror(ENOTSUP));
+}
+
+static gboolean folder_name_exists(const GDBusPropertyTable *property,
+								void *data)
+{
+	struct media_player *mp = data;
+
+	if (mp->folder == NULL)
+		return FALSE;
+
+	return mp->folder->name != NULL;
+}
+
+static gboolean get_folder_name(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct media_player *mp = data;
+
+	if (mp->folder == NULL || mp->folder->name == NULL)
+		return FALSE;
+
+	DBG("%s", mp->folder->name);
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING,
+							&mp->folder->name);
+
+	return TRUE;
+}
+
+static gboolean items_exists(const GDBusPropertyTable *property, void *data)
+{
+	struct media_player *mp = data;
+
+	return mp->folder != NULL;
+}
+
+static gboolean get_items(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct media_player *mp = data;
+
+	if (mp->folder == NULL)
+		return FALSE;
+
+	DBG("%u", mp->folder->items);
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT32,
+							&mp->folder->items);
+
+	return TRUE;
+}
+
+static const GDBusMethodTable media_folder_methods[] = {
+	{ GDBUS_EXPERIMENTAL_METHOD("Search",
+			GDBUS_ARGS({ "string", "s" }, { "filter", "a{sv}" }),
+			GDBUS_ARGS({ "folder", "o" }),
+			media_folder_search) },
+	{ GDBUS_EXPERIMENTAL_METHOD("ListItems",
+			GDBUS_ARGS({ "filter", "a{sv}" }),
+			GDBUS_ARGS({ "items", "a{oa{sv}}" }),
+			media_folder_list_items) },
+	{ GDBUS_EXPERIMENTAL_METHOD("ChangeFolder",
+			GDBUS_ARGS({ "folder", "o" }), NULL,
+			media_folder_change_folder) },
+	{ }
+};
+
+static const GDBusPropertyTable media_folder_properties[] = {
+	{ "Name", "s", get_folder_name, NULL, folder_name_exists,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+	{ "NumberOfItems", "u", get_items, NULL, items_exists,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+	{ }
+};
+
+static void media_folder_destroy(struct media_folder *folder)
+{
+	g_free(folder->name);
+	g_free(folder);
+}
+
 void media_player_destroy(struct media_player *mp)
 {
 	DBG("%s", mp->path);
@@ -521,6 +626,13 @@ void media_player_destroy(struct media_player *mp)
 	if (mp->process_id > 0)
 		g_source_remove(mp->process_id);
 
+	if (mp->folder) {
+		g_dbus_unregister_interface(btd_get_dbus_connection(),
+						mp->path,
+						MEDIA_FOLDER_INTERFACE);
+		media_folder_destroy(mp->folder);
+	}
+
 	g_slist_free_full(mp->pending, g_free);
 
 	g_timer_destroy(mp->progress);
@@ -730,6 +842,33 @@ void media_player_set_name(struct media_player *mp, const char *name)
 					"Name");
 }
 
+void media_player_set_folder(struct media_player *mp, const char *path,
+								uint32_t items)
+{
+	struct media_folder *folder;
+
+	DBG("%s items %u", path, items);
+
+	if (mp->folder != NULL)
+		media_folder_destroy(mp->folder);
+
+	folder = g_new0(struct media_folder, 1);
+	folder->name = g_strdup(path);
+	folder->items = items;
+	mp->folder = folder;
+
+	if (!g_dbus_register_interface(btd_get_dbus_connection(),
+					mp->path, MEDIA_FOLDER_INTERFACE,
+					media_folder_methods,
+					NULL,
+					media_folder_properties, mp, NULL)) {
+		error("D-Bus failed to register %s on %s path",
+					MEDIA_FOLDER_INTERFACE, mp->path);
+		media_folder_destroy(mp->folder);
+		mp->folder = NULL;
+	}
+}
+
 void media_player_set_features(struct media_player *mp, uint64_t *features)
 {
 	DBG("0x%08zx %08zx", features[0], features[1]);
diff --git a/profiles/audio/player.h b/profiles/audio/player.h
index b546ba7..1ac9800 100644
--- a/profiles/audio/player.h
+++ b/profiles/audio/player.h
@@ -51,6 +51,8 @@ void media_player_set_type(struct media_player *mp, const char *type);
 void media_player_set_subtype(struct media_player *mp, const char *subtype);
 void media_player_set_features(struct media_player *mp, uint64_t *features);
 void media_player_set_name(struct media_player *mp, const char *name);
+void media_player_set_folder(struct media_player *mp, const char *path,
+								uint32_t items);
 
 void media_player_set_callbacks(struct media_player *mp,
 				const struct media_player_callback *cbs,
-- 
1.8.1


^ permalink raw reply related

* [PATCH BlueZ 5/6 v2] AVRCP: Split event handing to its own functions
From: Luiz Augusto von Dentz @ 2013-02-01 21:28 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1359754105-8185-1-git-send-email-luiz.dentz@gmail.com>

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

---
 profiles/audio/avrcp.c | 141 +++++++++++++++++++++++++++++--------------------
 1 file changed, 85 insertions(+), 56 deletions(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 793b5cf..d1eb8ce 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -2005,21 +2005,94 @@ static void avrcp_get_media_player_list(struct avrcp *session)
 				avrcp_get_media_player_list_rsp, session);
 }
 
+static void avrcp_volume_changed(struct avrcp *session,
+						struct avrcp_header *pdu)
+{
+	struct avrcp_player *player = session->player;
+	uint8_t volume;
+
+	if (player == NULL)
+		return;
+
+	volume = pdu->params[1] & 0x7F;
+
+	player->cb->set_volume(volume, session->dev, player->user_data);
+}
+
+static void avrcp_status_changed(struct avrcp *session,
+						struct avrcp_header *pdu)
+{
+	struct avrcp_player *player = session->player;
+	struct media_player *mp = player->user_data;
+	uint8_t value;
+	const char *curval, *strval;
+
+	value = pdu->params[1];
+
+	curval = media_player_get_status(mp);
+	strval = status_to_string(value);
+
+	if (g_strcmp0(curval, strval) == 0)
+		return;
+
+	media_player_set_status(mp, strval);
+	avrcp_get_play_status(session);
+}
+
+static void avrcp_track_changed(struct avrcp *session,
+						struct avrcp_header *pdu)
+{
+	avrcp_get_element_attributes(session);
+	avrcp_get_play_status(session);
+
+}
+
+static void avrcp_setting_changed(struct avrcp *session,
+						struct avrcp_header *pdu)
+{
+	struct avrcp_player *player = session->player;
+	struct media_player *mp = player->user_data;
+	uint8_t count = pdu->params[1];
+	int i;
+
+	for (i = 2; count > 0; count--, i += 2) {
+		const char *key;
+		const char *value;
+
+		key = attr_to_str(pdu->params[i]);
+		if (key == NULL)
+			continue;
+
+		value = attrval_to_str(pdu->params[i], pdu->params[i + 1]);
+		if (value == NULL)
+			continue;
+
+		media_player_set_setting(mp, key, value);
+	}
+}
+
+static void avrcp_addressed_player_changed(struct avrcp *session,
+						struct avrcp_header *pdu)
+{
+	struct avrcp_player *player = session->player;
+	uint16_t id = bt_get_be16(&pdu->params[1]);
+
+	if (player->id == id)
+		return;
+
+	player->id = id;
+	player->uid_counter = bt_get_le16(&pdu->params[3]);
+	avrcp_get_media_player_list(session);
+}
+
 static gboolean avrcp_handle_event(struct avctp *conn,
 					uint8_t code, uint8_t subunit,
 					uint8_t *operands, size_t operand_count,
 					void *user_data)
 {
 	struct avrcp *session = user_data;
-	struct avrcp_player *player = session->player;
 	struct avrcp_header *pdu = (void *) operands;
-	struct media_player *mp;
 	uint8_t event;
-	uint8_t value;
-	uint8_t count;
-	uint16_t id;
-	const char *curval, *strval;
-	int i;
 
 	if (code != AVC_CTYPE_INTERIM && code != AVC_CTYPE_CHANGED)
 		return FALSE;
@@ -2034,63 +2107,19 @@ static gboolean avrcp_handle_event(struct avctp *conn,
 
 	switch (event) {
 	case AVRCP_EVENT_VOLUME_CHANGED:
-		value = pdu->params[1] & 0x7F;
-
-		if (player)
-			player->cb->set_volume(value, session->dev,
-							player->user_data);
-
+		avrcp_volume_changed(session, pdu);
 		break;
 	case AVRCP_EVENT_STATUS_CHANGED:
-		mp = player->user_data;
-		value = pdu->params[1];
-
-		curval = media_player_get_status(mp);
-		strval = status_to_string(value);
-
-		if (g_strcmp0(curval, strval) != 0) {
-			media_player_set_status(mp, strval);
-			avrcp_get_play_status(session);
-		}
-
+		avrcp_status_changed(session, pdu);
 		break;
 	case AVRCP_EVENT_TRACK_CHANGED:
-		avrcp_get_element_attributes(session);
-		avrcp_get_play_status(session);
-
+		avrcp_track_changed(session, pdu);
 		break;
-
 	case AVRCP_EVENT_SETTINGS_CHANGED:
-		mp = player->user_data;
-		count = pdu->params[1];
-
-		for (i = 2; count > 0; count--, i += 2) {
-			const char *key;
-			const char *value;
-
-			key = attr_to_str(pdu->params[i]);
-			if (key == NULL)
-				continue;
-
-			value = attrval_to_str(pdu->params[i],
-						pdu->params[i + 1]);
-			if (value == NULL)
-				continue;
-
-			media_player_set_setting(mp, key, value);
-		}
-
+		avrcp_setting_changed(session, pdu);
 		break;
-
 	case AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED:
-		id = bt_get_be16(&pdu->params[1]);
-
-		if (player->id == id)
-			break;
-
-		player->id = id;
-		player->uid_counter = bt_get_be16(&pdu->params[3]);
-		avrcp_get_media_player_list(session);
+		avrcp_addressed_player_changed(session, pdu);
 		break;
 	}
 
-- 
1.8.1


^ permalink raw reply related

* [PATCH BlueZ 4/6 v2] AVRCP: Get player list if supported
From: Luiz Augusto von Dentz @ 2013-02-01 21:28 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1359754105-8185-1-git-send-email-luiz.dentz@gmail.com>

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

If addressed player changed is supported get the list of players
---
 profiles/audio/avrcp.c  | 147 ++++++++++++++++++++++++++++++++++++++++++++++++
 profiles/audio/player.c | 119 +++++++++++++++++++++++++++++++++++++++
 profiles/audio/player.h |   4 ++
 3 files changed, 270 insertions(+)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 8187ddf..793b5cf 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -99,6 +99,7 @@
 #define AVRCP_REQUEST_CONTINUING	0x40
 #define AVRCP_ABORT_CONTINUING		0x41
 #define AVRCP_SET_ABSOLUTE_VOLUME	0x50
+#define AVRCP_GET_FOLDER_ITEMS		0x71
 #define AVRCP_GENERAL_REJECT		0xA0
 
 /* Capabilities for AVRCP_GET_CAPABILITIES pdu */
@@ -1859,6 +1860,151 @@ static void avrcp_get_element_attributes(struct avrcp *session)
 					session);
 }
 
+static const char *type_to_string(uint8_t type)
+{
+	switch (type & 0x0F) {
+	case 0x01:
+		return "Audio";
+	case 0x02:
+		return "Video";
+	case 0x03:
+		return "Audio, Video";
+	case 0x04:
+		return "Audio Broadcasting";
+	case 0x05:
+		return "Audio, Audio Broadcasting";
+	case 0x06:
+		return "Video, Audio Broadcasting";
+	case 0x07:
+		return "Audio, Video, Audio Broadcasting";
+	case 0x08:
+		return "Video Broadcasting";
+	case 0x09:
+		return "Audio, Video Broadcasting";
+	case 0x0A:
+		return "Video, Video Broadcasting";
+	case 0x0B:
+		return "Audio, Video, Video Broadcasting";
+	case 0x0C:
+		return "Audio Broadcasting, Video Broadcasting";
+	case 0x0D:
+		return "Audio, Audio Broadcasting, Video Broadcasting";
+	case 0x0E:
+		return "Video, Audio Broadcasting, Video Broadcasting";
+	case 0x0F:
+		return "Audio, Video, Audio Broadcasting, Video Broadcasting";
+	}
+
+	return "None";
+}
+
+static const char *subtype_to_string(uint32_t subtype)
+{
+	switch (subtype & 0x03) {
+	case 0x01:
+		return "Audio Book";
+	case 0x02:
+		return "Podcast";
+	case 0x03:
+		return "Audio Book, Podcast";
+	}
+
+	return "None";
+}
+
+static void avrcp_parse_media_player_item(struct avrcp *session,
+					uint8_t *operands, uint16_t len)
+{
+	struct avrcp_player *player = session->player;
+	struct media_player *mp = player->user_data;
+	uint16_t id;
+	uint32_t subtype;
+	const char *curval, *strval;
+	uint64_t features[2];
+	char name[255];
+
+	if (len < 28)
+		return;
+
+	id = bt_get_be16(&operands[0]);
+
+	if (player->id != id)
+		return;
+
+	media_player_set_type(mp, type_to_string(operands[2]));
+
+	subtype = bt_get_be32(&operands[3]);
+
+	media_player_set_subtype(mp, subtype_to_string(subtype));
+
+	curval = media_player_get_status(mp);
+	strval = status_to_string(operands[7]);
+
+	if (g_strcmp0(curval, strval) != 0) {
+		media_player_set_status(mp, strval);
+		avrcp_get_play_status(session);
+	}
+
+	features[0] = bt_get_be64(&operands[8]);
+	features[1] = bt_get_be64(&operands[16]);
+
+	media_player_set_features(mp, features);
+
+	if (operands[26] == 0)
+		return;
+
+	memcpy(name, &operands[27], operands[26]);
+
+	media_player_set_name(mp, name);
+}
+
+static gboolean avrcp_get_media_player_list_rsp(struct avctp *conn,
+						uint8_t *operands,
+						size_t operand_count,
+						void *user_data)
+{
+	struct avrcp *session = user_data;
+	uint16_t count;
+	int i;
+
+	if (operands[3] != AVRCP_STATUS_SUCCESS || operand_count < 5)
+		return FALSE;
+
+	count = bt_get_be16(&operands[6]);
+
+	for (i = 8; count; count--) {
+		uint8_t type;
+		uint16_t len;
+
+		type = operands[i++];
+		len = bt_get_be16(&operands[i]);
+		i += 2;
+
+		if (type != 0x01) {
+			i += len;
+			continue;
+		}
+
+		avrcp_parse_media_player_item(session, &operands[i], len);
+	}
+
+	return FALSE;
+}
+
+static void avrcp_get_media_player_list(struct avrcp *session)
+{
+	uint8_t buf[AVRCP_BROWSING_HEADER_LENGTH + 10];
+	struct avrcp_browsing_header *pdu = (void *) buf;
+
+	memset(buf, 0, sizeof(buf));
+
+	pdu->pdu_id = AVRCP_GET_FOLDER_ITEMS;
+	pdu->param_len = htons(10);
+
+	avctp_send_browsing_req(session->conn, buf, sizeof(buf),
+				avrcp_get_media_player_list_rsp, session);
+}
+
 static gboolean avrcp_handle_event(struct avctp *conn,
 					uint8_t code, uint8_t subunit,
 					uint8_t *operands, size_t operand_count,
@@ -1944,6 +2090,7 @@ static gboolean avrcp_handle_event(struct avctp *conn,
 
 		player->id = id;
 		player->uid_counter = bt_get_be16(&pdu->params[3]);
+		avrcp_get_media_player_list(session);
 		break;
 	}
 
diff --git a/profiles/audio/player.c b/profiles/audio/player.c
index fb750fd..b4efa70 100644
--- a/profiles/audio/player.c
+++ b/profiles/audio/player.c
@@ -57,6 +57,10 @@ struct pending_req {
 
 struct media_player {
 	char			*device;	/* Device path */
+	char			*name;		/* Player name */
+	char			*type;		/* Player type */
+	char			*subtype;	/* Player subtype */
+	uint64_t		features[2];	/* Player features */
 	char			*path;		/* Player object path */
 	GHashTable		*settings;	/* Player settings */
 	GHashTable		*track;		/* Player current track */
@@ -272,6 +276,72 @@ static gboolean get_device(const GDBusPropertyTable *property,
 	return TRUE;
 }
 
+static gboolean name_exists(const GDBusPropertyTable *property, void *data)
+{
+	struct media_player *mp = data;
+
+	return mp->name != NULL;
+}
+
+static gboolean get_name(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct media_player *mp = data;
+
+	if (mp->name == NULL)
+		return FALSE;
+
+	DBG("%s", mp->name);
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &mp->name);
+
+	return TRUE;
+}
+
+static gboolean type_exists(const GDBusPropertyTable *property, void *data)
+{
+	struct media_player *mp = data;
+
+	return mp->type != NULL;
+}
+
+static gboolean get_type(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct media_player *mp = data;
+
+	if (mp->type == NULL)
+		return FALSE;
+
+	DBG("%s", mp->type);
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &mp->type);
+
+	return TRUE;
+}
+
+static gboolean subtype_exists(const GDBusPropertyTable *property, void *data)
+{
+	struct media_player *mp = data;
+
+	return mp->subtype != NULL;
+}
+
+static gboolean get_subtype(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct media_player *mp = data;
+
+	if (mp->subtype == NULL)
+		return FALSE;
+
+	DBG("%s", mp->subtype);
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &mp->subtype);
+
+	return TRUE;
+}
+
 static DBusMessage *media_player_play(DBusConnection *conn, DBusMessage *msg,
 								void *data)
 {
@@ -410,6 +480,12 @@ static const GDBusSignalTable media_player_signals[] = {
 };
 
 static const GDBusPropertyTable media_player_properties[] = {
+	{ "Name", "s", get_name, NULL, name_exists,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+	{ "Type", "s", get_type, NULL, type_exists,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+	{ "Subtype", "s", get_subtype, NULL, subtype_exists,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
 	{ "Position", "u", get_position, NULL, NULL,
 					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
 	{ "Status", "s", get_status, NULL, status_exists,
@@ -452,6 +528,9 @@ void media_player_destroy(struct media_player *mp)
 	g_free(mp->status);
 	g_free(mp->path);
 	g_free(mp->device);
+	g_free(mp->subtype);
+	g_free(mp->type);
+	g_free(mp->name);
 	g_free(mp);
 }
 
@@ -618,6 +697,46 @@ void media_player_set_metadata(struct media_player *mp, const char *key,
 	g_hash_table_replace(mp->track, g_strdup(key), value);
 }
 
+void media_player_set_type(struct media_player *mp, const char *type)
+{
+	DBG("%s", type);
+
+	mp->type = g_strdup(type);
+
+	g_dbus_emit_property_changed(btd_get_dbus_connection(),
+					mp->path, MEDIA_PLAYER_INTERFACE,
+					"Type");
+}
+
+void media_player_set_subtype(struct media_player *mp, const char *subtype)
+{
+	DBG("%s", subtype);
+
+	mp->subtype = g_strdup(subtype);
+
+	g_dbus_emit_property_changed(btd_get_dbus_connection(),
+					mp->path, MEDIA_PLAYER_INTERFACE,
+					"Subtype");
+}
+
+void media_player_set_name(struct media_player *mp, const char *name)
+{
+	DBG("%s", name);
+
+	mp->name = g_strdup(name);
+
+	g_dbus_emit_property_changed(btd_get_dbus_connection(),
+					mp->path, MEDIA_PLAYER_INTERFACE,
+					"Name");
+}
+
+void media_player_set_features(struct media_player *mp, uint64_t *features)
+{
+	DBG("0x%08zx %08zx", features[0], features[1]);
+
+	memcpy(features, mp->features, sizeof(mp->features));
+}
+
 void media_player_set_callbacks(struct media_player *mp,
 				const struct media_player_callback *cbs,
 				void *user_data)
diff --git a/profiles/audio/player.h b/profiles/audio/player.h
index fec1d06..b546ba7 100644
--- a/profiles/audio/player.h
+++ b/profiles/audio/player.h
@@ -47,6 +47,10 @@ const char *media_player_get_status(struct media_player *mp);
 void media_player_set_status(struct media_player *mp, const char *status);
 void media_player_set_metadata(struct media_player *mp, const char *key,
 						void *data, size_t len);
+void media_player_set_type(struct media_player *mp, const char *type);
+void media_player_set_subtype(struct media_player *mp, const char *subtype);
+void media_player_set_features(struct media_player *mp, uint64_t *features);
+void media_player_set_name(struct media_player *mp, const char *name);
 
 void media_player_set_callbacks(struct media_player *mp,
 				const struct media_player_callback *cbs,
-- 
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