* Re: [PATCH BlueZ 1/2] avctp: Create ignore quirk
From: Joao Paulo Rechi Vita @ 2013-01-25 17:05 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZK7nK6LPq1rYv40fq_Ko_MB2f-ANG08ozHhSLyg_BUAMg@mail.gmail.com>
On Fri, Jan 25, 2013 at 10:35 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Joao,
>
> On Tue, Jan 22, 2013 at 8:18 PM, João Paulo Rechi Vita
> <jprvita@openbossa.org> wrote:
>> Create a quirk to be able to accept and trow away certain keys.
>> ---
>> profiles/audio/avctp.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
>> index 61890cc..f7e607e 100644
>> --- a/profiles/audio/avctp.c
>> +++ b/profiles/audio/avctp.c
>> @@ -58,7 +58,8 @@
>> #include "avctp.h"
>> #include "avrcp.h"
>>
>> -#define QUIRK_NO_RELEASE 1 << 0
>> +#define QUIRK_NO_RELEASE 1 << 0
>> +#define QUIRK_IGNORE 1 << 1
>>
>> /* Message types */
>> #define AVCTP_COMMAND 0
>> @@ -287,6 +288,11 @@ static size_t handle_panel_passthrough(struct avctp *session,
>>
>> key_quirks = session->key_quirks[key_map[i].avc];
>>
>> + if (key_quirks & QUIRK_IGNORE) {
>> + DBG("AV/C: ignoring %s %s", key_map[i].name, status);
>> + break;
>> + }
>> +
>> if (key_quirks & QUIRK_NO_RELEASE) {
>> if (!pressed) {
>> DBG("AV/C: Ignoring release");
>> --
>> 1.7.11.7
>
> In the end I think we should just accept the commands normally, let me
> quote the recommendations (RD=Rendering Device MP=Media Player):
>
> "Recommendation 16:
>
> If volume is changed on the RD, the RD should not send an AVRCP
> volume command to the MP device.
>
> Motivation 16:
>
> Sending an AVRCP volume command to the MP may cause the MP to send
> again an AVRCP volume
> command to the RD device which could lead to an endless loop of
> AVRCP volume commands.
>
> Recommendation 17:
>
> If a device receives an AVRCP volume command, it shall not send back
> an AVRCP volume command.
>
> Motivation 17:
>
> This will also ensure that endless loop does not happen with
> existing devices which do not comply with the
> recommendation."
>
> So there is nothing against the RD accepting the Volume Up/Down it
> should just no send it back.
>
All right, so I'll update the patches to forward the Volume Up and
Down to uinput.
--
João Paulo Rechi Vita
Openbossa Labs - INdT
^ permalink raw reply
* [PATCH 1/1] obexd: Add PushMessage
From: Christian Fetzer @ 2013-01-25 16:44 UTC (permalink / raw)
To: linux-bluetooth
From: Christian Fetzer <christian.fetzer@bmw-carit.de>
Push message has been implemented similar to send file (OPP),
the message to send (in bMessage format) is read from a file.
---
doc/obex-api.txt | 22 ++++++++
obexd/client/map.c | 161 +++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 183 insertions(+)
diff --git a/doc/obex-api.txt b/doc/obex-api.txt
index 6246933..759c4d8 100644
--- a/doc/obex-api.txt
+++ b/doc/obex-api.txt
@@ -630,6 +630,28 @@ Methods void SetFolder(string name)
Possible errors: org.bluez.obex.Error.Failed
+ object, dict
+ PushMessage(string sourcefile, string folder, dict args)
+
+ Transfer a message (in bMessage format) to the
+ remote device.
+
+ The message is transferred either to the given folder,
+ or to the current folder if folder is omitted.
+
+ Possible args: Transparent, Retry, Charset
+
+ The returned path represents the newly created transfer,
+ which should be used to find out if the content has been
+ successfully transferred or if the operation fails.
+
+ The properties of this transfer are also returned along
+ with the object path, to avoid a call to GetAll.
+
+ Possible errors: org.bluez.obex.Error.InvalidArguments
+ org.bluez.obex.Error.Failed
+
+
Filter: uint16 Offset:
Offset of the first item, default is 0
diff --git a/obexd/client/map.c b/obexd/client/map.c
index ea5681a..2b665ec 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -52,6 +52,7 @@
#define DEFAULT_COUNT 1024
#define DEFAULT_OFFSET 0
+#define CHARSET_NATIVE 0
#define CHARSET_UTF8 1
static const char * const filter_list[] = {
@@ -1495,6 +1496,160 @@ fail:
return reply;
}
+static DBusMessage *push_message(struct map_data *map,
+ DBusMessage *message,
+ const char *filename,
+ const char *folder,
+ GObexApparam *apparam)
+{
+ struct obc_transfer *transfer;
+ GError *err = NULL;
+ DBusMessage *reply;
+
+ transfer = obc_transfer_put("x-bt/message", folder, filename,
+ NULL, 0, &err);
+ if (transfer == NULL) {
+ g_obex_apparam_free(apparam);
+ goto fail;
+ }
+
+ obc_transfer_set_apparam(transfer, apparam);
+
+ if (!obc_session_queue(map->session, transfer, NULL, NULL, &err))
+ goto fail;
+
+ return obc_transfer_create_dbus_reply(transfer, message);
+
+fail:
+ reply = g_dbus_create_error(message, ERROR_INTERFACE ".Failed", "%s",
+ err->message);
+ g_error_free(err);
+ return reply;
+}
+
+static GObexApparam *parse_transparent(GObexApparam *apparam,
+ DBusMessageIter *iter)
+{
+ dbus_bool_t transparent;
+
+ if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_BOOLEAN)
+ return NULL;
+
+ dbus_message_iter_get_basic(iter, &transparent);
+
+ return g_obex_apparam_set_uint8(apparam, MAP_AP_TRANSPARENT,
+ transparent ? TRUE : FALSE);
+}
+
+static GObexApparam *parse_retry(GObexApparam *apparam, DBusMessageIter *iter)
+{
+ dbus_bool_t retry;
+
+ if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_BOOLEAN)
+ return NULL;
+
+ dbus_message_iter_get_basic(iter, &retry);
+
+ return g_obex_apparam_set_uint8(apparam, MAP_AP_RETRY,
+ retry ? TRUE : FALSE);
+}
+
+static GObexApparam *parse_charset(GObexApparam *apparam, DBusMessageIter *iter)
+{
+ guint8 charset = 0;
+ const char *string;
+
+ if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_STRING)
+ return NULL;
+
+ dbus_message_iter_get_basic(iter, &string);
+
+ if (strcasecmp(string, "native") == 0)
+ charset = CHARSET_NATIVE;
+ else if (strcasecmp(string, "utf8") == 0)
+ charset = CHARSET_UTF8;
+ else
+ return NULL;
+
+ return g_obex_apparam_set_uint8(apparam, MAP_AP_CHARSET, charset);
+}
+
+static GObexApparam *parse_push_options(GObexApparam *apparam,
+ DBusMessageIter *iter)
+{
+ DBusMessageIter array;
+
+ if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_ARRAY)
+ return NULL;
+
+ dbus_message_iter_recurse(iter, &array);
+
+ while (dbus_message_iter_get_arg_type(&array) == DBUS_TYPE_DICT_ENTRY) {
+ const char *key;
+ DBusMessageIter value, entry;
+
+ dbus_message_iter_recurse(&array, &entry);
+ dbus_message_iter_get_basic(&entry, &key);
+
+ dbus_message_iter_next(&entry);
+ dbus_message_iter_recurse(&entry, &value);
+
+ if (strcasecmp(key, "Transparent") == 0) {
+ if (parse_transparent(apparam, &value) == NULL)
+ return NULL;
+ } else if (strcasecmp(key, "Retry") == 0) {
+ if (parse_retry(apparam, &value) == NULL)
+ return NULL;
+ } else if (strcasecmp(key, "Charset") == 0) {
+ if (parse_charset(apparam, &value) == NULL)
+ return NULL;
+ }
+
+ dbus_message_iter_next(&array);
+ }
+
+ return apparam;
+}
+
+static DBusMessage *map_push_message(DBusConnection *connection,
+ DBusMessage *message, void *user_data)
+{
+ struct map_data *map = user_data;
+ char *filename;
+ char *folder;
+ GObexApparam *apparam;
+ DBusMessageIter args;
+
+ dbus_message_iter_init(message, &args);
+
+ if (dbus_message_iter_get_arg_type(&args) != DBUS_TYPE_STRING)
+ return g_dbus_create_error(message,
+ ERROR_INTERFACE ".InvalidArguments", NULL);
+
+ dbus_message_iter_get_basic(&args, &filename);
+
+ dbus_message_iter_next(&args);
+
+ if (dbus_message_iter_get_arg_type(&args) != DBUS_TYPE_STRING) {
+ return g_dbus_create_error(message,
+ ERROR_INTERFACE ".InvalidArguments", NULL);
+ }
+
+ dbus_message_iter_get_basic(&args, &folder);
+
+ dbus_message_iter_next(&args);
+
+ apparam = g_obex_apparam_set_uint8(NULL, MAP_AP_CHARSET, CHARSET_UTF8);
+
+ if (parse_push_options(apparam, &args) == NULL) {
+ g_obex_apparam_free(apparam);
+ return g_dbus_create_error(message,
+ ERROR_INTERFACE ".InvalidArguments", NULL);
+ }
+
+ return push_message(map, message, filename, folder, apparam);
+}
+
static const GDBusMethodTable map_methods[] = {
{ GDBUS_ASYNC_METHOD("SetFolder",
GDBUS_ARGS({ "name", "s" }), NULL,
@@ -1515,6 +1670,12 @@ static const GDBusMethodTable map_methods[] = {
NULL,
NULL,
map_update_inbox) },
+ { GDBUS_ASYNC_METHOD("PushMessage",
+ GDBUS_ARGS({ "file", "s" }, { "folder", "s" },
+ { "args", "a{sv}" }),
+ GDBUS_ARGS({ "transfer", "o" },
+ { "properties", "a{sv}" }),
+ map_push_message) },
{ }
};
--
1.8.1
^ permalink raw reply related
* Re: [PATCH BlueZ] adapter: Fix not removing a device when the adapter is down
From: Johan Hedberg @ 2013-01-25 16:30 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <1359124259-31316-1-git-send-email-vinicius.gomes@openbossa.org>
Hi Vinicius,
On Fri, Jan 25, 2013, Vinicius Costa Gomes wrote:
> It is a documented error that MGMT_UNPAIR_DEVICE should return an
> error if the controller is not powered.
>
> This error is being ignored, and if the user asks for a device to be
> removed while the controller is powered down, the keys associated with
> that device will still be in the kernel side.
>
> The solution is to return the user an error when it is asked to remove
> a device and the controller is not powered.
> ---
> src/adapter.c | 3 +++
> 1 file changed, 3 insertions(+)
Applied. Thanks.
Johan
^ permalink raw reply
* [PATCH BlueZ] adapter: Fix not removing a device when the adapter is down
From: Vinicius Costa Gomes @ 2013-01-25 14:30 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Vinicius Costa Gomes
It is a documented error that MGMT_UNPAIR_DEVICE should return an error
if the controller is not powered.
This error is being ignored, and if the user asks for a device to be removed
while the controller is powered down, the keys associated with that device
will still be in the kernel side.
The solution is to return the user an error when it is asked to remove a
device and the controller is not powered.
---
src/adapter.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/adapter.c b/src/adapter.c
index beb50cd..3f39d82 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2097,6 +2097,9 @@ static DBusMessage *remove_device(DBusConnection *conn,
if (!list)
return btd_error_does_not_exist(msg);
+ if (!(adapter->current_settings & MGMT_SETTING_POWERED))
+ return btd_error_not_ready(msg);
+
device = list->data;
device_set_temporary(device, TRUE);
--
1.8.1.1
^ permalink raw reply related
* [RFC BlueZ] media-api: Add org.bluez.MediaFolder1
From: Luiz Augusto von Dentz @ 2013-01-25 13:38 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
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.
---
doc/media-api.txt | 183 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 183 insertions(+)
diff --git a/doc/media-api.txt b/doc/media-api.txt
index 972716e..54e53b9 100644
--- a/doc/media-api.txt
+++ b/doc/media-api.txt
@@ -226,6 +226,189 @@ Properties string Equalizer [readwrite]
Device object path.
+ string Name [readonly]
+
+ Player name
+
+ boolean Browsable [readonly]
+
+ If present indicates the player can be browsed using
+ MediaLibrary interface.
+
+ Possible values:
+
+ True: Supported and active
+ False: Supported but inactive
+
+ Note: If supported but inactive clients can enable it
+ by using MediaLibrary interface but it might interfere
+ in the playback of other players.
+
+
+ boolean Searchable [readonly]
+
+ If present indicates the player can be searched using
+ MediaLibrary interface.
+
+ Possible values:
+
+ True: Supported and active
+ False: Supported but inactive
+
+ Note: If supported but inactive clients can enable it
+ by using MediaLibrary interface but it might interfere
+ in the playback of other players.
+
+ array{string} Buttons [readonly]
+
+ If available indicates the buttons supported by the
+ player.
+
+ Possible values:
+
+ "Play", "Pause", "Stop", "Next", "Previous",
+ "FastForward" and "Rewind"
+
+ Default value: All
+
+MediaFolder1 hierarchy
+======================
+
+Service unique name (Target role)
+ org.bluez (Controller role)
+Interface org.bluez.MediaFolder1 [Experimental]
+Object path freely definable (Target role)
+ [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/playerX
+ (Controller role)
+
+Methods object Search(string value, dict filter)
+
+ Return a folder object containing the search result.
+
+ To list the items found use the folder object returned
+ and pass to ChangeFolder.
+
+ array{objects, properties} ListItems(dict filter)
+
+ Return a list of items found
+
+ void ChangeFolder(object folder)
+
+ Change current folder.
+
+ Note: By changing folder the items of previous folder
+ might be destroyed and have to be listed again, the
+ exception is NowPlaying folder which should be always
+ present while the player is active.
+
+Properties uint32 NumberOfItems:
+
+ Number of items in the folder
+
+ string Name:
+
+ Folder name:
+
+ Possible values:
+ "/Filesystem/...": Filesystem scope
+ "/NowPlaying/...": NowPlaying scope
+
+ Note: /NowPlaying folder might not be listed if player
+ is stopped, folders created by Search are virtual so
+ once another Search is perform or the folder is
+ changed using ChangeFolder it will no longer be listed.
+
+Filters uint32 Start:
+
+ Offset of the first item.
+
+ Default value: 0
+
+ uint32 End:
+
+ Offset of the last item.
+
+ Default value: NumbeOfItems
+
+ array{string} Attributes
+
+ Item properties that should be included in the list.
+
+ Possible Values:
+
+ "title", "artist", "album", "genre",
+ "number-of-tracks", "number", "duration"
+
+ Default Value: All
+
+MediaItem1 hierarchy
+====================
+
+Service unique name (Target role)
+ org.bluez (Controller role)
+Interface org.bluez.MediaItem1 [Experimental]
+Object path freely definable (Target role)
+ [variable
+ prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/playerX/itemX
+ (Controller role)
+
+Methods void Play()
+
+ Play item
+
+ void AddtoNowPlaying()
+
+ Add item to now playing list
+
+Properties string Name [readonly]
+
+ Item displayable name
+
+ boolean Folder [readonly]
+
+ Indicates whether the item is a folder
+
+ string Type [readonly]
+
+ Item type
+
+ Possible values:
+
+ Folder: "Mixed", "Titles", "Albums", "Artists"
+ Other Items: "Video", "Audio"
+
+ boolean Playable [readonly]
+
+ Indicates if the item can be played
+
+ string Title:
+
+ Item title name
+
+ string Artist:
+
+ Item artist name
+
+ string Album:
+
+ Item album name
+
+ string Genre:
+
+ Item genre name
+
+ uint32 NumberOfTracks:
+
+ Item album number of tracks in total
+
+ uint32 Number:
+
+ Item album number
+
+ uint32 Duration:
+
+ Item duration in milliseconds
+
MediaEndpoint1 hierarchy
========================
--
1.8.1
^ permalink raw reply related
* Re: [PATCH BlueZ 1/2] avctp: Create ignore quirk
From: Luiz Augusto von Dentz @ 2013-01-25 13:35 UTC (permalink / raw)
To: João Paulo Rechi Vita; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1358878703-8100-2-git-send-email-jprvita@openbossa.org>
Hi Joao,
On Tue, Jan 22, 2013 at 8:18 PM, João Paulo Rechi Vita
<jprvita@openbossa.org> wrote:
> Create a quirk to be able to accept and trow away certain keys.
> ---
> profiles/audio/avctp.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
> index 61890cc..f7e607e 100644
> --- a/profiles/audio/avctp.c
> +++ b/profiles/audio/avctp.c
> @@ -58,7 +58,8 @@
> #include "avctp.h"
> #include "avrcp.h"
>
> -#define QUIRK_NO_RELEASE 1 << 0
> +#define QUIRK_NO_RELEASE 1 << 0
> +#define QUIRK_IGNORE 1 << 1
>
> /* Message types */
> #define AVCTP_COMMAND 0
> @@ -287,6 +288,11 @@ static size_t handle_panel_passthrough(struct avctp *session,
>
> key_quirks = session->key_quirks[key_map[i].avc];
>
> + if (key_quirks & QUIRK_IGNORE) {
> + DBG("AV/C: ignoring %s %s", key_map[i].name, status);
> + break;
> + }
> +
> if (key_quirks & QUIRK_NO_RELEASE) {
> if (!pressed) {
> DBG("AV/C: Ignoring release");
> --
> 1.7.11.7
In the end I think we should just accept the commands normally, let me
quote the recommendations (RD=Rendering Device MP=Media Player):
"Recommendation 16:
If volume is changed on the RD, the RD should not send an AVRCP
volume command to the MP device.
Motivation 16:
Sending an AVRCP volume command to the MP may cause the MP to send
again an AVRCP volume
command to the RD device which could lead to an endless loop of
AVRCP volume commands.
Recommendation 17:
If a device receives an AVRCP volume command, it shall not send back
an AVRCP volume command.
Motivation 17:
This will also ensure that endless loop does not happen with
existing devices which do not comply with the
recommendation."
So there is nothing against the RD accepting the Volume Up/Down it
should just no send it back.
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH 1/6 v2] Bluetooth: Split UUIDs to three separate lists
From: Marcel Holtmann @ 2013-01-25 11:40 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1359097050-17552-2-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> The primary purpose of the UUIDs is to eable generation of EIR and AD
> data. In these data formats the UUIDs are split into separate fields
> based on whether they're 16, 32 or 128 bit UUIDs. To make the generation
> of these data fields simple this patch splits the UUIDs into three
> separate lists based on their type.
>
> To keep cases needing access to all UUIDs easy the UUIDs are also kept
> in an "all" list. This is particularly useful for exposing the UUIDs
> through sysfs as well as removing UUIDs (whether removing a specific one
> or all UUIDs).
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> v2: Removed enum bt_uuid_t since all it was needed for was selecting
> which list to add to.
>
> include/net/bluetooth/hci_core.h | 4 +++
> net/bluetooth/hci_core.c | 14 ++++----
> net/bluetooth/hci_sysfs.c | 2 +-
> net/bluetooth/mgmt.c | 67 +++++++++++++++++++-------------------
> 4 files changed, 45 insertions(+), 42 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index bcf8ffe..d3b1d1b 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -85,6 +85,7 @@ struct bdaddr_list {
>
> struct bt_uuid {
> struct list_head list;
> + struct list_head all;
> u8 uuid[16];
> u8 svc_hint;
> };
I am not 100% convinced we should be doing this actually. If we store
the UUIDs as 128-bit values anyway, why do we need three lists.
> @@ -256,6 +257,9 @@ struct hci_dev {
> struct list_head blacklist;
>
> struct list_head uuids;
> + struct list_head uuid16;
> + struct list_head uuid32;
> + struct list_head uuid128;
I would name them uuid16_list etc. if we stick with this.
And are we adding each entry twice? Or how does that work? I am getting
a bad feeling here.
> struct list_head link_keys;
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index e061b35..8818521 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1183,14 +1183,11 @@ static void hci_discov_off(struct work_struct *work)
>
> int hci_uuids_clear(struct hci_dev *hdev)
> {
> - struct list_head *p, *n;
> -
> - list_for_each_safe(p, n, &hdev->uuids) {
> - struct bt_uuid *uuid;
> + struct bt_uuid *uuid, *tmp;
>
> - uuid = list_entry(p, struct bt_uuid, list);
> -
> - list_del(p);
> + list_for_each_entry_safe(uuid, tmp, &hdev->uuids, all) {
> + list_del(&uuid->list);
> + list_del(&uuid->all);
> kfree(uuid);
> }
>
> @@ -1718,6 +1715,9 @@ struct hci_dev *hci_alloc_dev(void)
> INIT_LIST_HEAD(&hdev->mgmt_pending);
> INIT_LIST_HEAD(&hdev->blacklist);
> INIT_LIST_HEAD(&hdev->uuids);
> + INIT_LIST_HEAD(&hdev->uuid16);
> + INIT_LIST_HEAD(&hdev->uuid32);
> + INIT_LIST_HEAD(&hdev->uuid128);
> INIT_LIST_HEAD(&hdev->link_keys);
> INIT_LIST_HEAD(&hdev->long_term_keys);
> INIT_LIST_HEAD(&hdev->remote_oob_data);
> diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
> index 23b4e24..9a60c74 100644
> --- a/net/bluetooth/hci_sysfs.c
> +++ b/net/bluetooth/hci_sysfs.c
> @@ -483,7 +483,7 @@ static int uuids_show(struct seq_file *f, void *p)
>
> hci_dev_lock(hdev);
>
> - list_for_each_entry(uuid, &hdev->uuids, list)
> + list_for_each_entry(uuid, &hdev->uuids, all)
> print_bt_uuid(f, uuid->uuid);
This all thing makes is not so pretty code. You would expect list here.
>
> hci_dev_unlock(hdev);
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index e7f944f..1257ed0 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -435,28 +435,6 @@ static u32 get_current_settings(struct hci_dev *hdev)
>
> #define PNP_INFO_SVCLASS_ID 0x1200
>
> -static u8 bluetooth_base_uuid[] = {
> - 0xFB, 0x34, 0x9B, 0x5F, 0x80, 0x00, 0x00, 0x80,
> - 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> -};
> -
> -static u16 get_uuid16(u8 *uuid128)
> -{
> - u32 val;
> - int i;
> -
> - for (i = 0; i < 12; i++) {
> - if (bluetooth_base_uuid[i] != uuid128[i])
> - return 0;
> - }
> -
> - val = get_unaligned_le32(&uuid128[12]);
> - if (val > 0xffff)
> - return 0;
> -
> - return (u16) val;
> -}
> -
> static void create_eir(struct hci_dev *hdev, u8 *data)
> {
> u8 *ptr = data;
> @@ -510,13 +488,10 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
> memset(uuid16_list, 0, sizeof(uuid16_list));
>
> /* Group all UUID16 types */
> - list_for_each_entry(uuid, &hdev->uuids, list) {
> + list_for_each_entry(uuid, &hdev->uuid16, list) {
> u16 uuid16;
>
> - uuid16 = get_uuid16(uuid->uuid);
> - if (uuid16 == 0)
> - return;
> -
> + uuid16 = get_unaligned_le16(&uuid->uuid[12]);
> if (uuid16 < 0x1100)
> continue;
Why are we not storing the UUID size with it. So we just loop the same
list three times.
You have to keep the list of all UUIDs anyway. And we are not expecting
a bunch of hundred UUIDs. This list will be in the range of 10-20 max.
> @@ -592,7 +567,7 @@ static u8 get_service_classes(struct hci_dev *hdev)
> struct bt_uuid *uuid;
> u8 val = 0;
>
> - list_for_each_entry(uuid, &hdev->uuids, list)
> + list_for_each_entry(uuid, &hdev->uuids, all)
> val |= uuid->svc_hint;
>
> return val;
> @@ -1304,11 +1279,34 @@ unlock:
> return err;
> }
>
> +static u8 bluetooth_base_uuid[] = {
> + 0xfb, 0x34, 0x9b, 0x5f, 0x80, 0x00, 0x00, 0x80,
> + 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +};
> +
> +static struct list_head *select_uuid_list(struct hci_dev *hdev, u8 *uuid)
> +{
> + u32 val;
> + int i;
> +
> + for (i = 0; i < 12; i++) {
> + if (bluetooth_base_uuid[i] != uuid[i])
> + return &hdev->uuid128;
> + }
I know it is in the original code as well, but what is wrong with
memcmp.
> +
> + val = get_unaligned_le32(&uuid[12]);
> + if (val > 0xffff)
> + return &hdev->uuid32;
> +
> + return &hdev->uuid16;
> +}
> +
> static int add_uuid(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
> {
> struct mgmt_cp_add_uuid *cp = data;
> struct pending_cmd *cmd;
> struct bt_uuid *uuid;
> + struct list_head *l;
> int err;
>
> BT_DBG("request for %s", hdev->name);
> @@ -1330,7 +1328,10 @@ static int add_uuid(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
> memcpy(uuid->uuid, cp->uuid, 16);
> uuid->svc_hint = cp->svc_hint;
>
> - list_add(&uuid->list, &hdev->uuids);
> + l = select_uuid_list(hdev, cp->uuid);
> + list_add_tail(&uuid->list, l);
> +
> + list_add_tail(&uuid->all, &hdev->uuids);
>
> err = update_class(hdev);
> if (err < 0)
> @@ -1374,7 +1375,7 @@ static int remove_uuid(struct sock *sk, struct hci_dev *hdev, void *data,
> {
> struct mgmt_cp_remove_uuid *cp = data;
> struct pending_cmd *cmd;
> - struct list_head *p, *n;
> + struct bt_uuid *match, *tmp;
> u8 bt_uuid_any[] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
> int err, found;
>
> @@ -1402,13 +1403,11 @@ static int remove_uuid(struct sock *sk, struct hci_dev *hdev, void *data,
>
> found = 0;
>
> - list_for_each_safe(p, n, &hdev->uuids) {
> - struct bt_uuid *match = list_entry(p, struct bt_uuid, list);
> -
> + list_for_each_entry_safe(match, tmp, &hdev->uuids, all) {
> if (memcmp(match->uuid, cp->uuid, 16) != 0)
> continue;
> -
> list_del(&match->list);
> + list_del(&match->all);
> kfree(match);
> found++;
> }
Regards
Marcel
^ permalink raw reply
* Re: [RFC BlueZ 0/6] LE Connection Establishment fixes
From: Marcel Holtmann @ 2013-01-25 11:30 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <1359083227-13122-1-git-send-email-vinicius.gomes@openbossa.org>
Hi Vinicius,
> Sending this as a RFC because I don't know if this is what you guys
> had in mind.
>
> Patches 1/6/ and 2/6 are simple enough fixes and should be considered
> for inclusion, perhaps 6/6 as well, but the improvement is
> non-functional.
>
> The only problem with this aproach that I found while testing is the
> problem exposed on the message of commit 5/6: If the remote device is
> out of range during pairing any connection attempt will fail until the
> Connection Complete event comes (with an error). This is the reason
> that we put the devices in the connect list before pairing.
>
> Is this reason enough to have the device put in the connect list
> during pairing?
such a reasoning is pretty much wrong on all levels. An out of range
device might never come back in range and you end up with a device on
the connect list forever. And if it ever comes back, we end up with some
unexpected connection attempt to that device.
The only sane approach would have been to add a timeout and just abort
the pairing attempt after it. Everything else is just trying to fix a
symptom and making it actually worse while doing so.
Regards
Marcel
^ permalink raw reply
* Re: [RFC BlueZ 6/6] gas: Move all the code to only one file
From: Anderson Lizardo @ 2013-01-25 10:55 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <1359083227-13122-7-git-send-email-vinicius.gomes@openbossa.org>
Hi Vinicius,
On Thu, Jan 24, 2013 at 11:07 PM, Vinicius Costa Gomes
<vinicius.gomes@openbossa.org> wrote:
> +static int gatt_init(void)
> +{
> + btd_profile_register(&gatt_profile);
> +
> + return 0;
> +}
> +
> +static void gatt_exit(void)
> +{
> + btd_profile_register(&gatt_profile);
It should be btd_profile_unregister() here.
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [RFC BlueZ 0/6] LE Connection Establishment fixes
From: Johan Hedberg @ 2013-01-25 9:55 UTC (permalink / raw)
To: Vinicius Costa Gomes, linux-bluetooth
In-Reply-To: <20130125071215.GC17746@x220>
Hi Vinicius,
On Fri, Jan 25, 2013, Johan Hedberg wrote:
> On Fri, Jan 25, 2013, Vinicius Costa Gomes wrote:
> > The only problem with this aproach that I found while testing is the
> > problem exposed on the message of commit 5/6: If the remote device is
> > out of range during pairing any connection attempt will fail until the
> > Connection Complete event comes (with an error). This is the reason
> > that we put the devices in the connect list before pairing.
> >
> > Is this reason enough to have the device put in the connect list
> > during pairing?
>
> I don't think so. The mgmt_pair_device should have a timeout for LE
> devices on the kernel side, but since this is not the case with current
> kernel versions we'll need a workaround in user space. I.e. instead of
> using scanning (I assume that's what putting it on the connect list will
> do) you should just use a simple timeout (g_timeout_add_seconds) and
> call mgmt_cancel_pair_device if it expires.
I realized that you were mainly talking about the socket timeout and not
mgmt_pair_device timeout. The socket does have a proper kernel side
timeout while mgmt_pair_device doesn't (yet) so the latter needs
protecting (checking for connection status before deciding what to do is
racy so the protection needs to be there).
Anyway, I went ahead and fixed the pairing part and that's all I have
time for today. If there's still something needing fixing with the
reconnections feel free to send further patches.
Johan
^ permalink raw reply
* Re: [PATCH] tools: Fix format string warnings for g_dbus_create_error
From: Luiz Augusto von Dentz @ 2013-01-25 9:53 UTC (permalink / raw)
To: Chan-yeol Park; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1359106090-31381-1-git-send-email-chanyeol.park@samsung.com>
Hi Chan-yeol,
On Fri, Jan 25, 2013 at 11:28 AM, Chan-yeol Park
<chanyeol.park@samsung.com> wrote:
> This patch fixes gcc warnings for "format not a string literal and no
> format arguments".
> ---
> tools/mpris-player.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/mpris-player.c b/tools/mpris-player.c
> index 95ef8ab..aafa142 100644
> --- a/tools/mpris-player.c
> +++ b/tools/mpris-player.c
> @@ -830,7 +830,7 @@ static void player_reply(DBusMessage *message, void *user_data)
> dbus_error_init(&err);
> if (dbus_set_error_from_message(&err, message)) {
> fprintf(stderr, "error: %s", err.name);
> - reply = g_dbus_create_error(msg, err.name, err.message);
> + reply = g_dbus_create_error(msg, err.name, "%s", err.message);
> dbus_error_free(&err);
> } else
> reply = g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
> --
> 1.7.10.4
Applied, thanks.
--
Luiz Augusto von Dentz
^ permalink raw reply
* [PATCH] tools: Fix format string warnings for g_dbus_create_error
From: Chan-yeol Park @ 2013-01-25 9:28 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1359021484-6958-1-git-send-email-luiz.dentz@gmail.com>
This patch fixes gcc warnings for "format not a string literal and no
format arguments".
---
tools/mpris-player.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/mpris-player.c b/tools/mpris-player.c
index 95ef8ab..aafa142 100644
--- a/tools/mpris-player.c
+++ b/tools/mpris-player.c
@@ -830,7 +830,7 @@ static void player_reply(DBusMessage *message, void *user_data)
dbus_error_init(&err);
if (dbus_set_error_from_message(&err, message)) {
fprintf(stderr, "error: %s", err.name);
- reply = g_dbus_create_error(msg, err.name, err.message);
+ reply = g_dbus_create_error(msg, err.name, "%s", err.message);
dbus_error_free(&err);
} else
reply = g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] tools: Fix glib assert in mgmt-tester if hci_vhci module is not loaded
From: Johan Hedberg @ 2013-01-25 7:50 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1358799954-6140-1-git-send-email-szymon@janc.net.pl>
Hi Szymon,
On Mon, Jan 21, 2013, Szymon Janc wrote:
> Print descriptive warning on HCI emulation setup failure and fail test
> gracefully. This is better comparing to glib assertion when not fully
> initialized HCI emulation is being unreferenced.
> ---
> src/shared/hciemu.c | 31 ++++++++++++++++++++++---------
> tools/mgmt-tester.c | 4 ++++
> 2 files changed, 26 insertions(+), 9 deletions(-)
Applied. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH BlueZ 1/6 v2] tools: Make mpris-player to export players using MPRIS interface
From: Johan Hedberg @ 2013-01-25 7:42 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1359021484-6958-1-git-send-email-luiz.dentz@gmail.com>
Hi Luiz,
On Thu, Jan 24, 2013, Luiz Augusto von Dentz wrote:
> Players found are exported in private connections using
> org.mpris.MediaPlayer2.<device name> on the session bus.
> ---
> v2: Fix generating invalid player bus name if device name contains invalid
> chars.
>
> tools/mpris-player.c | 644 +++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 623 insertions(+), 21 deletions(-)
All patches in this set have been applied. Thanks.
Johan
^ permalink raw reply
* Re: [RFC BlueZ 6/6] gas: Move all the code to only one file
From: Johan Hedberg @ 2013-01-25 7:36 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <1359083227-13122-7-git-send-email-vinicius.gomes@openbossa.org>
Hi Vinicius,
On Fri, Jan 25, 2013, Vinicius Costa Gomes wrote:
> 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
You seem to have forgotten to remove gas.h in this patch. You remove it
from Makefile.plugins but it's still in the tree and gas.c includes it.
You'll also want to make gas_register/unregister static together with
removing the header file.
Johan
^ permalink raw reply
* Re: [RFC BlueZ 4/6] device: Rename device_att_connect to device_connect_le
From: Johan Hedberg @ 2013-01-25 7:32 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <1359083227-13122-5-git-send-email-vinicius.gomes@openbossa.org>
Hi Vinicius,
On Fri, Jan 25, 2013, Vinicius Costa Gomes wrote:
> This makes it clearer that this (currently unused) function should
> only be used with LE devices.
> ---
> src/device.c | 2 +-
> src/device.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
Patches 3 and 4 have been applied.
Johan
^ permalink raw reply
* Re: [RFC BlueZ 0/6] LE Connection Establishment fixes
From: Johan Hedberg @ 2013-01-25 7:12 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <1359083227-13122-1-git-send-email-vinicius.gomes@openbossa.org>
Hi Vinicius,
On Fri, Jan 25, 2013, Vinicius Costa Gomes wrote:
> The only problem with this aproach that I found while testing is the
> problem exposed on the message of commit 5/6: If the remote device is
> out of range during pairing any connection attempt will fail until the
> Connection Complete event comes (with an error). This is the reason
> that we put the devices in the connect list before pairing.
>
> Is this reason enough to have the device put in the connect list
> during pairing?
I don't think so. The mgmt_pair_device should have a timeout for LE
devices on the kernel side, but since this is not the case with current
kernel versions we'll need a workaround in user space. I.e. instead of
using scanning (I assume that's what putting it on the connect list will
do) you should just use a simple timeout (g_timeout_add_seconds) and
call mgmt_cancel_pair_device if it expires.
Johan
^ permalink raw reply
* Re: [RFC BlueZ 2/6] device: Fix memory leak while storing GATT capable devices
From: Johan Hedberg @ 2013-01-25 7:08 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <1359083227-13122-3-git-send-email-vinicius.gomes@openbossa.org>
Hi Vinicius,
On Fri, Jan 25, 2013, Vinicius Costa Gomes wrote:
> bt_uuid2string() returns a newly allocated string, and so it should
> be free'd.
> ---
> src/device.c | 3 +++
> 1 file changed, 3 insertions(+)
This patch has been applied. Thanks.
Johan
^ permalink raw reply
* Re: [RFC BlueZ 1/6] device: Fix invalid memory access during Find Included
From: Johan Hedberg @ 2013-01-25 7:07 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <1359083227-13122-2-git-send-email-vinicius.gomes@openbossa.org>
Hi Vinicius,
On Fri, Jan 25, 2013, Vinicius Costa Gomes wrote:
> 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)
> ---
> src/device.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/device.c b/src/device.c
> index bb79b38..893e4bb 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -2963,7 +2963,7 @@ static void find_included_cb(GSList *includes, uint8_t status,
> struct gatt_primary *prim;
> GSList *l;
>
> - if (includes == NULL)
> + if (includes == NULL || status != 0)
> goto done;
>
> for (l = includes; l; l = l->next) {
I think the bigger question is why is includes not NULL if status is not
zero? Does it contain invalid entries? Isn't that something that should
be fixed instead? Also, why isn't the code logging something in the case
of status != 0?
Johan
^ permalink raw reply
* [PATCH 6/6 v2] Bluetooth: Add support for 128-bit UUIDs in EIR data
From: Johan Hedberg @ 2013-01-25 6:57 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1359097050-17552-1-git-send-email-johan.hedberg@gmail.com>
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>
---
net/bluetooth/mgmt.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 48d81fc..b13e83e 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -504,6 +504,36 @@ static u8 *create_uuid32_list(struct hci_dev *hdev, u8 *data, ptrdiff_t len)
return ptr;
}
+static u8 *create_uuid128_list(struct hci_dev *hdev, u8 *data, ptrdiff_t len)
+{
+ u8 *ptr = data, *uuids_start = NULL;
+ struct bt_uuid *uuid;
+
+ if (len < 18)
+ return ptr;
+
+ list_for_each_entry(uuid, &hdev->uuid128, list) {
+ if (!uuids_start) {
+ uuids_start = ptr;
+ uuids_start[0] = 1;
+ uuids_start[1] = EIR_UUID128_ALL;
+ ptr += 2;
+ }
+
+ /* Stop if not enough space to put next UUID */
+ if ((ptr - data) + 16 > len) {
+ uuids_start[1] = EIR_UUID128_SOME;
+ break;
+ }
+
+ memcpy(ptr, uuid->uuid, 16);
+ ptr += 16;
+ uuids_start[0] += 16;
+ }
+
+ return ptr;
+}
+
static void create_eir(struct hci_dev *hdev, u8 *data)
{
u8 *ptr = data;
@@ -549,6 +579,7 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
ptr = create_uuid16_list(hdev, ptr, HCI_MAX_EIR_LENGTH - (ptr - data));
ptr = create_uuid32_list(hdev, ptr, HCI_MAX_EIR_LENGTH - (ptr - data));
+ ptr = create_uuid128_list(hdev, ptr, HCI_MAX_EIR_LENGTH - (ptr - data));
}
static int update_eir(struct hci_dev *hdev)
--
1.7.10.4
^ permalink raw reply related
* [PATCH 5/6 v2] Bluetooth: Add support for 32-bit UUIDs in EIR data
From: Johan Hedberg @ 2013-01-25 6:57 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1359097050-17552-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
This patch adds the necessary code for inserting a list of 32-bit UUIDs
into the EIR data.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/mgmt.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 3c8e69f..48d81fc 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -474,6 +474,36 @@ static u8 *create_uuid16_list(struct hci_dev *hdev, u8 *data, ptrdiff_t len)
return ptr;
}
+static u8 *create_uuid32_list(struct hci_dev *hdev, u8 *data, ptrdiff_t len)
+{
+ u8 *ptr = data, *uuids_start = NULL;
+ struct bt_uuid *uuid;
+
+ if (len < 6)
+ return ptr;
+
+ list_for_each_entry(uuid, &hdev->uuid32, list) {
+ if (!uuids_start) {
+ uuids_start = ptr;
+ uuids_start[0] = 1;
+ uuids_start[1] = EIR_UUID32_ALL;
+ ptr += 2;
+ }
+
+ /* Stop if not enough space to put next UUID */
+ if ((ptr - data) + sizeof(u32) > len) {
+ uuids_start[1] = EIR_UUID32_SOME;
+ break;
+ }
+
+ memcpy(ptr, &uuid->uuid[12], sizeof(u32));
+ ptr += sizeof(u32);
+ uuids_start[0] += sizeof(u32);
+ }
+
+ return ptr;
+}
+
static void create_eir(struct hci_dev *hdev, u8 *data)
{
u8 *ptr = data;
@@ -518,6 +548,7 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
}
ptr = create_uuid16_list(hdev, ptr, HCI_MAX_EIR_LENGTH - (ptr - data));
+ ptr = create_uuid32_list(hdev, ptr, HCI_MAX_EIR_LENGTH - (ptr - data));
}
static int update_eir(struct hci_dev *hdev)
--
1.7.10.4
^ permalink raw reply related
* [PATCH 4/6 v2] Bluetooth: Refactor UUID-16 list generation into its own function
From: Johan Hedberg @ 2013-01-25 6:57 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1359097050-17552-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
We will need to create three separate UUID lists in the EIR data (for
16, 32 and 128 bit UUIDs) so the code is easier to follow if each list
is generated in their own function.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/mgmt.c | 72 ++++++++++++++++++++++++++++----------------------
1 file changed, 40 insertions(+), 32 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index abf941d..3c8e69f 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -435,11 +435,48 @@ static u32 get_current_settings(struct hci_dev *hdev)
#define PNP_INFO_SVCLASS_ID 0x1200
+static u8 *create_uuid16_list(struct hci_dev *hdev, u8 *data, ptrdiff_t len)
+{
+ u8 *ptr = data, *uuids_start = NULL;
+ struct bt_uuid *uuid;
+
+ if (len < 4)
+ return ptr;
+
+ list_for_each_entry(uuid, &hdev->uuid16, list) {
+ u16 uuid16;
+
+ uuid16 = get_unaligned_le16(&uuid->uuid[12]);
+ if (uuid16 < 0x1100)
+ continue;
+
+ if (uuid16 == PNP_INFO_SVCLASS_ID)
+ continue;
+
+ if (!uuids_start) {
+ uuids_start = ptr;
+ uuids_start[0] = 1;
+ uuids_start[1] = EIR_UUID16_ALL;
+ ptr += 2;
+ }
+
+ /* Stop if not enough space to put next UUID */
+ if ((ptr - data) + sizeof(u16) > len) {
+ uuids_start[1] = EIR_UUID16_SOME;
+ break;
+ }
+
+ *ptr++ = (uuid16 & 0x00ff);
+ *ptr++ = (uuid16 & 0xff00) >> 8;
+ uuids_start[0] += sizeof(uuid16);
+ }
+
+ return ptr;
+}
+
static void create_eir(struct hci_dev *hdev, u8 *data)
{
u8 *ptr = data;
- u8 *uuids_start;
- struct bt_uuid *uuid;
size_t name_len;
name_len = strlen(hdev->dev_name);
@@ -480,36 +517,7 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
ptr += 10;
}
- uuids_start = NULL;
-
- /* Group all UUID16 types */
- list_for_each_entry(uuid, &hdev->uuid16, list) {
- u16 uuid16;
-
- uuid16 = get_unaligned_le16(&uuid->uuid[12]);
- if (uuid16 < 0x1100)
- continue;
-
- if (uuid16 == PNP_INFO_SVCLASS_ID)
- continue;
-
- if (!uuids_start) {
- uuids_start = ptr;
- uuids_start[0] = 1;
- uuids_start[1] = EIR_UUID16_ALL;
- ptr += 2;
- }
-
- /* Stop if not enough space to put next UUID */
- if ((ptr - data) + 2 + sizeof(u16) > HCI_MAX_EIR_LENGTH) {
- uuids_start[1] = EIR_UUID16_SOME;
- break;
- }
-
- *ptr++ = (uuid16 & 0x00ff);
- *ptr++ = (uuid16 & 0xff00) >> 8;
- uuids_start[0] += sizeof(uuid16);
- }
+ ptr = create_uuid16_list(hdev, ptr, HCI_MAX_EIR_LENGTH - (ptr - data));
}
static int update_eir(struct hci_dev *hdev)
--
1.7.10.4
^ permalink raw reply related
* [PATCH 3/6 v2] Bluetooth: Remove useless eir_len variable from EIR creation
From: Johan Hedberg @ 2013-01-25 6:57 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1359097050-17552-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
The amount of data encoded so far in the create_eir() function can be
calculated simply through the difference between the data and ptr
pointer variables. The eir_len variable then becomes essentially
useless.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/mgmt.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index b3a1397..abf941d 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -439,7 +439,6 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
{
u8 *ptr = data;
u8 *uuids_start;
- u16 eir_len = 0;
struct bt_uuid *uuid;
size_t name_len;
@@ -458,7 +457,6 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
memcpy(ptr + 2, hdev->dev_name, name_len);
- eir_len += (name_len + 2);
ptr += (name_len + 2);
}
@@ -467,7 +465,6 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
ptr[1] = EIR_TX_POWER;
ptr[2] = (u8) hdev->inq_tx_power;
- eir_len += 3;
ptr += 3;
}
@@ -480,7 +477,6 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
put_unaligned_le16(hdev->devid_product, ptr + 6);
put_unaligned_le16(hdev->devid_version, ptr + 8);
- eir_len += 10;
ptr += 10;
}
@@ -502,18 +498,16 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
uuids_start[0] = 1;
uuids_start[1] = EIR_UUID16_ALL;
ptr += 2;
- eir_len += 2;
}
/* Stop if not enough space to put next UUID */
- if (eir_len + 2 + sizeof(u16) > HCI_MAX_EIR_LENGTH) {
+ if ((ptr - data) + 2 + sizeof(u16) > HCI_MAX_EIR_LENGTH) {
uuids_start[1] = EIR_UUID16_SOME;
break;
}
*ptr++ = (uuid16 & 0x00ff);
*ptr++ = (uuid16 & 0xff00) >> 8;
- eir_len += sizeof(uuid16);
uuids_start[0] += sizeof(uuid16);
}
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 2/6 v2] Bluetooth: Simplify UUID16 list generation for EIR
From: Johan Hedberg @ 2013-01-25 6:57 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1359097050-17552-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
There's no need to use two separate loops to generate a UUID list for
the EIR data. This patch merges the two loops previously used for the
16-bit UUID list generation into a single loop, thus simplifying the
code a great deal.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/mgmt.c | 46 +++++++++++++++-------------------------------
1 file changed, 15 insertions(+), 31 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 1257ed0..b3a1397 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -438,9 +438,8 @@ static u32 get_current_settings(struct hci_dev *hdev)
static void create_eir(struct hci_dev *hdev, u8 *data)
{
u8 *ptr = data;
+ u8 *uuids_start;
u16 eir_len = 0;
- u16 uuid16_list[HCI_MAX_EIR_LENGTH / sizeof(u16)];
- int i, truncated = 0;
struct bt_uuid *uuid;
size_t name_len;
@@ -485,7 +484,7 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
ptr += 10;
}
- memset(uuid16_list, 0, sizeof(uuid16_list));
+ uuids_start = NULL;
/* Group all UUID16 types */
list_for_each_entry(uuid, &hdev->uuid16, list) {
@@ -498,39 +497,24 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
if (uuid16 == PNP_INFO_SVCLASS_ID)
continue;
+ if (!uuids_start) {
+ uuids_start = ptr;
+ uuids_start[0] = 1;
+ uuids_start[1] = EIR_UUID16_ALL;
+ ptr += 2;
+ eir_len += 2;
+ }
+
/* Stop if not enough space to put next UUID */
if (eir_len + 2 + sizeof(u16) > HCI_MAX_EIR_LENGTH) {
- truncated = 1;
+ uuids_start[1] = EIR_UUID16_SOME;
break;
}
- /* Check for duplicates */
- for (i = 0; uuid16_list[i] != 0; i++)
- if (uuid16_list[i] == uuid16)
- break;
-
- if (uuid16_list[i] == 0) {
- uuid16_list[i] = uuid16;
- eir_len += sizeof(u16);
- }
- }
-
- if (uuid16_list[0] != 0) {
- u8 *length = ptr;
-
- /* EIR Data type */
- ptr[1] = truncated ? EIR_UUID16_SOME : EIR_UUID16_ALL;
-
- ptr += 2;
- eir_len += 2;
-
- for (i = 0; uuid16_list[i] != 0; i++) {
- *ptr++ = (uuid16_list[i] & 0x00ff);
- *ptr++ = (uuid16_list[i] & 0xff00) >> 8;
- }
-
- /* EIR Data length */
- *length = (i * sizeof(u16)) + 1;
+ *ptr++ = (uuid16 & 0x00ff);
+ *ptr++ = (uuid16 & 0xff00) >> 8;
+ eir_len += sizeof(uuid16);
+ uuids_start[0] += sizeof(uuid16);
}
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 1/6 v2] Bluetooth: Split UUIDs to three separate lists
From: Johan Hedberg @ 2013-01-25 6:57 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1359097050-17552-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
The primary purpose of the UUIDs is to eable generation of EIR and AD
data. In these data formats the UUIDs are split into separate fields
based on whether they're 16, 32 or 128 bit UUIDs. To make the generation
of these data fields simple this patch splits the UUIDs into three
separate lists based on their type.
To keep cases needing access to all UUIDs easy the UUIDs are also kept
in an "all" list. This is particularly useful for exposing the UUIDs
through sysfs as well as removing UUIDs (whether removing a specific one
or all UUIDs).
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
v2: Removed enum bt_uuid_t since all it was needed for was selecting
which list to add to.
include/net/bluetooth/hci_core.h | 4 +++
net/bluetooth/hci_core.c | 14 ++++----
net/bluetooth/hci_sysfs.c | 2 +-
net/bluetooth/mgmt.c | 67 +++++++++++++++++++-------------------
4 files changed, 45 insertions(+), 42 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index bcf8ffe..d3b1d1b 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -85,6 +85,7 @@ struct bdaddr_list {
struct bt_uuid {
struct list_head list;
+ struct list_head all;
u8 uuid[16];
u8 svc_hint;
};
@@ -256,6 +257,9 @@ struct hci_dev {
struct list_head blacklist;
struct list_head uuids;
+ struct list_head uuid16;
+ struct list_head uuid32;
+ struct list_head uuid128;
struct list_head link_keys;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index e061b35..8818521 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1183,14 +1183,11 @@ static void hci_discov_off(struct work_struct *work)
int hci_uuids_clear(struct hci_dev *hdev)
{
- struct list_head *p, *n;
-
- list_for_each_safe(p, n, &hdev->uuids) {
- struct bt_uuid *uuid;
+ struct bt_uuid *uuid, *tmp;
- uuid = list_entry(p, struct bt_uuid, list);
-
- list_del(p);
+ list_for_each_entry_safe(uuid, tmp, &hdev->uuids, all) {
+ list_del(&uuid->list);
+ list_del(&uuid->all);
kfree(uuid);
}
@@ -1718,6 +1715,9 @@ struct hci_dev *hci_alloc_dev(void)
INIT_LIST_HEAD(&hdev->mgmt_pending);
INIT_LIST_HEAD(&hdev->blacklist);
INIT_LIST_HEAD(&hdev->uuids);
+ INIT_LIST_HEAD(&hdev->uuid16);
+ INIT_LIST_HEAD(&hdev->uuid32);
+ INIT_LIST_HEAD(&hdev->uuid128);
INIT_LIST_HEAD(&hdev->link_keys);
INIT_LIST_HEAD(&hdev->long_term_keys);
INIT_LIST_HEAD(&hdev->remote_oob_data);
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index 23b4e24..9a60c74 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -483,7 +483,7 @@ static int uuids_show(struct seq_file *f, void *p)
hci_dev_lock(hdev);
- list_for_each_entry(uuid, &hdev->uuids, list)
+ list_for_each_entry(uuid, &hdev->uuids, all)
print_bt_uuid(f, uuid->uuid);
hci_dev_unlock(hdev);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index e7f944f..1257ed0 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -435,28 +435,6 @@ static u32 get_current_settings(struct hci_dev *hdev)
#define PNP_INFO_SVCLASS_ID 0x1200
-static u8 bluetooth_base_uuid[] = {
- 0xFB, 0x34, 0x9B, 0x5F, 0x80, 0x00, 0x00, 0x80,
- 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-};
-
-static u16 get_uuid16(u8 *uuid128)
-{
- u32 val;
- int i;
-
- for (i = 0; i < 12; i++) {
- if (bluetooth_base_uuid[i] != uuid128[i])
- return 0;
- }
-
- val = get_unaligned_le32(&uuid128[12]);
- if (val > 0xffff)
- return 0;
-
- return (u16) val;
-}
-
static void create_eir(struct hci_dev *hdev, u8 *data)
{
u8 *ptr = data;
@@ -510,13 +488,10 @@ static void create_eir(struct hci_dev *hdev, u8 *data)
memset(uuid16_list, 0, sizeof(uuid16_list));
/* Group all UUID16 types */
- list_for_each_entry(uuid, &hdev->uuids, list) {
+ list_for_each_entry(uuid, &hdev->uuid16, list) {
u16 uuid16;
- uuid16 = get_uuid16(uuid->uuid);
- if (uuid16 == 0)
- return;
-
+ uuid16 = get_unaligned_le16(&uuid->uuid[12]);
if (uuid16 < 0x1100)
continue;
@@ -592,7 +567,7 @@ static u8 get_service_classes(struct hci_dev *hdev)
struct bt_uuid *uuid;
u8 val = 0;
- list_for_each_entry(uuid, &hdev->uuids, list)
+ list_for_each_entry(uuid, &hdev->uuids, all)
val |= uuid->svc_hint;
return val;
@@ -1304,11 +1279,34 @@ unlock:
return err;
}
+static u8 bluetooth_base_uuid[] = {
+ 0xfb, 0x34, 0x9b, 0x5f, 0x80, 0x00, 0x00, 0x80,
+ 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+};
+
+static struct list_head *select_uuid_list(struct hci_dev *hdev, u8 *uuid)
+{
+ u32 val;
+ int i;
+
+ for (i = 0; i < 12; i++) {
+ if (bluetooth_base_uuid[i] != uuid[i])
+ return &hdev->uuid128;
+ }
+
+ val = get_unaligned_le32(&uuid[12]);
+ if (val > 0xffff)
+ return &hdev->uuid32;
+
+ return &hdev->uuid16;
+}
+
static int add_uuid(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
{
struct mgmt_cp_add_uuid *cp = data;
struct pending_cmd *cmd;
struct bt_uuid *uuid;
+ struct list_head *l;
int err;
BT_DBG("request for %s", hdev->name);
@@ -1330,7 +1328,10 @@ static int add_uuid(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
memcpy(uuid->uuid, cp->uuid, 16);
uuid->svc_hint = cp->svc_hint;
- list_add(&uuid->list, &hdev->uuids);
+ l = select_uuid_list(hdev, cp->uuid);
+ list_add_tail(&uuid->list, l);
+
+ list_add_tail(&uuid->all, &hdev->uuids);
err = update_class(hdev);
if (err < 0)
@@ -1374,7 +1375,7 @@ static int remove_uuid(struct sock *sk, struct hci_dev *hdev, void *data,
{
struct mgmt_cp_remove_uuid *cp = data;
struct pending_cmd *cmd;
- struct list_head *p, *n;
+ struct bt_uuid *match, *tmp;
u8 bt_uuid_any[] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
int err, found;
@@ -1402,13 +1403,11 @@ static int remove_uuid(struct sock *sk, struct hci_dev *hdev, void *data,
found = 0;
- list_for_each_safe(p, n, &hdev->uuids) {
- struct bt_uuid *match = list_entry(p, struct bt_uuid, list);
-
+ list_for_each_entry_safe(match, tmp, &hdev->uuids, all) {
if (memcmp(match->uuid, cp->uuid, 16) != 0)
continue;
-
list_del(&match->list);
+ list_del(&match->all);
kfree(match);
found++;
}
--
1.7.10.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox