* Re: [PATCH] Fix crash while reading from mapped file
From: Luiz Augusto von Dentz @ 2010-12-17 13:25 UTC (permalink / raw)
To: Anderson Lizardo
Cc: Lukasz Pawlik, Bastien Nocera, Johan Hedberg, linux-bluetooth
In-Reply-To: <AANLkTinHyH6i_pq-77_SYEkyf3jfwbfqL_WXav6nB2tc@mail.gmail.com>
Hi,
On Fri, Dec 17, 2010 at 11:29 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi,
>
> On Thu, Dec 16, 2010 at 11:28 AM, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
>> Hi,
>>
>> On Wed, Dec 15, 2010 at 8:19 PM, Anderson Lizardo
>> <anderson.lizardo@openbossa.org> wrote:
>>> On Tue, Dec 14, 2010 at 2:20 PM, Lukasz Pawlik <lucas.pawlik@gmail.com> wrote:
>>>> Hi,
>>>>
>>>>> If somebody can explain what that code is supposed to do, then writing a
>>>>> glib-ish version should be trivial, without having to duplicate the
>>>>> contents again. Then again, using something like:
>>>>> g_mapped_file_new ();
>>>>> g_strsplit ();
>>>>> g_mapped_file_unref ();
>>>>
>>>> That won't fix anything since g_mapped_file_new uses mmap function so
>>>> contents may not be zero-terminated and we want use string function
>>>> next.
>>>
>>> What about using g_strstr_len() instead of strpbrk() on the original
>>> code? See http://library.gnome.org/devel/glib/unstable/glib-String-Utility-Functions.html#g-strstr-len
>>
>> That looks to be a good replacement for strpbrk, probably 1 line patch.
>
> I guess we cannot use any of glib functions here since textfile.c is
> also used in some tools which doesn't link with glib, so if we don't
> want to add this dependency to the than we need some other way to fix
> it.
>
> What about this:
>
> diff --git a/src/textfile.c b/src/textfile.c
> index 2429cc7..2e4c642 100644
> --- a/src/textfile.c
> +++ b/src/textfile.c
> @@ -394,7 +394,7 @@ int textfile_foreach(const char *pathname,
> textfile_cb func, void *data)
> goto unlock;
> }
>
> - size = st.st_size;
> + size = st.st_size + 1;
>
> map = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, 0);
> if (!map || map == MAP_FAILED) {
>
> It will probably use 1 more page if the file size is multiple of the
> page size but it seems correct if you compare to e.g. malloc, well if
> the possibility of the extra page is too much than we need our own
> version of g_strstr_len/strpbrk_len like the following:
>
> http://www.google.com/codesearch/p?hl=en#cZwlSNS7aEw/external/bluetooth/glib/glib/gstrfuncs.c&q=g_strstr_len&d=4
The patch doesn't really work, it cause Non-existent physical address
error, anyway I have sent a patch that introduces strpbrk_len, but I
will probably be changing to strnpbrk to conform with some other strn*
variants.
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* [PATCH v2] Fix crash when mmaping files which size is multiple of page size
From: Luiz Augusto von Dentz @ 2010-12-17 13:31 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
In this case the buffer returned by mmap is not NULL terminated so
functions like strpbrk that expect a string goes out of bounds.
To fix this strpbrk_len was introduced which takes the size of the buffer
making sure it never goes out of bounds.
---
src/textfile.c | 38 +++++++++++++++++++++++++++++++++-----
1 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/src/textfile.c b/src/textfile.c
index 2429cc7..d115ff6 100644
--- a/src/textfile.c
+++ b/src/textfile.c
@@ -156,6 +156,28 @@ static inline int write_key_value(int fd, const char *key, const char *value)
return err;
}
+static char *strnpbrk(const char *s, ssize_t len, const char *accept)
+{
+ const char *p = s;
+ const char *end;
+
+ end = s + len - 1;
+
+ while (p <= end && *p) {
+ const char *a = accept;
+
+ while (*a) {
+ if (*p == *a)
+ return (char *) p;
+ a++;
+ }
+
+ p++;
+ }
+
+ return NULL;
+}
+
static int write_key(const char *pathname, const char *key, const char *value, int icase)
{
struct stat st;
@@ -207,7 +229,7 @@ static int write_key(const char *pathname, const char *key, const char *value, i
base = off - map;
- end = strpbrk(off, "\r\n");
+ end = strnpbrk(off, size, "\r\n");
if (!end) {
err = EILSEQ;
goto unmap;
@@ -315,7 +337,7 @@ static char *read_key(const char *pathname, const char *key, int icase)
goto unmap;
}
- end = strpbrk(off, "\r\n");
+ end = strnpbrk(off, size - (map - off), "\r\n");
if (!end) {
err = EILSEQ;
goto unmap;
@@ -404,8 +426,8 @@ int textfile_foreach(const char *pathname, textfile_cb func, void *data)
off = map;
- while (1) {
- end = strpbrk(off, " ");
+ while (size - (off - map) > 0) {
+ end = strnpbrk(off, size - (off - map), " ");
if (!end) {
err = EILSEQ;
break;
@@ -424,7 +446,13 @@ int textfile_foreach(const char *pathname, textfile_cb func, void *data)
off = end + 1;
- end = strpbrk(off, "\r\n");
+ if (size - (off - map) < 0) {
+ err = EILSEQ;
+ free(key);
+ break;
+ }
+
+ end = strnpbrk(off, size - (off - map), "\r\n");
if (!end) {
err = EILSEQ;
free(key);
--
1.7.1
^ permalink raw reply related
* Re: [PATCH v2] Fix crash when mmaping files which size is multiple of page size
From: Johan Hedberg @ 2010-12-17 13:53 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1292592686-14908-1-git-send-email-luiz.dentz@gmail.com>
Hi Luiz,
On Fri, Dec 17, 2010, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
>
> In this case the buffer returned by mmap is not NULL terminated so
> functions like strpbrk that expect a string goes out of bounds.
>
> To fix this strpbrk_len was introduced which takes the size of the buffer
> making sure it never goes out of bounds.
> ---
> src/textfile.c | 38 +++++++++++++++++++++++++++++++++-----
> 1 files changed, 33 insertions(+), 5 deletions(-)
Thanks. The patch looks good and seems to compile and run fine too. It
has been pushed upstream.
Johan
^ permalink raw reply
* [PATCH 1/4] Move get_eir_uuids() from src/adapter.c to src/event.c
From: Bruna Moreira @ 2010-12-17 14:57 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
From: Anderson Lizardo <anderson.lizardo@openbossa.org>
Moving get_eir_uuids() to src/event.c removes the need to pass the raw
EIR data to higher layers. Now it is not necessary to pass the original
GSList of service UUIDs, because the list is concatenated (with
verification of duplicate entries) on adapter_update_device_from_info()
(for LE) and adapter_update_found_devices() (for BR/EDR).
---
src/adapter.c | 145 +++++++++++---------------------------------------------
src/adapter.h | 7 +--
src/event.c | 114 +++++++++++++++++++++++++++++++++++++++++++--
3 files changed, 142 insertions(+), 124 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 5118306..34bf24a 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2696,117 +2696,8 @@ static char **strlist2array(GSList *list)
return array;
}
-static GSList *get_eir_uuids(uint8_t *eir_data, size_t eir_length, GSList *list)
-{
- uint16_t len = 0;
- size_t total;
- size_t uuid16_count = 0;
- size_t uuid32_count = 0;
- size_t uuid128_count = 0;
- uint8_t *uuid16;
- uint8_t *uuid32;
- uint8_t *uuid128;
- uuid_t service;
- char *uuid_str;
- unsigned int i;
-
- if (eir_data == NULL || eir_length == 0)
- return list;
-
- while (len < eir_length - 1) {
- uint8_t field_len = eir_data[0];
-
- /* Check for the end of EIR */
- if (field_len == 0)
- break;
-
- switch (eir_data[1]) {
- case EIR_UUID16_SOME:
- case EIR_UUID16_ALL:
- uuid16_count = field_len / 2;
- uuid16 = &eir_data[2];
- break;
- case EIR_UUID32_SOME:
- case EIR_UUID32_ALL:
- uuid32_count = field_len / 4;
- uuid32 = &eir_data[2];
- break;
- case EIR_UUID128_SOME:
- case EIR_UUID128_ALL:
- uuid128_count = field_len / 16;
- uuid128 = &eir_data[2];
- break;
- }
-
- len += field_len + 1;
- eir_data += field_len + 1;
- }
-
- /* Bail out if got incorrect length */
- if (len > eir_length)
- return list;
-
- total = uuid16_count + uuid32_count + uuid128_count;
-
- if (!total)
- return list;
-
- /* Generate uuids in SDP format (EIR data is Little Endian) */
- service.type = SDP_UUID16;
- for (i = 0; i < uuid16_count; i++) {
- uint16_t val16 = uuid16[1];
-
- val16 = (val16 << 8) + uuid16[0];
- service.value.uuid16 = val16;
- uuid_str = bt_uuid2string(&service);
- if (g_slist_find_custom(list, uuid_str,
- (GCompareFunc) strcmp) == NULL)
- list = g_slist_append(list, uuid_str);
- else
- g_free(uuid_str);
- uuid16 += 2;
- }
-
- service.type = SDP_UUID32;
- for (i = uuid16_count; i < uuid32_count + uuid16_count; i++) {
- uint32_t val32 = uuid32[3];
- int k;
-
- for (k = 2; k >= 0; k--)
- val32 = (val32 << 8) + uuid32[k];
-
- service.value.uuid32 = val32;
- uuid_str = bt_uuid2string(&service);
- if (g_slist_find_custom(list, uuid_str,
- (GCompareFunc) strcmp) == NULL)
- list = g_slist_append(list, uuid_str);
- else
- g_free(uuid_str);
- uuid32 += 4;
- }
-
- service.type = SDP_UUID128;
- for (i = uuid32_count + uuid16_count; i < total; i++) {
- int k;
-
- for (k = 0; k < 16; k++)
- service.value.uuid128.data[k] = uuid128[16 - k - 1];
-
- uuid_str = bt_uuid2string(&service);
- if (g_slist_find_custom(list, uuid_str,
- (GCompareFunc) strcmp) == NULL)
- list = g_slist_append(list, uuid_str);
- else
- g_free(uuid_str);
- uuid128 += 16;
- }
-
- return list;
-}
-
void adapter_emit_device_found(struct btd_adapter *adapter,
- struct remote_dev_info *dev,
- uint8_t *eir_data, size_t eir_length)
+ struct remote_dev_info *dev)
{
struct btd_device *device;
char peer_addr[18], local_addr[18];
@@ -2823,8 +2714,7 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
if (device)
paired = device_is_paired(device);
- /* Extract UUIDs from extended inquiry response if any */
- dev->services = get_eir_uuids(eir_data, eir_length, dev->services);
+ /* The uuids string array is updated only if necessary */
uuid_count = g_slist_length(dev->services);
if (dev->services && dev->uuid_count != uuid_count) {
g_strfreev(dev->uuids);
@@ -2911,8 +2801,25 @@ static gboolean extract_eir_flags(uint8_t *flags, uint8_t *eir_data)
return TRUE;
}
+static void remove_same_uuid(gpointer data, gpointer user_data)
+{
+ struct remote_dev_info *dev = user_data;
+ GSList *l;
+
+ for (l = dev->services; l; l = l->next) {
+ char *current_uuid = l->data;
+ char *new_uuid = data;
+
+ if (strcmp(current_uuid, new_uuid) == 0) {
+ g_free(current_uuid);
+ dev->services = g_slist_delete_link(dev->services, l);
+ break;
+ }
+ }
+}
+
void adapter_update_device_from_info(struct btd_adapter *adapter,
- le_advertising_info *info)
+ le_advertising_info *info, GSList *services)
{
struct remote_dev_info *dev;
bdaddr_t bdaddr;
@@ -2935,6 +2842,9 @@ void adapter_update_device_from_info(struct btd_adapter *adapter,
adapter->found_devices = g_slist_sort(adapter->found_devices,
(GCompareFunc) dev_rssi_cmp);
+ g_slist_foreach(services, remove_same_uuid, dev);
+ dev->services = g_slist_concat(dev->services, services);
+
if (info->length) {
char *tmp_name = bt_extract_eir_name(info->data, NULL);
if (tmp_name) {
@@ -2947,13 +2857,13 @@ void adapter_update_device_from_info(struct btd_adapter *adapter,
/* FIXME: check if other information was changed before emitting the
* signal */
- adapter_emit_device_found(adapter, dev, info->data, info->length);
+ adapter_emit_device_found(adapter, dev);
}
void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
int8_t rssi, uint32_t class, const char *name,
const char *alias, gboolean legacy,
- name_status_t name_status, uint8_t *eir_data)
+ GSList *services, name_status_t name_status)
{
struct remote_dev_info *dev;
gboolean new_dev;
@@ -2979,7 +2889,10 @@ void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
adapter->found_devices = g_slist_sort(adapter->found_devices,
(GCompareFunc) dev_rssi_cmp);
- adapter_emit_device_found(adapter, dev, eir_data, EIR_DATA_LENGTH);
+ g_slist_foreach(services, remove_same_uuid, dev);
+ dev->services = g_slist_concat(dev->services, services);
+
+ adapter_emit_device_found(adapter, dev);
}
int adapter_remove_found_device(struct btd_adapter *adapter, bdaddr_t *bdaddr)
diff --git a/src/adapter.h b/src/adapter.h
index a02f61c..efcf5b8 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -124,15 +124,14 @@ gboolean adapter_is_ready(struct btd_adapter *adapter);
struct remote_dev_info *adapter_search_found_devices(struct btd_adapter *adapter,
struct remote_dev_info *match);
void adapter_update_device_from_info(struct btd_adapter *adapter,
- le_advertising_info *info);
+ le_advertising_info *info, GSList *services);
void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
int8_t rssi, uint32_t class, const char *name,
const char *alias, gboolean legacy,
- name_status_t name_status, uint8_t *eir_data);
+ GSList *services, name_status_t name_status);
int adapter_remove_found_device(struct btd_adapter *adapter, bdaddr_t *bdaddr);
void adapter_emit_device_found(struct btd_adapter *adapter,
- struct remote_dev_info *dev,
- uint8_t *eir_data, size_t eir_length);
+ struct remote_dev_info *dev);
void adapter_mode_changed(struct btd_adapter *adapter, uint8_t scan_mode);
void adapter_update_local_name(struct btd_adapter *adapter, const char *name);
void adapter_service_insert(const bdaddr_t *bdaddr, void *rec);
diff --git a/src/event.c b/src/event.c
index 47b50c4..0e6c86f 100644
--- a/src/event.c
+++ b/src/event.c
@@ -56,6 +56,7 @@
#include "agent.h"
#include "storage.h"
#include "event.h"
+#include "sdpd.h"
static gboolean get_adapter_and_device(bdaddr_t *src, bdaddr_t *dst,
struct btd_adapter **adapter,
@@ -299,9 +300,107 @@ void btd_event_simple_pairing_complete(bdaddr_t *local, bdaddr_t *peer,
device_simple_pairing_complete(device, status);
}
+static GSList *get_eir_uuids(uint8_t *eir_data, size_t eir_length)
+{
+ uint16_t len = 0;
+ size_t total;
+ size_t uuid16_count = 0;
+ size_t uuid32_count = 0;
+ size_t uuid128_count = 0;
+ GSList *list = NULL;
+ uint8_t *uuid16 = NULL;
+ uint8_t *uuid32 = NULL;
+ uint8_t *uuid128 = NULL;
+ uuid_t service;
+ char *uuid_str;
+ unsigned int i;
+
+ if (eir_data == NULL || eir_length == 0)
+ return list;
+
+ while (len < eir_length - 1) {
+ uint8_t field_len = eir_data[0];
+
+ /* Check for the end of EIR */
+ if (field_len == 0)
+ break;
+
+ switch (eir_data[1]) {
+ case EIR_UUID16_SOME:
+ case EIR_UUID16_ALL:
+ uuid16_count = field_len / 2;
+ uuid16 = &eir_data[2];
+ break;
+ case EIR_UUID32_SOME:
+ case EIR_UUID32_ALL:
+ uuid32_count = field_len / 4;
+ uuid32 = &eir_data[2];
+ break;
+ case EIR_UUID128_SOME:
+ case EIR_UUID128_ALL:
+ uuid128_count = field_len / 16;
+ uuid128 = &eir_data[2];
+ break;
+ }
+
+ len += field_len + 1;
+ eir_data += field_len + 1;
+ }
+
+ /* Bail out if got incorrect length */
+ if (len > eir_length)
+ return list;
+
+ total = uuid16_count + uuid32_count + uuid128_count;
+
+ if (!total)
+ return list;
+
+ /* Generate uuids in SDP format (EIR data is Little Endian) */
+ service.type = SDP_UUID16;
+ for (i = 0; i < uuid16_count; i++) {
+ uint16_t val16 = uuid16[1];
+
+ val16 = (val16 << 8) + uuid16[0];
+ service.value.uuid16 = val16;
+ uuid_str = bt_uuid2string(&service);
+ list = g_slist_append(list, uuid_str);
+ uuid16 += 2;
+ }
+
+ service.type = SDP_UUID32;
+ for (i = uuid16_count; i < uuid32_count + uuid16_count; i++) {
+ uint32_t val32 = uuid32[3];
+ int k;
+
+ for (k = 2; k >= 0; k--)
+ val32 = (val32 << 8) + uuid32[k];
+
+ service.value.uuid32 = val32;
+ uuid_str = bt_uuid2string(&service);
+ list = g_slist_append(list, uuid_str);
+ uuid32 += 4;
+ }
+
+ service.type = SDP_UUID128;
+ for (i = uuid32_count + uuid16_count; i < total; i++) {
+ int k;
+
+ for (k = 0; k < 16; k++)
+ service.value.uuid128.data[k] = uuid128[16 - k - 1];
+
+ uuid_str = bt_uuid2string(&service);
+ list = g_slist_append(list, uuid_str);
+ uuid128 += 16;
+ }
+
+ return list;
+}
+
void btd_event_advertising_report(bdaddr_t *local, le_advertising_info *info)
{
struct btd_adapter *adapter;
+ GSList *services = NULL;
adapter = manager_find_adapter(local);
if (adapter == NULL) {
@@ -309,7 +408,10 @@ void btd_event_advertising_report(bdaddr_t *local, le_advertising_info *info)
return;
}
- adapter_update_device_from_info(adapter, info);
+ /* Extract UUIDs from advertising data if any */
+ services = get_eir_uuids(info->data, info->length);
+
+ adapter_update_device_from_info(adapter, info, services);
}
void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
@@ -322,6 +424,7 @@ void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
struct remote_dev_info *dev, match;
uint8_t name_type = 0x00;
name_status_t name_status;
+ GSList *services = NULL;
int state;
dbus_bool_t legacy;
unsigned char features[8];
@@ -350,6 +453,9 @@ void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
adapter_set_state(adapter, state);
}
+ /* Extract UUIDs from extended inquiry response if any */
+ services = get_eir_uuids(data, EIR_DATA_LENGTH);
+
memset(&match, 0, sizeof(struct remote_dev_info));
bacpy(&match.bdaddr, peer);
match.name_status = NAME_SENT;
@@ -358,7 +464,7 @@ void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
if (dev) {
adapter_update_found_devices(adapter, peer, rssi, class,
NULL, NULL, dev->legacy,
- NAME_NOT_REQUIRED, data);
+ services, NAME_NOT_REQUIRED);
return;
}
@@ -410,7 +516,7 @@ void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
/* add in the list to track name sent/pending */
adapter_update_found_devices(adapter, peer, rssi, class, name, alias,
- legacy, name_status, data);
+ legacy, services, name_status);
g_free(name);
g_free(alias);
@@ -500,7 +606,7 @@ void btd_event_remote_name(bdaddr_t *local, bdaddr_t *peer, uint8_t status,
if (dev_info) {
g_free(dev_info->name);
dev_info->name = g_strdup(name);
- adapter_emit_device_found(adapter, dev_info, NULL, 0);
+ adapter_emit_device_found(adapter, dev_info);
}
if (device)
--
1.7.0.4
^ permalink raw reply related
* [PATCH 2/4] Modify get_eir_uuids() to parse other EIR data
From: Bruna Moreira @ 2010-12-17 14:57 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1292597844-17135-1-git-send-email-bruna.moreira@openbossa.org>
From: Anderson Lizardo <anderson.lizardo@openbossa.org>
Currently, EIR parsing (name, UUIDs, flags etc.) is done in various
places. This leads to duplicated checks and does not support parsing
multiple AD/EIR structures on a single transfer.
These changes modify get_eir_uuids() to parse other EIR data types and
avoid passing raw EIR data to higher layers.
To accomplish this, get_eir_uuids() was renamed to parse_eir_data(), and
a new (internal) "struct eir_data" was introduced to store the parsed
data.
This first commit only handles the services UUIDs, following patches
will move other EIR parsing to it until everything is done inside
parse_eir_data().
---
src/event.c | 49 ++++++++++++++++++++++++++++++++-----------------
1 files changed, 32 insertions(+), 17 deletions(-)
diff --git a/src/event.c b/src/event.c
index 0e6c86f..3352717 100644
--- a/src/event.c
+++ b/src/event.c
@@ -58,6 +58,10 @@
#include "event.h"
#include "sdpd.h"
+struct eir_data {
+ GSList *services;
+};
+
static gboolean get_adapter_and_device(bdaddr_t *src, bdaddr_t *dst,
struct btd_adapter **adapter,
struct btd_device **device,
@@ -300,14 +304,14 @@ void btd_event_simple_pairing_complete(bdaddr_t *local, bdaddr_t *peer,
device_simple_pairing_complete(device, status);
}
-static GSList *get_eir_uuids(uint8_t *eir_data, size_t eir_length)
+static int parse_eir_data(struct eir_data *eir, uint8_t *eir_data,
+ size_t eir_length)
{
uint16_t len = 0;
size_t total;
size_t uuid16_count = 0;
size_t uuid32_count = 0;
size_t uuid128_count = 0;
- GSList *list = NULL;
uint8_t *uuid16 = NULL;
uint8_t *uuid32 = NULL;
uint8_t *uuid128 = NULL;
@@ -315,8 +319,9 @@ static GSList *get_eir_uuids(uint8_t *eir_data, size_t eir_length)
char *uuid_str;
unsigned int i;
+ /* No EIR data to parse */
if (eir_data == NULL || eir_length == 0)
- return list;
+ return 0;
while (len < eir_length - 1) {
uint8_t field_len = eir_data[0];
@@ -349,12 +354,13 @@ static GSList *get_eir_uuids(uint8_t *eir_data, size_t eir_length)
/* Bail out if got incorrect length */
if (len > eir_length)
- return list;
+ return -EINVAL;
total = uuid16_count + uuid32_count + uuid128_count;
+ /* No UUIDs were parsed, so skip code below */
if (!total)
- return list;
+ return 0;
/* Generate uuids in SDP format (EIR data is Little Endian) */
service.type = SDP_UUID16;
@@ -364,7 +370,7 @@ static GSList *get_eir_uuids(uint8_t *eir_data, size_t eir_length)
val16 = (val16 << 8) + uuid16[0];
service.value.uuid16 = val16;
uuid_str = bt_uuid2string(&service);
- list = g_slist_append(list, uuid_str);
+ eir->services = g_slist_append(eir->services, uuid_str);
uuid16 += 2;
}
@@ -378,7 +384,7 @@ static GSList *get_eir_uuids(uint8_t *eir_data, size_t eir_length)
service.value.uuid32 = val32;
uuid_str = bt_uuid2string(&service);
- list = g_slist_append(list, uuid_str);
+ eir->services = g_slist_append(eir->services, uuid_str);
uuid32 += 4;
}
@@ -390,17 +396,18 @@ static GSList *get_eir_uuids(uint8_t *eir_data, size_t eir_length)
service.value.uuid128.data[k] = uuid128[16 - k - 1];
uuid_str = bt_uuid2string(&service);
- list = g_slist_append(list, uuid_str);
+ eir->services = g_slist_append(eir->services, uuid_str);
uuid128 += 16;
}
- return list;
+ return 0;
}
void btd_event_advertising_report(bdaddr_t *local, le_advertising_info *info)
{
struct btd_adapter *adapter;
- GSList *services = NULL;
+ struct eir_data eir_data;
+ int err;
adapter = manager_find_adapter(local);
if (adapter == NULL) {
@@ -409,9 +416,13 @@ void btd_event_advertising_report(bdaddr_t *local, le_advertising_info *info)
}
/* Extract UUIDs from advertising data if any */
- services = get_eir_uuids(info->data, info->length);
+ memset(&eir_data, 0, sizeof(eir_data));
+ err = parse_eir_data(&eir_data, info->data, info->length);
+ if (err < 0)
+ error("Error parsing advertising data: %s (%d)",
+ strerror(-err), -err);
- adapter_update_device_from_info(adapter, info, services);
+ adapter_update_device_from_info(adapter, info, eir_data.services);
}
void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
@@ -424,8 +435,8 @@ void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
struct remote_dev_info *dev, match;
uint8_t name_type = 0x00;
name_status_t name_status;
- GSList *services = NULL;
- int state;
+ struct eir_data eir_data;
+ int state, err;
dbus_bool_t legacy;
unsigned char features[8];
@@ -454,7 +465,10 @@ void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
}
/* Extract UUIDs from extended inquiry response if any */
- services = get_eir_uuids(data, EIR_DATA_LENGTH);
+ memset(&eir_data, 0, sizeof(eir_data));
+ err = parse_eir_data(&eir_data, data, EIR_DATA_LENGTH);
+ if (err < 0)
+ error("Error parsing EIR data: %s (%d)", strerror(-err), -err);
memset(&match, 0, sizeof(struct remote_dev_info));
bacpy(&match.bdaddr, peer);
@@ -464,7 +478,8 @@ void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
if (dev) {
adapter_update_found_devices(adapter, peer, rssi, class,
NULL, NULL, dev->legacy,
- services, NAME_NOT_REQUIRED);
+ eir_data.services,
+ NAME_NOT_REQUIRED);
return;
}
@@ -516,7 +531,7 @@ void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
/* add in the list to track name sent/pending */
adapter_update_found_devices(adapter, peer, rssi, class, name, alias,
- legacy, services, name_status);
+ legacy, eir_data.services, name_status);
g_free(name);
g_free(alias);
--
1.7.0.4
^ permalink raw reply related
* [PATCH 3/4] Move AD flags parsing to parse_eir_data()
From: Bruna Moreira @ 2010-12-17 14:57 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1292597844-17135-1-git-send-email-bruna.moreira@openbossa.org>
From: Anderson Lizardo <anderson.lizardo@openbossa.org>
---
src/adapter.c | 22 ++++------------------
src/adapter.h | 3 ++-
src/event.c | 7 ++++++-
3 files changed, 12 insertions(+), 20 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 34bf24a..c74019d 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2786,21 +2786,6 @@ static struct remote_dev_info *get_found_dev(struct btd_adapter *adapter,
return dev;
}
-static gboolean extract_eir_flags(uint8_t *flags, uint8_t *eir_data)
-{
- if (eir_data[0] == 0)
- return FALSE;
-
- if (eir_data[1] != EIR_FLAGS)
- return FALSE;
-
- /* For now, only one octet is used for flags */
- if (flags)
- *flags = eir_data[2];
-
- return TRUE;
-}
-
static void remove_same_uuid(gpointer data, gpointer user_data)
{
struct remote_dev_info *dev = user_data;
@@ -2819,7 +2804,8 @@ static void remove_same_uuid(gpointer data, gpointer user_data)
}
void adapter_update_device_from_info(struct btd_adapter *adapter,
- le_advertising_info *info, GSList *services)
+ le_advertising_info *info,
+ GSList *services, uint8_t flags)
{
struct remote_dev_info *dev;
bdaddr_t bdaddr;
@@ -2845,14 +2831,14 @@ void adapter_update_device_from_info(struct btd_adapter *adapter,
g_slist_foreach(services, remove_same_uuid, dev);
dev->services = g_slist_concat(dev->services, services);
+ dev->flags = flags;
+
if (info->length) {
char *tmp_name = bt_extract_eir_name(info->data, NULL);
if (tmp_name) {
g_free(dev->name);
dev->name = tmp_name;
}
-
- extract_eir_flags(info->data, &dev->flags);
}
/* FIXME: check if other information was changed before emitting the
diff --git a/src/adapter.h b/src/adapter.h
index efcf5b8..d5dceb9 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -124,7 +124,8 @@ gboolean adapter_is_ready(struct btd_adapter *adapter);
struct remote_dev_info *adapter_search_found_devices(struct btd_adapter *adapter,
struct remote_dev_info *match);
void adapter_update_device_from_info(struct btd_adapter *adapter,
- le_advertising_info *info, GSList *services);
+ le_advertising_info *info,
+ GSList *services, uint8_t flags);
void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
int8_t rssi, uint32_t class, const char *name,
const char *alias, gboolean legacy,
diff --git a/src/event.c b/src/event.c
index 3352717..4672106 100644
--- a/src/event.c
+++ b/src/event.c
@@ -60,6 +60,7 @@
struct eir_data {
GSList *services;
+ uint8_t flags;
};
static gboolean get_adapter_and_device(bdaddr_t *src, bdaddr_t *dst,
@@ -346,6 +347,9 @@ static int parse_eir_data(struct eir_data *eir, uint8_t *eir_data,
uuid128_count = field_len / 16;
uuid128 = &eir_data[2];
break;
+ case EIR_FLAGS:
+ eir->flags = eir_data[2];
+ break;
}
len += field_len + 1;
@@ -422,7 +426,8 @@ void btd_event_advertising_report(bdaddr_t *local, le_advertising_info *info)
error("Error parsing advertising data: %s (%d)",
strerror(-err), -err);
- adapter_update_device_from_info(adapter, info, eir_data.services);
+ adapter_update_device_from_info(adapter, info, eir_data.services,
+ eir_data.flags);
}
void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
--
1.7.0.4
^ permalink raw reply related
* [PATCH 4/4] Move local name parsing to parse_eir_data()
From: Bruna Moreira @ 2010-12-17 14:57 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1292597844-17135-1-git-send-email-bruna.moreira@openbossa.org>
From: Anderson Lizardo <anderson.lizardo@openbossa.org>
---
src/adapter.c | 12 +++++-------
src/adapter.h | 3 ++-
src/event.c | 34 ++++++++++++++++++++++------------
src/glib-helper.c | 22 ----------------------
src/glib-helper.h | 1 -
5 files changed, 29 insertions(+), 43 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index c74019d..b826d4a 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2805,7 +2805,8 @@ static void remove_same_uuid(gpointer data, gpointer user_data)
void adapter_update_device_from_info(struct btd_adapter *adapter,
le_advertising_info *info,
- GSList *services, uint8_t flags)
+ char *name, GSList *services,
+ uint8_t flags)
{
struct remote_dev_info *dev;
bdaddr_t bdaddr;
@@ -2833,12 +2834,9 @@ void adapter_update_device_from_info(struct btd_adapter *adapter,
dev->flags = flags;
- if (info->length) {
- char *tmp_name = bt_extract_eir_name(info->data, NULL);
- if (tmp_name) {
- g_free(dev->name);
- dev->name = tmp_name;
- }
+ if (name) {
+ g_free(dev->name);
+ dev->name = name;
}
/* FIXME: check if other information was changed before emitting the
diff --git a/src/adapter.h b/src/adapter.h
index d5dceb9..52417b6 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -125,7 +125,8 @@ struct remote_dev_info *adapter_search_found_devices(struct btd_adapter *adapter
struct remote_dev_info *match);
void adapter_update_device_from_info(struct btd_adapter *adapter,
le_advertising_info *info,
- GSList *services, uint8_t flags);
+ char *name, GSList *services,
+ uint8_t flags);
void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
int8_t rssi, uint32_t class, const char *name,
const char *alias, gboolean legacy,
diff --git a/src/event.c b/src/event.c
index 4672106..e138312 100644
--- a/src/event.c
+++ b/src/event.c
@@ -61,6 +61,8 @@
struct eir_data {
GSList *services;
uint8_t flags;
+ char *name;
+ gboolean name_complete;
};
static gboolean get_adapter_and_device(bdaddr_t *src, bdaddr_t *dst,
@@ -350,6 +352,16 @@ static int parse_eir_data(struct eir_data *eir, uint8_t *eir_data,
case EIR_FLAGS:
eir->flags = eir_data[2];
break;
+ case EIR_NAME_SHORT:
+ case EIR_NAME_COMPLETE:
+ if (g_utf8_validate((char *) &eir_data[2],
+ field_len - 1, NULL))
+ eir->name = g_strndup((char *) &eir_data[2],
+ field_len - 1);
+ else
+ eir->name = g_strdup("");
+ eir->name_complete = eir_data[1] == EIR_NAME_COMPLETE;
+ break;
}
len += field_len + 1;
@@ -426,8 +438,8 @@ void btd_event_advertising_report(bdaddr_t *local, le_advertising_info *info)
error("Error parsing advertising data: %s (%d)",
strerror(-err), -err);
- adapter_update_device_from_info(adapter, info, eir_data.services,
- eir_data.flags);
+ adapter_update_device_from_info(adapter, info, eir_data.name,
+ eir_data.services, eir_data.flags);
}
void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
@@ -436,9 +448,8 @@ void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
char filename[PATH_MAX + 1];
struct btd_adapter *adapter;
struct btd_device *device;
- char local_addr[18], peer_addr[18], *alias, *name, *tmp_name;
+ char local_addr[18], peer_addr[18], *alias, *name;
struct remote_dev_info *dev, match;
- uint8_t name_type = 0x00;
name_status_t name_status;
struct eir_data eir_data;
int state, err;
@@ -513,25 +524,24 @@ void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
} else
legacy = TRUE;
- tmp_name = bt_extract_eir_name(data, &name_type);
- if (tmp_name) {
- if (name_type == 0x09) {
- write_device_name(local, peer, tmp_name);
+ if (eir_data.name) {
+ if (eir_data.name_complete) {
+ write_device_name(local, peer, eir_data.name);
name_status = NAME_NOT_REQUIRED;
if (name)
g_free(name);
- name = tmp_name;
+ name = eir_data.name;
} else {
if (name)
- free(tmp_name);
+ free(eir_data.name);
else
- name = tmp_name;
+ name = eir_data.name;
}
}
- if (name && name_type != 0x08)
+ if (name && eir_data.name_complete)
name_status = NAME_SENT;
/* add in the list to track name sent/pending */
diff --git a/src/glib-helper.c b/src/glib-helper.c
index 6505249..c440e60 100644
--- a/src/glib-helper.c
+++ b/src/glib-helper.c
@@ -780,25 +780,3 @@ GSList *bt_string2list(const gchar *str)
return l;
}
-
-char *bt_extract_eir_name(uint8_t *data, uint8_t *type)
-{
- if (!data || !type)
- return NULL;
-
- if (data[0] == 0)
- return NULL;
-
- if (type)
- *type = data[1];
-
- switch (data[1]) {
- case EIR_NAME_SHORT:
- case EIR_NAME_COMPLETE:
- if (!g_utf8_validate((char *) (data + 2), data[0] - 1, NULL))
- return g_strdup("");
- return g_strndup((char *) (data + 2), data[0] - 1);
- }
-
- return NULL;
-}
diff --git a/src/glib-helper.h b/src/glib-helper.h
index 5bb20a6..10718bb 100644
--- a/src/glib-helper.h
+++ b/src/glib-helper.h
@@ -42,4 +42,3 @@ char *bt_name2string(const char *string);
int bt_string2uuid(uuid_t *uuid, const char *string);
gchar *bt_list2string(GSList *list);
GSList *bt_string2list(const gchar *str);
-char *bt_extract_eir_name(uint8_t *data, uint8_t *type);
--
1.7.0.4
^ permalink raw reply related
* [RFC][PATCH] Re: EDR support
From: Manuel Naranjo @ 2010-12-17 16:41 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: BlueZ
In-Reply-To: <1292585241.2658.0.camel@aeonflux>
[-- Attachment #1: Type: text/plain, Size: 2544 bytes --]
Hi Marcel,
> Hi Manuel,
>
>> I was checking the latest BlueZ source code and the kernel source code,
>> and I noticed that by default EDR is disabled, when a dongle is detected
>> net/bluetooth/hci_core.c initializes the packet types with DM1, DH1 and
>> HV1, and then net/bluetooth/hci_event.c does it for DM3-5 DH3-5 and
>> HV3-5, but it never initializes the 2DHx or 3DHx. Was it made on purpose
>> or is it a bug in the code?
> you have read the specification and realized that EDR uses inverse
> values?
I can't find that, but still I could find that according to the 2.1
specs + EDR when you want to enable EDR is up to the master or slave to
ask for the package type switch, that means as far as I understand ask
the other side to enable 2-DH1,2-DH3,2-DH5,3-DH1,3-DH3,3-DH5.
I get from this that from the BlueZ point of view is either the kernel
that should do that, or the BlueZ daemon or just rely that to the user
application.
But I think there are a few of non covered issues here, first when you
attach an EDR dongle which has the EDR (this means is capable of doing
2mbps and 3mbps acl connections the kernel side doesn't show this to the
userland (hciconfig), I made a patch that does exactly that, during the
device recognition of the kernel just read the features provided and or
that to the pkt_type flag. This helps a lot, now my dongle shows it can
do EDR:
EDR compatible:
Features: 0xff 0xff 0x8f 0xfe 0x9b 0xff 0x59 0x83
Packet type: DM1 DM3 DM5 DH1 DH3 DH5 HV1 HV2 HV3 2-DH1 2-DH3
2-DH5 3-DH1 3-DH3 3-DH5
EDR not compatible:
Features: 0xff 0xff 0x8f 0xf8 0x1b 0xf8 0x00 0x80
Packet type: DM1 DM3 DM5 DH1 DH3 DH5 HV1 HV2 HV3
If you think the patch is sane to be included then I format it properly
so far I made it against compat-wireless-2.6.36-4 but it wouldn't be an
issue to make it against the current kernel source.
Next thing was playing with hcitool cpt and that did work, I get speed
improvements, from ~ 60KBs I get up to 100KBs to the same device, just
by forcing it to only do 3-DH5 (which I know is not a good idea, because
it's best for both radios to adapt, based on the environment).
I think it's a good idea to rely EDR "enabling" to the user space,
otherwise connection complexity on the kernel well be way harder to
keep. I think the best would be to add some stuff to the DBUS so first
let the user know which packet types are available, second to allow it
to change on a new connection.
Please let me know what you think about it,
Manuel
[-- Attachment #2: enableEDR.patch --]
[-- Type: text/plain, Size: 2926 bytes --]
diff -uprN compat-wireless-2.6.36-4/include/net/bluetooth/hci.h compat-wireless-2.6.36-4.mine//include/net/bluetooth/hci.h
--- compat-wireless-2.6.36-4/include/net/bluetooth/hci.h 2010-11-08 22:00:51.000000000 -0300
+++ compat-wireless-2.6.36-4.mine//include/net/bluetooth/hci.h 2010-12-11 19:13:14.000000000 -0300
@@ -1,6 +1,7 @@
/*
BlueZ - Bluetooth protocol stack for Linux
Copyright (C) 2000-2001 Qualcomm Incorporated
+ Copyright (C) 2010 Naranjo Manuel Francisco <manuel@aircable.net>
Written 2000,2001 by Maxim Krasnyansky <maxk@qualcomm.com>
@@ -120,17 +121,31 @@ enum {
#define HCI_VENDOR_PKT 0xff
/* HCI packet types */
+#define HCI_2DH1 0x0002
+#define HCI_3DH1 0x0004
#define HCI_DM1 0x0008
-#define HCI_DM3 0x0400
-#define HCI_DM5 0x4000
#define HCI_DH1 0x0010
+#define HCI_2DH3 0x0100
+#define HCI_3DH3 0x0200
+#define HCI_DM3 0x0400
#define HCI_DH3 0x0800
+#define HCI_2DH5 0x1000
+#define HCI_3DH5 0x2000
+#define HCI_DM5 0x4000
#define HCI_DH5 0x8000
#define HCI_HV1 0x0020
#define HCI_HV2 0x0040
#define HCI_HV3 0x0080
+#define HCI_EV3 0x0008
+#define HCI_EV4 0x0010
+#define HCI_EV5 0x0020
+#define HCI_2EV3 0x0040
+#define HCI_3EV3 0x0080
+#define HCI_2EV5 0x0100
+#define HCI_3EV5 0x0200
+
#define SCO_PTYPE_MASK (HCI_HV1 | HCI_HV2 | HCI_HV3)
#define ACL_PTYPE_MASK (~SCO_PTYPE_MASK)
@@ -183,6 +198,12 @@ enum {
#define LMP_PSCHEME 0x02
#define LMP_PCONTROL 0x04
+#define LMP_EDR_ACL_2M 0x02
+#define LMP_EDR_ACL_3M 0x04
+#define LMP_ENH_ISCAN 0x08
+#define LMP_ILACE_ISCAN 0x10
+#define LMP_ILACE_PSCAN 0x20
+#define LMP_RSSI_INQ 0x40
#define LMP_ESCO 0x80
#define LMP_EV4 0x01
diff -uprN compat-wireless-2.6.36-4/net/bluetooth/hci_event.c compat-wireless-2.6.36-4.mine//net/bluetooth/hci_event.c
--- compat-wireless-2.6.36-4/net/bluetooth/hci_event.c 2010-11-08 22:00:51.000000000 -0300
+++ compat-wireless-2.6.36-4.mine//net/bluetooth/hci_event.c 2010-12-11 19:13:46.000000000 -0300
@@ -1,6 +1,7 @@
/*
BlueZ - Bluetooth protocol stack for Linux
Copyright (c) 2000-2001, 2010, Code Aurora Forum. All rights reserved.
+ Copyright (c) 2010 Naranjo Manuel Francisco <manuel@aircable.net>
Written 2000,2001 by Maxim Krasnyansky <maxk@qualcomm.com>
@@ -478,6 +479,26 @@ static void hci_cc_read_local_features(s
if (hdev->features[3] & LMP_ESCO)
hdev->esco_type |= (ESCO_EV3);
+ if (hdev->features[3] & LMP_EDR_ACL_2M){
+ hdev->pkt_type |= (HCI_2DH1);
+
+ if (hdev->features[0] & LMP_3SLOT)
+ hdev->pkt_type |= (HCI_2DH3);
+
+ if (hdev->features[0] & LMP_5SLOT)
+ hdev->pkt_type |= (HCI_2DH5);
+ }
+
+ if (hdev->features[3] & LMP_EDR_ACL_3M){
+ hdev->pkt_type |= (HCI_3DH1);
+
+ if (hdev->features[0] & LMP_3SLOT)
+ hdev->pkt_type |= (HCI_3DH3);
+
+ if (hdev->features[0] & LMP_5SLOT)
+ hdev->pkt_type |= (HCI_3DH5);
+ }
+
if (hdev->features[4] & LMP_EV4)
hdev->esco_type |= (ESCO_EV4);
^ permalink raw reply
* [PATCH 0/1] Add enc_read_blob_req() as defined in BT Core v4.0
From: Brian Gix @ 2010-12-17 19:08 UTC (permalink / raw)
Cc: rshaffer, padovan, linux-bluetooth
Trivial addition of ATT API to encode READ_BLOB_REQ pkt.
--
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* [PATCH] Add enc_read_blob_req() as defined in BT Core Spec v4.0
From: Brian Gix @ 2010-12-17 19:08 UTC (permalink / raw)
Cc: rshaffer, padovan, linux-bluetooth, Brian Gix
In-Reply-To: <1292612933-31095-1-git-send-email-bgix@codeaurora.org>
---
attrib/att.c | 19 +++++++++++++++++++
attrib/att.h | 2 ++
2 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/attrib/att.c b/attrib/att.c
index 445b192..f8dbc02 100644
--- a/attrib/att.c
+++ b/attrib/att.c
@@ -542,6 +542,25 @@ uint16_t enc_read_req(uint16_t handle, uint8_t *pdu, int len)
return min_len;
}
+uint16_t enc_read_blob_req(uint16_t handle, uint16_t offset, uint8_t *pdu,
+ int len)
+{
+ const uint16_t min_len = sizeof(pdu[0]) + sizeof(handle) +
+ sizeof(offset);
+
+ if (pdu == NULL)
+ return 0;
+
+ if (len < min_len)
+ return 0;
+
+ pdu[0] = ATT_OP_READ_BLOB_REQ;
+ att_put_u16(handle, &pdu[1]);
+ att_put_u16(offset, &pdu[3]);
+
+ return min_len;
+}
+
uint16_t dec_read_req(const uint8_t *pdu, int len, uint16_t *handle)
{
const uint16_t min_len = sizeof(pdu[0]) + sizeof(*handle);
diff --git a/attrib/att.h b/attrib/att.h
index 2c8c724..0b8612e 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -202,6 +202,8 @@ uint16_t enc_write_req(uint16_t handle, const uint8_t *value, int vlen,
uint16_t dec_write_req(const uint8_t *pdu, int len, uint16_t *handle,
uint8_t *value, int *vlen);
uint16_t enc_read_req(uint16_t handle, uint8_t *pdu, int len);
+uint16_t enc_read_blob_req(uint16_t handle, uint16_t offset, uint8_t *pdu,
+ int len);
uint16_t dec_read_req(const uint8_t *pdu, int len, uint16_t *handle);
uint16_t enc_read_resp(uint8_t *value, int vlen, uint8_t *pdu, int len);
uint16_t dec_read_resp(const uint8_t *pdu, int len, uint8_t *value, int *vlen);
--
1.7.2.2
^ permalink raw reply related
* Re: [PATCH 0/1] Add enc_read_blob_req() as defined in BT Core v4.0
From: Brian Gix @ 2010-12-17 21:01 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1292612933-31095-1-git-send-email-bgix@codeaurora.org>
Hi All
On Fri, 2010-12-17 at 11:08 -0800, Brian Gix wrote:
> Trivial addition of ATT API to encode READ_BLOB_REQ pkt.
>
I apologize for messing up what must seem to be trivial rules for what
information to put where in the patches I am trying to get incorporated.
This particular patch is in fact a trivial addition to the ATT code,
which I believe probably is identical in all respects (including WS I
hope) to how anyone else here would have implemented it.
My follow on to this will be multi-step GATT procedures, I am going to
try as an RFC first, as the concept of GATT procedures that contain more
than one ATT Request/Response will be something that undoubtedly will
cause people to want to offer input on.
I also apologize for the sporadic nature of communication this week, as
my workstation had a catastrophic failure which is taking some time to
recover from.
> --
> Brian Gix
> bgix@codeaurora.org
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [RFC 0/1] Implement Compound (Multi-step) GATT Procedure Support
From: Brian Gix @ 2010-12-17 22:48 UTC (permalink / raw)
Cc: rshaffer, padovan, linux-bluetooth
The following two proposed patches implement a method for correctly
staging GATT procedures that require multiple ATT req/resp exchanges.
The first RFC / PATCH is gattrib.[ch], which allows the caller to specify
a gboolean indicating that the command being queued is Compound
(even if the procedure ends up being single/atomic) and a queue ID
number, which allows the caller to cancel the GATT procedure at any
stage of the transaction.
IF -
The ATT opcode being queued is specified as compound, the resulting response
will not cause the next item in the queue to be sent until *after* the
response has been forwarded to the caller via the response callback.
IF -
The ATT opcode being queued is *not* compound, the resulting response
will service the pkt queue immediately (legacy functionality).
IF -
The ID passed to g_attrib_send_seq is ZERO, the command pkt will
be added to the TAIL of the queue, and a new ID assigned to it.
(Legacy functionality)
IF -
The ID passed to g_attrib_send_seq is NON-ZERO, the command pkt
will be added to the HEAD of the queue, causing it to be the next
pkt sent to the remote device. The NON-ZERO ID is also then able
to cancel the command.
The second RFC / PATCH is to gatt.c. It modifies the existing gatt_read_char()
function to recognize the need for a compound Read procedure, and implements
it as a READ_REQ + READ_BLOB_REQ + READ_BLOB_REQ ...<etc> as needed, using
the g_attrib_send_seq() logic from the first RFC / PATCH.
--
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* [RFC 1/2] Add g_attrib_send_seq()
From: Brian Gix @ 2010-12-17 22:48 UTC (permalink / raw)
Cc: rshaffer, padovan, linux-bluetooth, Brian Gix
In-Reply-To: <1292626132-30029-1-git-send-email-bgix@codeaurora.org>
Add g_attrib_send_seq() as an extension to g_attrib_send().
g_attrib_send_seq() functionally queues an entire
"GATT Procedure" as defined in the BT Core v4.0.
The intention is that the full procedure is run
to completion before the next GATT Procedure is
started. Subsequent ATT requests to a continuing
procedure are added to the head of the Attrib queue.
Fix g_attrib_send() to be the degenerative case of g_attrib_send_seq()
This function now chains to g_attrib_send_seq() with arguments
indicating that it is *not* a compound (multi-step) GATT
procedure, but rather that the entire procedure is performed
with a single ATT opcode request/response.
Fix received_data() to recognize that incoming response is (or isn't)
part of a compound Procedure, so that it waits for additional
requests before servicing the Attrib queue.
---
attrib/gattrib.c | 44 ++++++++++++++++++++++++++++++++++++--------
attrib/gattrib.h | 4 ++++
2 files changed, 40 insertions(+), 8 deletions(-)
diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index eace01b..8ef5d92 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -58,6 +58,7 @@ struct command {
guint16 len;
guint8 expected;
gboolean sent;
+ gboolean compound;
GAttribResultFunc func;
gpointer user_data;
GDestroyNotify notify;
@@ -285,6 +286,7 @@ static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer data)
uint8_t buf[512], status;
gsize len;
GIOStatus iostat;
+ gboolean compound = FALSE;
if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) {
attrib->read_watch = 0;
@@ -319,6 +321,8 @@ static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer data)
return attrib->events != NULL;
}
+ compound = cmd->compound;
+
if (buf[0] == ATT_OP_ERROR) {
status = buf[4];
goto done;
@@ -332,7 +336,8 @@ static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer data)
status = 0;
done:
- if (attrib->queue && g_queue_is_empty(attrib->queue) == FALSE)
+ if (!compound && attrib->queue &&
+ g_queue_is_empty(attrib->queue) == FALSE)
wake_up_sender(attrib);
if (cmd) {
@@ -340,6 +345,11 @@ done:
cmd->func(status, buf, len, cmd->user_data);
command_destroy(cmd);
+
+ if (compound && attrib->queue &&
+ g_queue_is_empty(attrib->queue) == FALSE)
+ wake_up_sender(attrib);
+
}
return TRUE;
@@ -367,12 +377,16 @@ GAttrib *g_attrib_new(GIOChannel *io)
return g_attrib_ref(attrib);
}
-guint g_attrib_send(GAttrib *attrib, guint8 opcode, const guint8 *pdu,
- guint16 len, GAttribResultFunc func,
- gpointer user_data, GDestroyNotify notify)
+guint g_attrib_send_seq(GAttrib *attrib, gboolean compound, guint id,
+ guint8 opcode, const guint8 *pdu, guint16 len,
+ GAttribResultFunc func, gpointer user_data,
+ GDestroyNotify notify)
{
struct command *c;
+ if (attrib == NULL || attrib->queue == NULL)
+ return 0;
+
c = g_try_new0(struct command, 1);
if (c == NULL)
return 0;
@@ -385,16 +399,30 @@ guint g_attrib_send(GAttrib *attrib, guint8 opcode, const guint8 *pdu,
c->func = func;
c->user_data = user_data;
c->notify = notify;
- c->id = ++attrib->next_cmd_id;
+ c->compound = compound;
- g_queue_push_tail(attrib->queue, c);
+ if (id) {
+ c->id = id;
+ g_queue_push_head(attrib->queue, c);
+ } else {
+ c->id = ++attrib->next_cmd_id;
+ g_queue_push_tail(attrib->queue, c);
- if (g_queue_get_length(attrib->queue) == 1)
- wake_up_sender(attrib);
+ if (g_queue_get_length(attrib->queue) == 1)
+ wake_up_sender(attrib);
+ }
return c->id;
}
+guint g_attrib_send(GAttrib *attrib, guint8 opcode, const guint8 *pdu,
+ guint16 len, GAttribResultFunc func,
+ gpointer user_data, GDestroyNotify notify)
+{
+ return g_attrib_send_seq(attrib, FALSE, 0, opcode,
+ pdu, len, func, user_data, notify);
+}
+
static gint command_cmp_by_id(gconstpointer a, gconstpointer b)
{
const struct command *cmd = a;
diff --git a/attrib/gattrib.h b/attrib/gattrib.h
index 0940289..ade21bc 100644
--- a/attrib/gattrib.h
+++ b/attrib/gattrib.h
@@ -53,6 +53,10 @@ gboolean g_attrib_set_destroy_function(GAttrib *attrib,
guint g_attrib_send(GAttrib *attrib, guint8 opcode, const guint8 *pdu,
guint16 len, GAttribResultFunc func,
gpointer user_data, GDestroyNotify notify);
+guint g_attrib_send_seq(GAttrib *attrib, gboolean compound, guint id,
+ guint8 opcode, const guint8 *pdu,
+ guint16 len, GAttribResultFunc func,
+ gpointer user_data, GDestroyNotify notify);
gboolean g_attrib_cancel(GAttrib *attrib, guint id);
gboolean g_attrib_cancel_all(GAttrib *attrib);
--
1.7.2.2
^ permalink raw reply related
* [RFC 2/2] Fix gatt_read_char() to work with long attributes
From: Brian Gix @ 2010-12-17 22:48 UTC (permalink / raw)
Cc: rshaffer, padovan, linux-bluetooth, Brian Gix
In-Reply-To: <1292626132-30029-1-git-send-email-bgix@codeaurora.org>
Fix gatt_read_char() so that it can correctly read an attribute longer
than (MTU-1) octets long. This is intended to be the first
use of the compound (multi-step) GATT Procedure mechanism
g_attrib_send_seq(), which will run the GATT procedure to completion
before executing any additional GATT Procedures. Functionaly, it
will check the result length of the original READ_REQ, and if
it is equal to the DEAFULT_MTU (23) octets long, it will make
a series of READ_BLOB_REQs until the entire remote attribute has
been read, after which it returns the result to the original
caller as if the data had been retrieved in a single ATT resp.
---
attrib/gatt.c | 134 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 132 insertions(+), 2 deletions(-)
diff --git a/attrib/gatt.c b/attrib/gatt.c
index bca8b49..3a89b30 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -97,15 +97,145 @@ guint gatt_read_char_by_uuid(GAttrib *attrib, uint16_t start, uint16_t end,
pdu, plen, func, user_data, NULL);
}
+struct read_long_data {
+ GAttrib *attrib;
+ GAttribResultFunc func;
+ gpointer user_data;
+ guint8 *buffer;
+ guint16 size;
+ guint16 handle;
+ guint id;
+ guint8 ref;
+};
+
+static void read_long_destroy(gpointer user_data)
+{
+ struct read_long_data *long_read = user_data;
+
+ if (--long_read->ref)
+ return;
+
+ if (long_read->buffer != NULL)
+ g_free(long_read->buffer);
+
+ g_free(long_read);
+}
+
+static void read_blob_helper(guint8 status, const guint8 *rpdu, guint16 rlen,
+ gpointer user_data)
+{
+ struct read_long_data *long_read = user_data;
+ uint8_t pdu[ATT_DEFAULT_MTU];
+ guint8 *tmp;
+ guint16 plen;
+ guint id;
+
+ if (status == ATT_ECODE_ATTR_NOT_LONG ||
+ status == ATT_ECODE_INVALID_OFFSET) {
+ status = 0;
+ goto done;
+ }
+
+ if (status != 0 || rlen == 1)
+ goto done;
+
+ tmp = g_try_realloc(long_read->buffer, long_read->size + rlen - 1);
+
+ if (tmp == NULL) {
+ status = ATT_ECODE_INSUFF_RESOURCES;
+ goto done;
+ }
+
+ memcpy(&tmp[long_read->size], &rpdu[1], rlen - 1);
+ long_read->buffer = tmp;
+ long_read->size += rlen - 1;
+
+ if (rlen < ATT_DEFAULT_MTU)
+ goto done;
+
+ plen = enc_read_blob_req(long_read->handle, long_read->size - 1,
+ pdu, sizeof(pdu));
+ id = g_attrib_send_seq(long_read->attrib, TRUE, long_read->id,
+ ATT_OP_READ_BLOB_REQ, pdu, plen,
+ read_blob_helper, long_read, read_long_destroy);
+
+ if (id != 0) {
+ long_read->ref++;
+ return;
+ }
+
+ status = ATT_ECODE_IO;
+
+done:
+ long_read->func(status, long_read->buffer, long_read->size,
+ long_read->user_data);
+}
+
+static void read_char_helper(guint8 status, const guint8 *rpdu,
+ guint16 rlen, gpointer user_data)
+{
+ struct read_long_data *long_read = user_data;
+ uint8_t pdu[ATT_DEFAULT_MTU];
+ guint16 plen;
+ guint id;
+
+ if (status != 0 || rlen < ATT_DEFAULT_MTU)
+ goto done;
+
+ long_read->buffer = g_malloc(rlen);
+
+ if (long_read->buffer == NULL)
+ goto done;
+
+ memcpy(long_read->buffer, rpdu, rlen);
+ long_read->size = rlen;
+
+ plen = enc_read_blob_req(long_read->handle, rlen - 1, pdu, sizeof(pdu));
+ id = g_attrib_send_seq(long_read->attrib, TRUE, long_read->id,
+ ATT_OP_READ_BLOB_REQ, pdu, plen, read_blob_helper,
+ long_read, read_long_destroy);
+
+ if (id != 0) {
+ long_read->ref++;
+ return;
+ }
+
+ status = ATT_ECODE_IO;
+
+done:
+ long_read->func(status, rpdu, rlen, long_read->user_data);
+}
+
guint gatt_read_char(GAttrib *attrib, uint16_t handle, GAttribResultFunc func,
gpointer user_data)
{
uint8_t pdu[ATT_DEFAULT_MTU];
guint16 plen;
+ guint id;
+ struct read_long_data *long_read;
+
+ long_read = g_try_new0(struct read_long_data, 1);
+
+ if (long_read == NULL)
+ return 0;
+
+ long_read->attrib = attrib;
+ long_read->func = func;
+ long_read->user_data = user_data;
+ long_read->handle = handle;
plen = enc_read_req(handle, pdu, sizeof(pdu));
- return g_attrib_send(attrib, ATT_OP_READ_REQ, pdu, plen, func,
- user_data, NULL);
+ id = g_attrib_send_seq(attrib, TRUE, 0, ATT_OP_READ_REQ, pdu, plen,
+ read_char_helper, long_read, read_long_destroy);
+
+ if (id == 0)
+ g_free(long_read);
+ else {
+ long_read->ref++;
+ long_read->id = id;
+ }
+
+ return id;
}
guint gatt_write_char(GAttrib *attrib, uint16_t handle, uint8_t *value,
--
1.7.2.2
^ permalink raw reply related
* [PATCH] Fix memory leak in adapter_service_ins_rem()
From: Anderson Lizardo @ 2010-12-19 1:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
---
src/adapter.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 5118306..407522f 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -980,7 +980,7 @@ static void adapter_service_ins_rem(const bdaddr_t *bdaddr, void *rec,
gboolean insert)
{
struct btd_adapter *adapter;
- GSList *adapters;
+ GSList *l, *adapters;
adapters = NULL;
@@ -995,8 +995,8 @@ static void adapter_service_ins_rem(const bdaddr_t *bdaddr, void *rec,
/* Emit D-Bus msg to all adapters */
adapters = manager_get_adapters();
- for (; adapters; adapters = adapters->next) {
- adapter = adapters->data;
+ for (l = adapters; l; l = l->next) {
+ adapter = l->data;
if (insert == TRUE)
adapter->services = sdp_list_insert_sorted(
@@ -1008,6 +1008,9 @@ static void adapter_service_ins_rem(const bdaddr_t *bdaddr, void *rec,
adapter_emit_uuids_updated(adapter);
}
+
+ if (bacmp(bdaddr, BDADDR_ANY) != 0)
+ g_slist_free(adapters);
}
void adapter_service_insert(const bdaddr_t *bdaddr, void *rec)
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] Fix memory leak in adapter_service_ins_rem()
From: Johan Hedberg @ 2010-12-19 8:48 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1292723784-10338-1-git-send-email-anderson.lizardo@openbossa.org>
Hi Lizardo,
On Sat, Dec 18, 2010, Anderson Lizardo wrote:
> ---
> src/adapter.c | 9 ++++++---
> 1 files changed, 6 insertions(+), 3 deletions(-)
Thanks for catching this, however I went ahead and pushed a slightly
different (and imho simpler) fix. manager_find_adapter needs to iterate
through adapters while comparing addresses anyway so we might as well do
the address comparison within the for-loop in adapter_service_ins_rem().
That way there's no need to create an artificial list for the specific
adapter case.
Johan
^ permalink raw reply
* Re: [PATCH] Add enc_read_blob_req() as defined in BT Core Spec v4.0
From: Johan Hedberg @ 2010-12-19 12:13 UTC (permalink / raw)
To: Brian Gix; +Cc: rshaffer, padovan, linux-bluetooth
In-Reply-To: <1292612933-31095-2-git-send-email-bgix@codeaurora.org>
Hi Brian,
On Fri, Dec 17, 2010, Brian Gix wrote:
> ---
> attrib/att.c | 19 +++++++++++++++++++
> attrib/att.h | 2 ++
> 2 files changed, 21 insertions(+), 0 deletions(-)
The patch has been pushed upstream. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH 1/4] Move get_eir_uuids() from src/adapter.c to src/event.c
From: Johan Hedberg @ 2010-12-19 12:27 UTC (permalink / raw)
To: Bruna Moreira; +Cc: linux-bluetooth, Anderson Lizardo
In-Reply-To: <1292597844-17135-1-git-send-email-bruna.moreira@openbossa.org>
Hi,
On Fri, Dec 17, 2010, Bruna Moreira wrote:
> Moving get_eir_uuids() to src/event.c removes the need to pass the raw
> EIR data to higher layers. Now it is not necessary to pass the original
> GSList of service UUIDs, because the list is concatenated (with
> verification of duplicate entries) on adapter_update_device_from_info()
> (for LE) and adapter_update_found_devices() (for BR/EDR).
> ---
> src/adapter.c | 145 +++++++++++---------------------------------------------
> src/adapter.h | 7 +--
> src/event.c | 114 +++++++++++++++++++++++++++++++++++++++++++--
> 3 files changed, 142 insertions(+), 124 deletions(-)
All four patches have been pushed upstream. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
From: Linus Walleij @ 2010-12-19 21:23 UTC (permalink / raw)
To: Par-Gunnar Hjalmdahl
Cc: Pavan Savoy, Vitaly Wool, Alan Cox, Arnd Bergmann, Samuel Ortiz,
Marcel Holtmann, linux-kernel, linux-bluetooth, Lukasz Rymanowski,
Par-Gunnar Hjalmdahl
In-Reply-To: <1292584829-28279-1-git-send-email-par-gunnar.p.hjalmdahl@stericsson.com>
2010/12/17 Par-Gunnar Hjalmdahl <par-gunnar.p.hjalmdahl@stericsson.com>:
> This is the 3rd patch set for the ST-Ericsson CG2900 connectivity
> controller. The CG2900 is a combo controller supporting GPS, Bluetooth,
> and FM radio. It uses HCI H:4 protocol to combine different functionalities
> on a common transport, where first byte in the data indicates the current
> channel. Channels 1-4 are standardized in the Bluetooth Core specification
> while the other channels are vendor specific.
I'm slightly confused by different comment threads on this patch set.
I would certainly appreciate if the subsystem maintainers and reviewers
who shaped this patch set could add their Acked-by to the parts
they're happy with.
Alan, Marcel, Sam & Arnd especially. (Your work is MUCH
appreciated.)
I'm trying to get an idea of what patches are OK and what patches
are being disputed here, so as to whether there is some consensus
or not.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
From: Vitaly Wool @ 2010-12-19 22:57 UTC (permalink / raw)
To: Linus Walleij
Cc: Par-Gunnar Hjalmdahl, Pavan Savoy, Alan Cox, Arnd Bergmann,
Samuel Ortiz, Marcel Holtmann, linux-kernel, linux-bluetooth,
Lukasz Rymanowski, Par-Gunnar Hjalmdahl
In-Reply-To: <AANLkTimCYea+G7Z+o-2BDCGHbRobM2n+VJLLcDO6g=9d@mail.gmail.com>
Hi Linus,
On Sun, Dec 19, 2010 at 10:23 PM, Linus Walleij
<linus.walleij@linaro.org> wrote:
> I'm slightly confused by different comment threads on this patch set.
let me first first repost the part from Arnd's email on the
architecture of the "shared transport" type of thing:
> I believe we already agree it should be something like
>
> bt ti-radio st-e-radio ti-gps st-e-gps broadcom-radio ...
> | | | | | | |
> +---------+----------+---------+--+----------+----------+---------+
> |
> common-hci-module
> |
> +-----------+-----------+
> | | | |
> serial spi i2c ...
I think an explanation on how the patchset from Par maps to this
architecture could be a good starting point for confusion minimization
:)
Thanks,
Vitaly
^ permalink raw reply
* [PATCH 1/3] Fix memory leak in src/rfkill.c
From: Anderson Lizardo @ 2010-12-20 4:29 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
Watches must be removed with g_source_remove() otherwise they leak a
reference count to the GIOChannel and internal memory allocations.
Also g_io_channel_set_close_on_unref() is used, so there is no need to
call g_io_channel_shutdown() by hand.
---
src/rfkill.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/rfkill.c b/src/rfkill.c
index 75397b8..6e9e040 100644
--- a/src/rfkill.c
+++ b/src/rfkill.c
@@ -139,6 +139,7 @@ static gboolean rfkill_event(GIOChannel *chan,
}
static GIOChannel *channel = NULL;
+static guint watch_id = 0;
void rfkill_init(void)
{
@@ -156,8 +157,8 @@ void rfkill_init(void)
channel = g_io_channel_unix_new(fd);
g_io_channel_set_close_on_unref(channel, TRUE);
- g_io_add_watch(channel, G_IO_IN | G_IO_NVAL | G_IO_HUP | G_IO_ERR,
- rfkill_event, NULL);
+ watch_id = g_io_add_watch(channel, G_IO_IN | G_IO_NVAL | G_IO_HUP |
+ G_IO_ERR, rfkill_event, NULL);
}
void rfkill_exit(void)
@@ -165,7 +166,9 @@ void rfkill_exit(void)
if (!channel)
return;
- g_io_channel_shutdown(channel, TRUE, NULL);
+ if (watch_id > 0)
+ g_source_remove(watch_id);
+
g_io_channel_unref(channel);
channel = NULL;
--
1.7.0.4
^ permalink raw reply related
* [PATCH 2/3] Fix memory leak in src/sdpd-server.c
From: Anderson Lizardo @ 2010-12-20 4:29 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1292819346-27318-1-git-send-email-anderson.lizardo@openbossa.org>
Watches must be removed with g_source_remove() otherwise they leak a
reference count to the GIOChannel and internal memory allocations.
Also do a little refactoring to avoid too many global variables.
---
src/sdpd-server.c | 83 +++++++++++++++++++++++++++++-----------------------
1 files changed, 46 insertions(+), 37 deletions(-)
diff --git a/src/sdpd-server.c b/src/sdpd-server.c
index efd6fd0..fbc4c96 100644
--- a/src/sdpd-server.c
+++ b/src/sdpd-server.c
@@ -48,9 +48,11 @@
#include "log.h"
#include "sdpd.h"
-static GIOChannel *l2cap_io = NULL, *unix_io = NULL;
-
-static int l2cap_sock, unix_sock;
+static struct sock_info {
+ int sk;
+ GIOChannel *io;
+ guint watch_id;
+} l2cap_sk, unix_sk;
/*
* SDP server initialization on startup includes creating the
@@ -71,8 +73,8 @@ static int init_server(uint16_t mtu, int master, int compat)
register_server_service();
/* Create L2CAP socket */
- l2cap_sock = socket(PF_BLUETOOTH, SOCK_SEQPACKET, BTPROTO_L2CAP);
- if (l2cap_sock < 0) {
+ l2cap_sk.sk = socket(PF_BLUETOOTH, SOCK_SEQPACKET, BTPROTO_L2CAP);
+ if (l2cap_sk.sk < 0) {
error("opening L2CAP socket: %s", strerror(errno));
return -1;
}
@@ -82,14 +84,16 @@ static int init_server(uint16_t mtu, int master, int compat)
bacpy(&l2addr.l2_bdaddr, BDADDR_ANY);
l2addr.l2_psm = htobs(SDP_PSM);
- if (bind(l2cap_sock, (struct sockaddr *) &l2addr, sizeof(l2addr)) < 0) {
+ if (bind(l2cap_sk.sk, (struct sockaddr *) &l2addr,
+ sizeof(l2addr)) < 0) {
error("binding L2CAP socket: %s", strerror(errno));
return -1;
}
if (master) {
int opt = L2CAP_LM_MASTER;
- if (setsockopt(l2cap_sock, SOL_L2CAP, L2CAP_LM, &opt, sizeof(opt)) < 0) {
+ if (setsockopt(l2cap_sk.sk, SOL_L2CAP, L2CAP_LM, &opt,
+ sizeof(opt)) < 0) {
error("setsockopt: %s", strerror(errno));
return -1;
}
@@ -99,7 +103,8 @@ static int init_server(uint16_t mtu, int master, int compat)
memset(&opts, 0, sizeof(opts));
optlen = sizeof(opts);
- if (getsockopt(l2cap_sock, SOL_L2CAP, L2CAP_OPTIONS, &opts, &optlen) < 0) {
+ if (getsockopt(l2cap_sk.sk, SOL_L2CAP, L2CAP_OPTIONS, &opts,
+ &optlen) < 0) {
error("getsockopt: %s", strerror(errno));
return -1;
}
@@ -107,25 +112,24 @@ static int init_server(uint16_t mtu, int master, int compat)
opts.omtu = mtu;
opts.imtu = mtu;
- if (setsockopt(l2cap_sock, SOL_L2CAP, L2CAP_OPTIONS, &opts, sizeof(opts)) < 0) {
+ if (setsockopt(l2cap_sk.sk, SOL_L2CAP, L2CAP_OPTIONS, &opts,
+ sizeof(opts)) < 0) {
error("setsockopt: %s", strerror(errno));
return -1;
}
}
- if (listen(l2cap_sock, 5) < 0) {
+ if (listen(l2cap_sk.sk, 5) < 0) {
error("listen: %s", strerror(errno));
return -1;
}
- if (!compat) {
- unix_sock = -1;
+ if (!compat)
return 0;
- }
/* Create local Unix socket */
- unix_sock = socket(PF_UNIX, SOCK_STREAM, 0);
- if (unix_sock < 0) {
+ unix_sk.sk = socket(PF_UNIX, SOCK_STREAM, 0);
+ if (unix_sk.sk < 0) {
error("opening UNIX socket: %s", strerror(errno));
return -1;
}
@@ -136,12 +140,13 @@ static int init_server(uint16_t mtu, int master, int compat)
unlink(unaddr.sun_path);
- if (bind(unix_sock, (struct sockaddr *) &unaddr, sizeof(unaddr)) < 0) {
+ if (bind(unix_sk.sk, (struct sockaddr *) &unaddr,
+ sizeof(unaddr)) < 0) {
error("binding UNIX socket: %s", strerror(errno));
return -1;
}
- if (listen(unix_sock, 5) < 0) {
+ if (listen(unix_sk.sk, 5) < 0) {
error("listen UNIX socket: %s", strerror(errno));
return -1;
}
@@ -195,21 +200,19 @@ static gboolean io_accept_event(GIOChannel *chan, GIOCondition cond, gpointer da
GIOChannel *io;
int nsk;
- if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) {
- g_io_channel_unref(chan);
+ if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL))
return FALSE;
- }
- if (data == &l2cap_sock) {
+ if (data == &l2cap_sk) {
struct sockaddr_l2 addr;
socklen_t len = sizeof(addr);
- nsk = accept(l2cap_sock, (struct sockaddr *) &addr, &len);
- } else if (data == &unix_sock) {
+ nsk = accept(l2cap_sk.sk, (struct sockaddr *) &addr, &len);
+ } else if (data == &unix_sk) {
struct sockaddr_un addr;
socklen_t len = sizeof(addr);
- nsk = accept(unix_sock, (struct sockaddr *) &addr, &len);
+ nsk = accept(unix_sk.sk, (struct sockaddr *) &addr, &len);
} else
return FALSE;
@@ -256,18 +259,20 @@ int start_sdp_server(uint16_t mtu, const char *did, uint32_t flags)
}
}
- l2cap_io = g_io_channel_unix_new(l2cap_sock);
- g_io_channel_set_close_on_unref(l2cap_io, TRUE);
+ l2cap_sk.io = g_io_channel_unix_new(l2cap_sk.sk);
+ g_io_channel_set_close_on_unref(l2cap_sk.io, TRUE);
- g_io_add_watch(l2cap_io, G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
- io_accept_event, &l2cap_sock);
+ l2cap_sk.watch_id = g_io_add_watch(l2cap_sk.io, G_IO_IN | G_IO_ERR |
+ G_IO_HUP | G_IO_NVAL,
+ io_accept_event, &l2cap_sk);
- if (compat && unix_sock > fileno(stderr)) {
- unix_io = g_io_channel_unix_new(unix_sock);
- g_io_channel_set_close_on_unref(unix_io, TRUE);
+ if (compat) {
+ unix_sk.io = g_io_channel_unix_new(unix_sk.sk);
+ g_io_channel_set_close_on_unref(unix_sk.io, TRUE);
- g_io_add_watch(unix_io, G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
- io_accept_event, &unix_sock);
+ unix_sk.watch_id = g_io_add_watch(unix_sk.io, G_IO_IN |
+ G_IO_ERR | G_IO_HUP | G_IO_NVAL,
+ io_accept_event, &unix_sk);
}
return 0;
@@ -279,9 +284,13 @@ void stop_sdp_server(void)
sdp_svcdb_reset();
- if (unix_io)
- g_io_channel_unref(unix_io);
+ if (l2cap_sk.watch_id > 0)
+ g_source_remove(l2cap_sk.watch_id);
+ g_io_channel_unref(l2cap_sk.io);
+ memset(&l2cap_sk, 0, sizeof(l2cap_sk));
- if (l2cap_io)
- g_io_channel_unref(l2cap_io);
+ if (unix_sk.watch_id > 0)
+ g_source_remove(unix_sk.watch_id);
+ g_io_channel_unref(unix_sk.io);
+ memset(&unix_sk, 0, sizeof(unix_sk));
}
--
1.7.0.4
^ permalink raw reply related
* [PATCH 3/3] Fix memory leaks in btio/btio.c
From: Anderson Lizardo @ 2010-12-20 4:29 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1292819346-27318-1-git-send-email-anderson.lizardo@openbossa.org>
The watches created by bt_io_* functions were not being removed from
main context, therefore the "destroy" function was never being called.
---
btio/btio.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
btio/btio.h | 2 ++
src/main.c | 3 +++
3 files changed, 56 insertions(+), 5 deletions(-)
diff --git a/btio/btio.c b/btio/btio.c
index d8439e0..0cd046e 100644
--- a/btio/btio.c
+++ b/btio/btio.c
@@ -65,12 +65,14 @@ struct connect {
BtIOConnect connect;
gpointer user_data;
GDestroyNotify destroy;
+ guint watch_id;
};
struct accept {
BtIOConnect connect;
gpointer user_data;
GDestroyNotify destroy;
+ guint watch_id;
};
struct server {
@@ -78,8 +80,11 @@ struct server {
BtIOConfirm confirm;
gpointer user_data;
GDestroyNotify destroy;
+ guint watch_id;
};
+static GSList *servers = NULL, *accepts = NULL, *connects = NULL;
+
static void server_remove(struct server *server)
{
if (server->destroy)
@@ -215,8 +220,11 @@ static void server_add(GIOChannel *io, BtIOConnect connect,
server->destroy = destroy;
cond = G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
- g_io_add_watch_full(io, G_PRIORITY_DEFAULT, cond, server_cb, server,
- (GDestroyNotify) server_remove);
+ server->watch_id = g_io_add_watch_full(io, G_PRIORITY_DEFAULT, cond,
+ server_cb, server,
+ (GDestroyNotify) server_remove);
+
+ servers = g_slist_append(servers, server);
}
static void connect_add(GIOChannel *io, BtIOConnect connect,
@@ -231,8 +239,11 @@ static void connect_add(GIOChannel *io, BtIOConnect connect,
conn->destroy = destroy;
cond = G_IO_OUT | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
- g_io_add_watch_full(io, G_PRIORITY_DEFAULT, cond, connect_cb, conn,
+ conn->watch_id = g_io_add_watch_full(io, G_PRIORITY_DEFAULT, cond,
+ connect_cb, conn,
(GDestroyNotify) connect_remove);
+
+ connects = g_slist_append(connects, conn);
}
static void accept_add(GIOChannel *io, BtIOConnect connect, gpointer user_data,
@@ -247,8 +258,11 @@ static void accept_add(GIOChannel *io, BtIOConnect connect, gpointer user_data,
accept->destroy = destroy;
cond = G_IO_OUT | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
- g_io_add_watch_full(io, G_PRIORITY_DEFAULT, cond, accept_cb, accept,
- (GDestroyNotify) accept_remove);
+ accept->watch_id = g_io_add_watch_full(io, G_PRIORITY_DEFAULT, cond,
+ accept_cb, accept,
+ (GDestroyNotify) accept_remove);
+
+ accepts = g_slist_append(accepts, accept);
}
static int l2cap_bind(int sock, const bdaddr_t *src, uint16_t psm,
@@ -1313,6 +1327,38 @@ GIOChannel *bt_io_listen(BtIOType type, BtIOConnect connect,
return io;
}
+void bt_io_destroy(void)
+{
+ GSList *l;
+
+ for (l = servers; l != NULL; l = g_slist_next(l)) {
+ struct server *server = l->data;
+
+ if (server->watch_id > 0)
+ g_source_remove(server->watch_id);
+ }
+ g_slist_free(servers);
+ servers = NULL;
+
+ for (l = connects; l != NULL; l = g_slist_next(l)) {
+ struct connect *conn = l->data;
+
+ if (conn->watch_id > 0)
+ g_source_remove(conn->watch_id);
+ }
+ g_slist_free(connects);
+ connects = NULL;
+
+ for (l = connects; l != NULL; l = g_slist_next(l)) {
+ struct accept *accept = l->data;
+
+ if (accept->watch_id > 0)
+ g_source_remove(accept->watch_id);
+ }
+ g_slist_free(accepts);
+ accepts = NULL;
+}
+
GQuark bt_io_error_quark(void)
{
return g_quark_from_static_string("bt-io-error-quark");
diff --git a/btio/btio.h b/btio/btio.h
index 53e8eaa..87e722f 100644
--- a/btio/btio.h
+++ b/btio/btio.h
@@ -95,4 +95,6 @@ GIOChannel *bt_io_listen(BtIOType type, BtIOConnect connect,
GDestroyNotify destroy, GError **err,
BtIOOption opt1, ...);
+void bt_io_destroy(void);
+
#endif
diff --git a/src/main.c b/src/main.c
index 1aaa181..1b7ff40 100644
--- a/src/main.c
+++ b/src/main.c
@@ -56,6 +56,7 @@
#include "dbus-common.h"
#include "agent.h"
#include "manager.h"
+#include "btio.h"
#ifdef HAVE_CAPNG
#include <cap-ng.h>
@@ -495,6 +496,8 @@ int main(int argc, char *argv[])
agent_exit();
+ bt_io_destroy();
+
g_main_loop_unref(event_loop);
if (config)
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH 1/3] Fix memory leak in src/rfkill.c
From: Johan Hedberg @ 2010-12-20 8:16 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1292819346-27318-1-git-send-email-anderson.lizardo@openbossa.org>
Hi Lizardo,
On Mon, Dec 20, 2010, Anderson Lizardo wrote:
> Watches must be removed with g_source_remove() otherwise they leak a
> reference count to the GIOChannel and internal memory allocations.
>
> Also g_io_channel_set_close_on_unref() is used, so there is no need to
> call g_io_channel_shutdown() by hand.
You do realize that the watch gets freed and removed from the main
context when the callback function returns FALSE, right? The only case
for all of these patches where you'd have unfreed memory is when you
stop iterating the mainloop while still having the sockets open (or even
if you close them during the very last iteration), i.e. when bluetoothd
is exiting. Is that the problem you're trying to fix here, i.e. getting
rid of unnecessary "noise" in the valgrind leak report?
Johan
^ permalink raw reply
* RE: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
From: Par-Gunnar HJALMDAHL @ 2010-12-20 9:15 UTC (permalink / raw)
To: Vitaly Wool, Linus Walleij
Cc: Pavan Savoy, Alan Cox, Arnd Bergmann, Samuel Ortiz,
Marcel Holtmann, linux-kernel@vger.kernel.org,
linux-bluetooth@vger.kernel.org, Lukasz Rymanowski,
Par-Gunnar Hjalmdahl
In-Reply-To: <AANLkTi=J1AdOgxA_S9p9Ts65PL3Mi5NXXWMf0f-=0AwN@mail.gmail.com>
Hi Vitaly,
> -----Original Message-----
> From: Vitaly Wool [mailto:vitalywool@gmail.com]
> Sent: den 19 december 2010 23:58
> To: Linus Walleij
> Cc: Par-Gunnar HJALMDAHL; Pavan Savoy; Alan Cox; Arnd Bergmann; Samuel
> Ortiz; Marcel Holtmann; linux-kernel@vger.kernel.org; linux-
> bluetooth@vger.kernel.org; Lukasz Rymanowski; Par-Gunnar Hjalmdahl
> Subject: Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
>=20
> Hi Linus,
>=20
> On Sun, Dec 19, 2010 at 10:23 PM, Linus Walleij
> <linus.walleij@linaro.org> wrote:
> > I'm slightly confused by different comment threads on this patch set.
>=20
> let me first first repost the part from Arnd's email on the
> architecture of the "shared transport" type of thing:
>=20
> > I believe we already agree it should be something like
> >
> > bt ti-radio st-e-radio ti-gps st-e-gps broadcom-radio
> ...
> > | | | | | | |
> > +---------+----------+---------+--+----------+----------+---------+
> > |
> > common-hci-module
> > |
> > +-----------+-----------+
> > | | | |
> > serial spi i2c ...
>=20
> I think an explanation on how the patchset from Par maps to this
> architecture could be a good starting point for confusion minimization
> :)
>=20
> Thanks,
> Vitaly
I would say our design would map like this:
common-hci-module: cg2900_core
serial, spi, i2c,... : cg2900_uart together with hci_ldisc (for other trans=
ports it would be different files)
bt, ti-radio, st-e-radio,...: cg2900_chip together with btcg2900 and other =
users per channel (cg2900_char_devices for users in User space)
So it is not a 1-to-1 mapping for the upper parts. I would draw it like thi=
s:
bt st-e-radio st-e-gps
| | |
+---------+----------+
|
ti-xx st-e cg2900...
| |
+---------+----------+
|
common-hci-module
|
+-----------+-----------+
| | | |
serial spi i2c ...
The reason for this difference I've gone through before. Basically there ar=
e so many special behaviors and needed handling that is individual for each=
chip (like startup and shutdown and in the case of CG2900 flow control ove=
r FM and BT channels for audio commands). If you then look at the users I g=
uess it would be possible to have one BT user, but it would have to be modi=
fied to handle vendor specific commands (as btcg2900 does with BT_ENABLE co=
mmand). As Arnd has drawn for FM and GPS the users would be completely indi=
vidual since they don't have a standardized interface.
/P-G
^ permalink raw reply
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