* Re: [PATCH 2/4] Use stored device name (if any) instead of EIR data
From: Anderson Lizardo @ 2010-12-27 15:21 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <AANLkTi=06P4ZpGO-sp7czZtn9FGLkBwPnrxNqDdGspfy@mail.gmail.com>
Hi,
On Mon, Dec 27, 2010 at 10:35 AM, Anderson Lizardo
<anderson.lizardo@openbossa.org> wrote:
> I've not found any references to the spec saying the shortened name
> has to be a prefix. I believe it is left to the implementer how to
> display such shortened names. bluez so far has made no distinction on
> the display of shortened names vs. complete names.
Interestingly, on Volume 3, Part C, 11.1.2, it is mentioned that for
Advertising Data (i.e. LE devices):
"A shortened name shall only contain contiguous
characters from the beginning of the full name. For example, if the device name
is ‘BT_Device_Name’ then the shortened name over BR/EDR could be
‘BT_Device’ while the shortened name on LE could be ‘BT_Dev’."
Nothing is said on the EIR format, though (see Vol 3, Part C, 8.1.2).
Regards,
--
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [PATCH 2/4] Use stored device name (if any) instead of EIR data
From: Anderson Lizardo @ 2010-12-27 15:33 UTC (permalink / raw)
To: Anderson Lizardo, Luiz Augusto von Dentz, linux-bluetooth
In-Reply-To: <20101227145322.GA16529@jh-x301>
Hi,
On Mon, Dec 27, 2010 at 10:53 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi,
>
> On Mon, Dec 27, 2010, Anderson Lizardo wrote:
>> > if the variable name is set
>> > than your could would skip eir name completely while the code did
>> > actually use the eir name, probably the eir name is updated more
>> > frequently so it is probably the most updated one and we should in
>> > fact update the storage if they don't match.
>>
>> I could not find on spec any mention that the "complete" EIR name may
>> change.
>
> It also doesn't say that it will not change. So the safe thing would be
> not to make any assumption about its permanence, right?
Agreed. I'll adapt this patch in a attempt to at least simplify the
current logic.
> Naturally the user of the remote device might change is name (if the
> remote device has such a feature) and then the EIR data would change
> too. Note that even if there's a name in storage it doesn't mean that
> it's from the same discovery session. It might be from a previous
> discovery session a long time ago and the user of the remote device
> might have changed its name since then. So imho a complete name in an
> EIR should always override whatever is in storage.
Ok, I'll keep this semantics but try to simplify the current logic.
> For the case of shortened names I don't think there's an indisputably
> right answer. The question is what is better: that which is "complete"
> or that which is more recent. I've got the feeling that the complete
> version would be better for the user, but someone might of course
> disagree with this.
For now, I'll keep the current (from master) behavior of only storing
complete EIR names and using the stored name in favor of the shortened
one. My intent is not to change behavior unless there is some need to
do so.
Regards,
--
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil
^ permalink raw reply
* [PATCH] Bluetooth: l2cap: fix misuse of logical operation in place of bitop
From: David Sterba @ 2010-12-27 18:00 UTC (permalink / raw)
To: padovan
Cc: linux-bluetooth, netdev, linux-kernel, David Sterba,
Marcel Holtmann, João Paulo Rechi Vita
CC: Marcel Holtmann <marcel@holtmann.org>
CC: "Gustavo F. Padovan" <padovan@profusion.mobi>
CC: João Paulo Rechi Vita <jprvita@profusion.mobi>
Signed-off-by: David Sterba <dsterba@suse.cz>
---
net/bluetooth/l2cap.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index cd8f6ea..bdfdfdc 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1893,8 +1893,8 @@ static int l2cap_sock_sendmsg(struct kiocb *iocb, struct socket *sock, struct ms
if (pi->mode == L2CAP_MODE_STREAMING) {
l2cap_streaming_send(sk);
} else {
- if (pi->conn_state & L2CAP_CONN_REMOTE_BUSY &&
- pi->conn_state && L2CAP_CONN_WAIT_F) {
+ if ((pi->conn_state & L2CAP_CONN_REMOTE_BUSY) &&
+ (pi->conn_state & L2CAP_CONN_WAIT_F)) {
err = len;
break;
}
--
1.7.3.4.626.g73e7b
^ permalink raw reply related
* [PATCH v2] Fix leak of EIR data if RSSI does not change
From: Anderson Lizardo @ 2010-12-27 18:17 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1293456068-11615-4-git-send-email-anderson.lizardo@openbossa.org>
If RSSI value does not change, memory used by parsed EIR data would leak
because it would not be assigned to the remote_dev_info structure.
Also simplify related code and replace a couple of g_free()'s with
free() (simply because they were allocated with malloc()).
---
src/adapter.c | 19 ++++++++++++-------
src/adapter.h | 11 ++++++-----
src/event.c | 47 ++++++++++++++++++++++++-----------------------
3 files changed, 42 insertions(+), 35 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index bfc2b8b..4b10189 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2917,7 +2917,7 @@ static void remove_same_uuid(gpointer data, gpointer user_data)
}
}
-void adapter_update_device_from_info(struct btd_adapter *adapter,
+gboolean adapter_update_device_from_info(struct btd_adapter *adapter,
bdaddr_t bdaddr, int8_t rssi,
uint8_t evt_type, char *name,
GSList *services, uint8_t flags)
@@ -2931,7 +2931,7 @@ void adapter_update_device_from_info(struct btd_adapter *adapter,
dev->le = TRUE;
dev->evt_type = evt_type;
} else if (dev->rssi == rssi)
- return;
+ return FALSE;
dev->rssi = rssi;
@@ -2951,12 +2951,15 @@ 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);
+
+ return TRUE;
}
-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,
- GSList *services, name_status_t name_status)
+gboolean 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, GSList *services,
+ name_status_t name_status)
{
struct remote_dev_info *dev;
gboolean new_dev;
@@ -2975,7 +2978,7 @@ void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
dev->legacy = legacy;
dev->name_status = name_status;
} else if (dev->rssi == rssi)
- return;
+ return FALSE;
dev->rssi = rssi;
@@ -2986,6 +2989,8 @@ void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
dev->services = g_slist_concat(dev->services, services);
adapter_emit_device_found(adapter, dev);
+
+ return TRUE;
}
int adapter_remove_found_device(struct btd_adapter *adapter, bdaddr_t *bdaddr)
diff --git a/src/adapter.h b/src/adapter.h
index 857eec8..cee0bd4 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -134,14 +134,15 @@ int adapter_get_state(struct btd_adapter *adapter);
int adapter_get_discover_type(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,
+gboolean adapter_update_device_from_info(struct btd_adapter *adapter,
bdaddr_t bdaddr, int8_t rssi,
uint8_t evt_type, 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,
- GSList *services, name_status_t name_status);
+gboolean 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, 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);
diff --git a/src/event.c b/src/event.c
index 178cea8..5beedb8 100644
--- a/src/event.c
+++ b/src/event.c
@@ -431,6 +431,13 @@ static int parse_eir_data(struct eir_data *eir, uint8_t *eir_data,
return 0;
}
+static void free_eir_data(struct eir_data *eir)
+{
+ g_slist_foreach(eir->services, (GFunc) g_free, NULL);
+ g_slist_free(eir->services);
+ g_free(eir->name);
+}
+
void btd_event_advertising_report(bdaddr_t *local, le_advertising_info *info)
{
struct btd_adapter *adapter;
@@ -452,9 +459,10 @@ void btd_event_advertising_report(bdaddr_t *local, le_advertising_info *info)
rssi = *(info->data + info->length);
- adapter_update_device_from_info(adapter, info->bdaddr, rssi,
+ if (!adapter_update_device_from_info(adapter, info->bdaddr, rssi,
info->evt_type, eir_data.name,
- eir_data.services, eir_data.flags);
+ eir_data.services, eir_data.flags))
+ free_eir_data(&eir_data);
}
static void update_lastseen(bdaddr_t *sba, bdaddr_t *dba)
@@ -529,6 +537,14 @@ void btd_event_device_found(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
else
name_status = NAME_NOT_REQUIRED;
+ /* Update storage if EIR contains the complete device name */
+ if (eir_data.name && eir_data.name_complete) {
+ write_device_name(local, peer, eir_data.name);
+ name_status = NAME_NOT_REQUIRED;
+ g_free(eir_data.name);
+ eir_data.name = NULL;
+ }
+
create_name(filename, PATH_MAX, STORAGEDIR, local_addr, "aliases");
alias = textfile_get(filename, peer_addr);
@@ -547,28 +563,13 @@ void btd_event_device_found(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
} else
legacy = TRUE;
- 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 = eir_data.name;
- } else {
- if (name)
- free(eir_data.name);
- else
- name = eir_data.name;
- }
- }
-
- adapter_update_found_devices(adapter, peer, rssi, class, name, alias,
- legacy, eir_data.services, name_status);
+ if (!adapter_update_found_devices(adapter, peer, rssi, class, name,
+ alias, legacy, eir_data.services,
+ name_status))
+ free_eir_data(&eir_data);
- g_free(name);
- g_free(alias);
+ free(name);
+ free(alias);
}
void btd_event_set_legacy_pairing(bdaddr_t *local, bdaddr_t *peer,
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH 2/4] Use stored device name (if any) instead of EIR data
From: Anderson Lizardo @ 2010-12-27 18:20 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1293456068-11615-2-git-send-email-anderson.lizardo@openbossa.org>
This patch should be ignored. Instead, I incorporated a small
refactoring on v2 of patch 4/4.
Regards,
--
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [PATCH 4/4] Fix leak of EIR data if RSSI does not change
From: Anderson Lizardo @ 2010-12-27 18:23 UTC (permalink / raw)
To: Anderson Lizardo, linux-bluetooth
In-Reply-To: <20101227151122.GD17203@jh-x301>
Hi Johan,
On Mon, Dec 27, 2010 at 11:11 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> In general this seems fine, but to avoid future memory leaks in case you
> go ahead and change the eir_data struct wouldn't it be better to have a
> separate free_eir_data() function which does the g_slist_foreach,
> g_slist_free and g_free?
I followed your suggestion and created a free_eir_data(). I also did a
simple code reordering that simplified the complete EIR name storage.
See v2 of this patch. (2/4 should be ignored).
Regards,
--
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [PATCH v2] Fix leak of EIR data if RSSI does not change
From: Johan Hedberg @ 2010-12-27 21:48 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1293473827-26418-1-git-send-email-anderson.lizardo@openbossa.org>
Hi Lizardo,
On Mon, Dec 27, 2010, Anderson Lizardo wrote:
> If RSSI value does not change, memory used by parsed EIR data would leak
> because it would not be assigned to the remote_dev_info structure.
>
> Also simplify related code and replace a couple of g_free()'s with
> free() (simply because they were allocated with malloc()).
Beware of words like "also" and "and" in commit messages. Often they are
a good indication that the patch shold be split into multiple ones.
Though in this case I can't really make up my mind if it's fine since
the changes are so strongly related. I'll leave it up to you to think
about it and decide if you can cleanly split this into two patches or
not.
> @@ -529,6 +537,14 @@ void btd_event_device_found(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
> else
> name_status = NAME_NOT_REQUIRED;
>
> + /* Update storage if EIR contains the complete device name */
> + if (eir_data.name && eir_data.name_complete) {
> + write_device_name(local, peer, eir_data.name);
> + name_status = NAME_NOT_REQUIRED;
> + g_free(eir_data.name);
> + eir_data.name = NULL;
> + }
> +
> create_name(filename, PATH_MAX, STORAGEDIR, local_addr, "aliases");
> alias = textfile_get(filename, peer_addr);
>
> @@ -547,28 +563,13 @@ void btd_event_device_found(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
> } else
> legacy = TRUE;
>
> - 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 = eir_data.name;
> - } else {
> - if (name)
> - free(eir_data.name);
> - else
> - name = eir_data.name;
> - }
> - }
> -
> - adapter_update_found_devices(adapter, peer, rssi, class, name, alias,
> - legacy, eir_data.services, name_status);
> + if (!adapter_update_found_devices(adapter, peer, rssi, class, name,
> + alias, legacy, eir_data.services,
> + name_status))
> + free_eir_data(&eir_data);
>
> - g_free(name);
> - g_free(alias);
> + free(name);
> + free(alias);
> }
>
> void btd_event_set_legacy_pairing(bdaddr_t *local, bdaddr_t *peer,
It looks to me like shortened names are completely ignored after this
patch, even to the extent that if there's a shortened name the memory
will be leaked if adapter_update_found_devices returns TRUE.
If we don't have any name in storage and all we have is a shortened name
then I think it should be included in the DeviceFound signal (which is
what the existing code does).
Johan
^ permalink raw reply
* Re: [PATCH v2] Fix leak of EIR data if RSSI does not change
From: Anderson Lizardo @ 2010-12-27 22:24 UTC (permalink / raw)
To: Anderson Lizardo, linux-bluetooth
In-Reply-To: <20101227214841.GA22777@jh-x301>
Hi Johan,
On Mon, Dec 27, 2010 at 5:48 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Beware of words like "also" and "and" in commit messages. Often they are
> a good indication that the patch shold be split into multiple ones.
> Though in this case I can't really make up my mind if it's fine since
> the changes are so strongly related. I'll leave it up to you to think
> about it and decide if you can cleanly split this into two patches or
> not.
I can split into two patches without problem.
> It looks to me like shortened names are completely ignored after this
> patch, even to the extent that if there's a shortened name the memory
> will be leaked if adapter_update_found_devices returns TRUE.
>
> If we don't have any name in storage and all we have is a shortened name
> then I think it should be included in the DeviceFound signal (which is
> what the existing code does).
You are right. Somehow I forgot about the shortened name case while
trying to refactor this part of the code.
I'll resend the patch with just the memory leak fix (taking care to
not introduce another one!) and if I still have any idea to simplify
this code (which I don't believe anymore) a second patch will come.
Regards,
--
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil
^ permalink raw reply
* [PATCH v2 1/4] Filtering interface name from program arguments
From: Michal Labedzki @ 2010-12-28 9:06 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Michal Labedzki
In-Reply-To: <AANLkTi%3dzo%3dpgCCgJ%3dsL9bUftf6Vv7nO7xac92zU6gECC%40mail.gmail.com>
tools: Name must be "hciX", where X is 0..HCI_MAX_DEV. Any other like "tty1,
"hci001" or "hci1x" will no longer work as "hci1".
tools: Add info that "bdaddr" can be put for parameter "interface".
---
attrib/gatttool.c | 9 ++++++---
compat/pand.c | 2 +-
lib/hci.c | 52 ++++++++++++++++++++++++++++++++++++++++++++--------
lib/hci_lib.h | 1 +
test/hciemu.c | 9 +++++----
test/l2test.c | 8 +++++---
test/rctest.c | 10 ++++++----
tools/ciptool.c | 9 +++++----
tools/hciconfig.c | 8 +++++++-
tools/hcitool.c | 4 ++--
tools/l2ping.c | 11 +++++++----
tools/main.c | 7 ++++---
tools/sdptool.c | 9 +++++----
13 files changed, 98 insertions(+), 41 deletions(-)
diff --git a/attrib/gatttool.c b/attrib/gatttool.c
index a234e36..43b4f57 100644
--- a/attrib/gatttool.c
+++ b/attrib/gatttool.c
@@ -105,10 +105,13 @@ static GIOChannel *do_connect(gboolean le)
/* Local adapter */
if (opt_src != NULL) {
- if (!strncmp(opt_src, "hci", 3))
- hci_devba(atoi(opt_src + 3), &sba);
- else
+ int devid;
+
+ devid = hci_filter_devname(opt_src);
+ if (devid < 0)
str2ba(opt_src, &sba);
+ else
+ hci_devba(devid, &sba);
} else
bacpy(&sba, BDADDR_ANY);
diff --git a/compat/pand.c b/compat/pand.c
index c3860fa..7331fad 100644
--- a/compat/pand.c
+++ b/compat/pand.c
@@ -586,7 +586,7 @@ static const char *main_help =
"\t--role -r <role> Local PAN role (PANU, NAP, GN)\n"
"\t--service -d <role> Remote PAN service (PANU, NAP, GN)\n"
"\t--ethernet -e <name> Network interface name\n"
- "\t--device -i <bdaddr> Source bdaddr\n"
+ "\t--device -i <hciX|bdaddr> Source interface or bdaddr\n"
"\t--nosdp -D Disable SDP\n"
"\t--auth -A Enable authentication\n"
"\t--encrypt -E Enable encryption\n"
diff --git a/lib/hci.c b/lib/hci.c
index 048fda4..fb29f87 100644
--- a/lib/hci.c
+++ b/lib/hci.c
@@ -887,22 +887,56 @@ int hci_get_route(bdaddr_t *bdaddr)
(long) (bdaddr ? bdaddr : BDADDR_ANY));
}
+/* return 1 if string is decimal number without leading zeros
+ * return 0 if not */
+static int is_devnumber(const char *c)
+{
+ if (c[0] == '0' && c[1] != 0)
+ return 0;
+
+ while (*c) {
+ if (*c < '0' || *c > '9')
+ return 0;
+ ++c;
+ }
+ return 1;
+}
+
+/* return HCI dev_id from string like "hciX", where X is dev_id
+ * return -1 if str has invalid format */
+int hci_filter_devname(const char *str)
+{
+ int dev_id;
+
+ if ((strlen(str) >= 4) && (!strncmp(str, "hci", 3)) &&
+ (is_devnumber(str + 3)))
+ dev_id = atoi(str + 3);
+ else
+ dev_id = -1;
+
+ if (dev_id >= HCI_MAX_DEV)
+ dev_id = -1;
+
+ return dev_id;
+}
+
+/* return dev_id, which is on UP state, from 'hciX' or 'bdaddr'
+ * otherwise return -1 */
int hci_devid(const char *str)
{
bdaddr_t ba;
- int id = -1;
+ int dev_id;
- if (!strncmp(str, "hci", 3) && strlen(str) >= 4) {
- id = atoi(str + 3);
- if (hci_devba(id, &ba) < 0)
- return -1;
- } else {
+ dev_id = hci_filter_devname(str);
+ if (dev_id < 0) {
errno = ENODEV;
str2ba(str, &ba);
- id = hci_for_each_dev(HCI_UP, __same_bdaddr, (long) &ba);
+ dev_id = hci_for_each_dev(HCI_UP, __same_bdaddr, (long) &ba);
+ } else if (hci_devba(dev_id, &ba) < 0) {
+ return -1;
}
- return id;
+ return dev_id;
}
int hci_devinfo(int dev_id, struct hci_dev_info *di)
@@ -925,6 +959,8 @@ int hci_devinfo(int dev_id, struct hci_dev_info *di)
return ret;
}
+/* return status 0 and bdaddr assigned to dev_id
+ * otherwise status -1 */
int hci_devba(int dev_id, bdaddr_t *bdaddr)
{
struct hci_dev_info di;
diff --git a/lib/hci_lib.h b/lib/hci_lib.h
index b63a2a4..ef00f99 100644
--- a/lib/hci_lib.h
+++ b/lib/hci_lib.h
@@ -60,6 +60,7 @@ int hci_inquiry(int dev_id, int len, int num_rsp, const uint8_t *lap, inquiry_in
int hci_devinfo(int dev_id, struct hci_dev_info *di);
int hci_devba(int dev_id, bdaddr_t *bdaddr);
int hci_devid(const char *str);
+int hci_filter_devname(const char *str);
int hci_read_local_name(int dd, int len, char *name, int to);
int hci_write_local_name(int dd, const char *name, int to);
diff --git a/test/hciemu.c b/test/hciemu.c
index a20374f..ae33d72 100644
--- a/test/hciemu.c
+++ b/test/hciemu.c
@@ -1234,15 +1234,16 @@ int main(int argc, char *argv[])
exit(1);
}
- if (strlen(argv[0]) > 3 && !strncasecmp(argv[0], "hci", 3)) {
+ dev = hci_filter_devname(argv[0]);
+ if (dev < 0) {
+ if (getbdaddrbyname(argv[0], &vdev.bdaddr) < 0)
+ exit(1);
+ } else {
dev = hci_devid(argv[0]);
if (dev < 0) {
perror("Invalid device");
exit(1);
}
- } else {
- if (getbdaddrbyname(argv[0], &vdev.bdaddr) < 0)
- exit(1);
}
if (detach) {
diff --git a/test/l2test.c b/test/l2test.c
index 17883a9..438ba39 100644
--- a/test/l2test.c
+++ b/test/l2test.c
@@ -1101,6 +1101,7 @@ int main(int argc, char *argv[])
{
struct sigaction sa;
int opt, sk, mode = RECV, need_addr = 0;
+ int devid;
bacpy(&bdaddr, BDADDR_ANY);
@@ -1175,10 +1176,11 @@ int main(int argc, char *argv[])
break;
case 'i':
- if (!strncasecmp(optarg, "hci", 3))
- hci_devba(atoi(optarg + 3), &bdaddr);
- else
+ devid = hci_filter_devname(optarg);
+ if (devid < 0)
str2ba(optarg, &bdaddr);
+ else
+ hci_devba(devid, &bdaddr);
break;
case 'P':
diff --git a/test/rctest.c b/test/rctest.c
index b3804f5..a828ad9 100644
--- a/test/rctest.c
+++ b/test/rctest.c
@@ -579,7 +579,7 @@ static void usage(void)
"\t-m multiple connects\n");
printf("Options:\n"
- "\t[-b bytes] [-i device] [-P channel] [-U uuid]\n"
+ "\t[-b bytes] [-i hciX|bdaddr] [-P channel] [-U uuid]\n"
"\t[-L seconds] enabled SO_LINGER option\n"
"\t[-W seconds] enable deferred setup\n"
"\t[-B filename] use data packets from file\n"
@@ -597,6 +597,7 @@ int main(int argc, char *argv[])
{
struct sigaction sa;
int opt, sk, mode = RECV, need_addr = 0;
+ int devid;
bacpy(&bdaddr, BDADDR_ANY);
@@ -644,10 +645,11 @@ int main(int argc, char *argv[])
break;
case 'i':
- if (!strncasecmp(optarg, "hci", 3))
- hci_devba(atoi(optarg + 3), &bdaddr);
- else
+ devid = hci_filter_devname(optarg);
+ if (devid < 0)
str2ba(optarg, &bdaddr);
+ else
+ hci_devba(devid, &bdaddr);
break;
case 'P':
diff --git a/tools/ciptool.c b/tools/ciptool.c
index edce9da..ec602ef 100644
--- a/tools/ciptool.c
+++ b/tools/ciptool.c
@@ -427,7 +427,7 @@ static void usage(void)
"\n");
printf("Options:\n"
- "\t-i [hciX|bdaddr] Local HCI device or BD Address\n"
+ "\t-i <hciX|bdaddr> Local HCI device or BD Address\n"
"\t-h, --help Display help\n"
"\n");
@@ -455,10 +455,11 @@ int main(int argc, char *argv[])
while ((opt = getopt_long(argc, argv, "+i:h", main_options, NULL)) != -1) {
switch(opt) {
case 'i':
- if (!strncmp(optarg, "hci", 3))
- hci_devba(atoi(optarg + 3), &bdaddr);
- else
+ i = hci_filter_devname(optarg);
+ if (i < 0)
str2ba(optarg, &bdaddr);
+ else
+ hci_devba(i, &bdaddr);
break;
case 'h':
usage();
diff --git a/tools/hciconfig.c b/tools/hciconfig.c
index f0ae11c..e20963d 100644
--- a/tools/hciconfig.c
+++ b/tools/hciconfig.c
@@ -1849,7 +1849,13 @@ int main(int argc, char *argv[])
exit(0);
}
- di.dev_id = atoi(argv[0] + 3);
+ i = hci_filter_devname(argv[0]);
+ if (i < 0) {
+ fprintf(stderr, "No valid device name.\n");
+ exit(1);
+ }
+ di.dev_id = i;
+
argc--; argv++;
if (ioctl(ctl, HCIGETDEVINFO, (void *) &di)) {
diff --git a/tools/hcitool.c b/tools/hcitool.c
index d50adaf..dc70e63 100644
--- a/tools/hcitool.c
+++ b/tools/hcitool.c
@@ -2560,8 +2560,8 @@ static void usage(void)
printf("Usage:\n"
"\thcitool [options] <command> [command parameters]\n");
printf("Options:\n"
- "\t--help\tDisplay help\n"
- "\t-i dev\tHCI device\n");
+ "\t--help Display help\n"
+ "\t-i <hciX|bdaddr> Local HCI device or BD Address\n");
printf("Commands:\n");
for (i = 0; command[i].cmd; i++)
printf("\t%-4s\t%s\n", command[i].cmd,
diff --git a/tools/l2ping.c b/tools/l2ping.c
index 29fb3d0..58c5faf 100644
--- a/tools/l2ping.c
+++ b/tools/l2ping.c
@@ -255,7 +255,8 @@ static void usage(void)
{
printf("l2ping - L2CAP ping\n");
printf("Usage:\n");
- printf("\tl2ping [-i device] [-s size] [-c count] [-t timeout] [-d delay] [-f] [-r] [-v] <bdaddr>\n");
+ printf("\tl2ping [-i <hciX|bdaddr>] [-s size] [-c count] [-t timeout] "
+ "[-d delay] [-f] [-r] [-v] <bdaddr>\n");
printf("\t-f Flood ping (delay = 0)\n");
printf("\t-r Reverse ping\n");
printf("\t-v Verify request and response payload\n");
@@ -264,6 +265,7 @@ static void usage(void)
int main(int argc, char *argv[])
{
int opt;
+ int devid;
/* Default options */
bacpy(&bdaddr, BDADDR_ANY);
@@ -271,10 +273,11 @@ int main(int argc, char *argv[])
while ((opt=getopt(argc,argv,"i:d:s:c:t:frv")) != EOF) {
switch(opt) {
case 'i':
- if (!strncasecmp(optarg, "hci", 3))
- hci_devba(atoi(optarg + 3), &bdaddr);
- else
+ devid = hci_filter_devname(optarg);
+ if (devid < 0)
str2ba(optarg, &bdaddr);
+ else
+ hci_devba(devid, &bdaddr);
break;
case 'd':
diff --git a/tools/main.c b/tools/main.c
index 6800445..c000fba 100644
--- a/tools/main.c
+++ b/tools/main.c
@@ -753,10 +753,11 @@ int main(int argc, char *argv[])
while ((opt = getopt_long(argc, argv, "+i:f:rahAESML:", main_options, NULL)) != -1) {
switch(opt) {
case 'i':
- if (strncmp(optarg, "hci", 3) == 0)
- hci_devba(atoi(optarg + 3), &bdaddr);
- else
+ dev_id = hci_filter_devname(optarg);
+ if (dev_id < 0)
str2ba(optarg, &bdaddr);
+ else
+ hci_devba(dev_id, &bdaddr);
break;
case 'f':
diff --git a/tools/sdptool.c b/tools/sdptool.c
index 140a46a..c782e62 100644
--- a/tools/sdptool.c
+++ b/tools/sdptool.c
@@ -4209,7 +4209,7 @@ static void usage(void)
"\tsdptool [options] <command> [command parameters]\n");
printf("Options:\n"
"\t-h\t\tDisplay help\n"
- "\t-i\t\tSpecify source interface\n");
+ "\t-i\t\tSpecify source interface or bdaddr\n");
printf("Commands:\n");
for (i = 0; command[i].cmd; i++)
@@ -4242,10 +4242,11 @@ int main(int argc, char *argv[])
while ((opt=getopt_long(argc, argv, "+i:h", main_options, NULL)) != -1) {
switch(opt) {
case 'i':
- if (!strncmp(optarg, "hci", 3))
- hci_devba(atoi(optarg + 3), &interface);
- else
+ i = hci_filter_devname(optarg);
+ if (i < 0)
str2ba(optarg, &interface);
+ else
+ hci_devba(i, &interface);
break;
case 'h':
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 4/4] Better filtering input string contain bdaddr
From: Michal Labedzki @ 2010-12-28 9:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Michal Labedzki
In-Reply-To: <AANLkTinXwbHZt4SSaz+tUUH+7QnQo7L4FKFKetmQm9Qx@mail.gmail.com>
---
lib/bluetooth.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++------
1 files changed, 46 insertions(+), 6 deletions(-)
diff --git a/lib/bluetooth.c b/lib/bluetooth.c
index 3a4e3f4..4e32a1d 100644
--- a/lib/bluetooth.c
+++ b/lib/bluetooth.c
@@ -60,23 +60,63 @@ char *batostr(const bdaddr_t *ba)
return str;
}
+/* convert to bdaddr_t string describes with regular expression "(XX:){0,5}XX"
+ * where X is "[0-9A-Fa-f]"
+ * shorter address is filling with ":00" at the end
+ */
bdaddr_t *strtoba(const char *str)
{
const char *ptr = str;
int i;
+ int parts;
+ int length;
+ bdaddr_t *ba;
+
+ length = strlen(str);
+ if (length <= 1 || length > 17)
+ return NULL;
- uint8_t *ba = bt_malloc(sizeof(bdaddr_t));
+ ba = bt_malloc(sizeof(bdaddr_t));
if (!ba)
return NULL;
- for (i = 0; i < 6; i++) {
- ba[i] = (uint8_t) strtol(ptr, NULL, 16);
- if (i != 5 && !(ptr = strchr(ptr,':')))
- ptr = ":00:00:00:00:00";
+ i = 0;
+ parts = 0;
+ while (*ptr) {
+ if (i == 2) {
+ if (*ptr == ':') {
+ i = 0;
+ ba->b[parts] = strtol(ptr - 2, NULL, 16);
+ parts++;
+ ptr++;
+ } else {
+ bt_free(ba);
+ return NULL;
+ }
+ }
+
+ if ((*ptr < 'A' || *ptr > 'F') &&
+ (*ptr < 'a' || *ptr > 'f') &&
+ (*ptr < '0' || *ptr > '9')) {
+ bt_free(ba);
+ return NULL;
+ }
+
+ i++;
ptr++;
}
- return (bdaddr_t *) ba;
+ if (*(ptr - 1) == ':' || i == 1) {
+ bt_free(ba);
+ return NULL;
+ }
+
+ ba->b[parts] = strtol(ptr - 2, NULL, 16);
+ for (i = parts + 1 ; i < 6 ; i++) {
+ ba->b[i] = 0;
+ }
+
+ return ba;
}
/* reverse bdaddr and do batostr */
--
1.7.0.4
^ permalink raw reply related
* [PATCH v2 2/4] Refactoring common code in bluetooth.c
From: Michal Labedzki @ 2010-12-28 9:17 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Michal Labedzki
In-Reply-To: <AANLkTi==TysBDUUXrDYy4bizr+3Nz20nn2fyc_g2g__U@mail.gmail.com>
in lib/bluetooth.c make "str2ba" using "strtoba", "ba2str" using "batostr"
add comments describe differences between these functions
---
lib/bluetooth.c | 34 +++++++++++++++++-----------------
1 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/lib/bluetooth.c b/lib/bluetooth.c
index 4af2ef6..3a4e3f4 100644
--- a/lib/bluetooth.c
+++ b/lib/bluetooth.c
@@ -55,8 +55,7 @@ char *batostr(const bdaddr_t *ba)
return NULL;
sprintf(str, "%2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X",
- ba->b[0], ba->b[1], ba->b[2],
- ba->b[3], ba->b[4], ba->b[5]);
+ ba->b[0], ba->b[1], ba->b[2], ba->b[3], ba->b[4], ba->b[5]);
return str;
}
@@ -80,29 +79,30 @@ bdaddr_t *strtoba(const char *str)
return (bdaddr_t *) ba;
}
+/* reverse bdaddr and do batostr */
int ba2str(const bdaddr_t *ba, char *str)
{
- uint8_t b[6];
-
- baswap((bdaddr_t *) b, ba);
- return sprintf(str, "%2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X",
- b[0], b[1], b[2], b[3], b[4], b[5]);
+ bdaddr_t b;
+ char *bastr;
+
+ baswap(&b, ba);
+ bastr = batostr(&b);
+ strcpy(str, bastr);
+ bt_free(bastr);
+ return *str && 1;
}
+/* do strtoba and return reverse bdaddr */
int str2ba(const char *str, bdaddr_t *ba)
{
- uint8_t b[6];
- const char *ptr = str;
+ bdaddr_t *b;
int i;
- for (i = 0; i < 6; i++) {
- b[i] = (uint8_t) strtol(ptr, NULL, 16);
- if (i != 5 && !(ptr = strchr(ptr, ':')))
- ptr = ":00:00:00:00:00";
- ptr++;
- }
-
- baswap(ba, (bdaddr_t *) b);
+ b = strtoba(str);
+ if (b == NULL)
+ return 0;
+ baswap(ba, b);
+ bt_free(b);
return 0;
}
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH 3/4] Fix memory leaks after using strtoba, batostr
From: Michal.Labedzki @ 2010-12-28 9:34 UTC (permalink / raw)
To: anderson.lizardo; +Cc: linux-bluetooth
In-Reply-To: <AANLkTikD4S-rD+NCXs7URSYv3Ah=-uToEw4YyCP4cA5x@mail.gmail.com>
>Hi,
>
>This patch would not be necessary if you keep only str2ba() and
>ba2str() (like I commented on patch 2/4).
>
>Regards,
Cannot keep only *2*() functions, because for two reason. Firstly, these functions are different (see discussion at patch 2/4).
Secondly, for example, "ba2str" use external buffer, while "batostr" create it internal. So simpler is using "batostr",
because there is no need to create temporary buffer.
So I think patch is (quite) useful.
Regards
^ permalink raw reply
* Re: [PATCH] ath3k-2.fw is obsolete and is not used by Atheros chipsets.
From: Bala Shanmugam @ 2010-12-28 11:40 UTC (permalink / raw)
To: dwmw2@infradead.org; +Cc: linux-bluetooth@vger.kernel.org, Luis Rodriguez
In-Reply-To: <1292922606-32255-1-git-send-email-sbalashanmugam@atheros.com>
Hi David,
Can you please upstream this patch.
Regards,
Bala.
^ permalink raw reply
* [PATCHv3] Bluetooth: Use non-flushable by default L2CAP data packets
From: Emeltchenko Andrei @ 2010-12-28 15:16 UTC (permalink / raw)
To: linux-bluetooth
From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
Modification of Nick Pelly <npelly@google.com> patch.
Changes to previous releases:
- rebasing, simplifying logic, using SOL_BLUETOOTH instead of SOL_L2CAP.
- LMP_NO_FLUSH feature mask corrected, added check to l2cap_send_sframe
Andrei Emeltchenko (1):
Bluetooth: Use non-flushable by default L2CAP data packets
include/net/bluetooth/bluetooth.h | 5 +++
include/net/bluetooth/hci.h | 2 +
include/net/bluetooth/hci_core.h | 1 +
include/net/bluetooth/l2cap.h | 1 +
net/bluetooth/hci_core.c | 7 +++-
net/bluetooth/l2cap.c | 57 ++++++++++++++++++++++++++++++++++--
6 files changed, 67 insertions(+), 6 deletions(-)
^ permalink raw reply
* [PATCHv3] Bluetooth: Use non-flushable by default L2CAP data packets
From: Emeltchenko Andrei @ 2010-12-28 15:16 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1293549367-2516-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
Modification of Nick Pelly <npelly@google.com> patch.
With Bluetooth 2.1 ACL packets can be flushable or non-flushable. This commit
makes ACL data packets non-flushable by default on compatible chipsets, and
adds the BT_FLUSHABLE socket option to explicitly request flushable ACL
data packets for a given L2CAP socket. This is useful for A2DP data which can
be safely discarded if it can not be delivered within a short time (while
other ACL data should not be discarded).
Note that making ACL data flushable has no effect unless the automatic flush
timeout for that ACL link is changed from its default of 0 (infinite).
Default packet types (for compatible chipsets):
Frame 34: 13 bytes on wire (104 bits), 13 bytes captured (104 bits)
Bluetooth HCI H4
Bluetooth HCI ACL Packet
.... 0000 0000 0010 = Connection Handle: 0x0002
..00 .... .... .... = PB Flag: First Non-automatically Flushable Packet (0)
00.. .... .... .... = BC Flag: Point-To-Point (0)
Data Total Length: 8
Bluetooth L2CAP Packet
After setting BT_FLUSHABLE
(sock.setsockopt(274 /*SOL_BLUETOOTH*/, 8 /* BT_FLUSHABLE */, 1 /* flush */))
Frame 34: 13 bytes on wire (104 bits), 13 bytes captured (104 bits)
Bluetooth HCI H4
Bluetooth HCI ACL Packet
.... 0000 0000 0010 = Connection Handle: 0x0002
..10 .... .... .... = PB Flag: First Automatically Flushable Packet (2)
00.. .... .... .... = BC Flag: Point-To-Point (0)
Data Total Length: 8
Bluetooth L2CAP Packet
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
---
include/net/bluetooth/bluetooth.h | 5 +++
include/net/bluetooth/hci.h | 2 +
include/net/bluetooth/hci_core.h | 1 +
include/net/bluetooth/l2cap.h | 1 +
net/bluetooth/hci_core.c | 7 +++-
net/bluetooth/l2cap.c | 57 ++++++++++++++++++++++++++++++++++--
6 files changed, 67 insertions(+), 6 deletions(-)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 0c5e725..ed7d775 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -64,6 +64,11 @@ struct bt_security {
#define BT_DEFER_SETUP 7
+#define BT_FLUSHABLE 8
+
+#define BT_FLUSHABLE_OFF 0
+#define BT_FLUSHABLE_ON 1
+
#define BT_INFO(fmt, arg...) printk(KERN_INFO "Bluetooth: " fmt "\n" , ## arg)
#define BT_ERR(fmt, arg...) printk(KERN_ERR "%s: " fmt "\n" , __func__ , ## arg)
#define BT_DBG(fmt, arg...) pr_debug("%s: " fmt "\n" , __func__ , ## arg)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 29a7a8c..5d033dc 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -150,6 +150,7 @@ enum {
#define EDR_ESCO_MASK (ESCO_2EV3 | ESCO_3EV3 | ESCO_2EV5 | ESCO_3EV5)
/* ACL flags */
+#define ACL_START_NO_FLUSH 0x00
#define ACL_CONT 0x01
#define ACL_START 0x02
#define ACL_ACTIVE_BCAST 0x04
@@ -194,6 +195,7 @@ enum {
#define LMP_EDR_3S_ESCO 0x80
#define LMP_SIMPLE_PAIR 0x08
+#define LMP_NO_FLUSH 0x40
/* Connection modes */
#define HCI_CM_ACTIVE 0x0000
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index a29feb0..59135a6 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -457,6 +457,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
#define lmp_sniffsubr_capable(dev) ((dev)->features[5] & LMP_SNIFF_SUBR)
#define lmp_esco_capable(dev) ((dev)->features[3] & LMP_ESCO)
#define lmp_ssp_capable(dev) ((dev)->features[6] & LMP_SIMPLE_PAIR)
+#define lmp_no_flush_capable(dev) ((dev)->features[6] & LMP_NO_FLUSH)
/* ----- HCI protocols ----- */
struct hci_proto {
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 7ad25ca..7f88a87 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -327,6 +327,7 @@ struct l2cap_pinfo {
__u8 sec_level;
__u8 role_switch;
__u8 force_reliable;
+ __u8 flushable;
__u8 conf_req[64];
__u8 conf_len;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 8b602d8..3c0990d 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1391,7 +1391,7 @@ void hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags)
skb->dev = (void *) hdev;
bt_cb(skb)->pkt_type = HCI_ACLDATA_PKT;
- hci_add_acl_hdr(skb, conn->handle, flags | ACL_START);
+ hci_add_acl_hdr(skb, conn->handle, flags);
list = skb_shinfo(skb)->frag_list;
if (!list) {
@@ -1409,12 +1409,15 @@ void hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags)
spin_lock_bh(&conn->data_q.lock);
__skb_queue_tail(&conn->data_q, skb);
+
+ flags &= ~ACL_START;
+ flags |= ACL_CONT;
do {
skb = list; list = list->next;
skb->dev = (void *) hdev;
bt_cb(skb)->pkt_type = HCI_ACLDATA_PKT;
- hci_add_acl_hdr(skb, conn->handle, flags | ACL_CONT);
+ hci_add_acl_hdr(skb, conn->handle, flags);
BT_DBG("%s frag %p len %d", hdev->name, skb, skb->len);
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index c791fcd..bedbb5f 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -362,13 +362,19 @@ static inline u8 l2cap_get_ident(struct l2cap_conn *conn)
static inline void l2cap_send_cmd(struct l2cap_conn *conn, u8 ident, u8 code, u16 len, void *data)
{
struct sk_buff *skb = l2cap_build_cmd(conn, code, ident, len, data);
+ u8 flags;
BT_DBG("code 0x%2.2x", code);
if (!skb)
return;
- hci_send_acl(conn->hcon, skb, 0);
+ if (lmp_no_flush_capable(conn->hcon->hdev))
+ flags = ACL_START_NO_FLUSH;
+ else
+ flags = ACL_START;
+
+ hci_send_acl(conn->hcon, skb, flags);
}
static inline void l2cap_send_sframe(struct l2cap_pinfo *pi, u16 control)
@@ -378,6 +384,7 @@ static inline void l2cap_send_sframe(struct l2cap_pinfo *pi, u16 control)
struct l2cap_conn *conn = pi->conn;
struct sock *sk = (struct sock *)pi;
int count, hlen = L2CAP_HDR_SIZE + 2;
+ u8 flags;
if (sk->sk_state != BT_CONNECTED)
return;
@@ -414,7 +421,12 @@ static inline void l2cap_send_sframe(struct l2cap_pinfo *pi, u16 control)
put_unaligned_le16(fcs, skb_put(skb, 2));
}
- hci_send_acl(pi->conn->hcon, skb, 0);
+ if (lmp_no_flush_capable(conn->hcon->hdev))
+ flags = ACL_START_NO_FLUSH;
+ else
+ flags = ACL_START;
+
+ hci_send_acl(pi->conn->hcon, skb, flags);
}
static inline void l2cap_send_rr_or_rnr(struct l2cap_pinfo *pi, u16 control)
@@ -900,6 +912,7 @@ static void l2cap_sock_init(struct sock *sk, struct sock *parent)
pi->sec_level = l2cap_pi(parent)->sec_level;
pi->role_switch = l2cap_pi(parent)->role_switch;
pi->force_reliable = l2cap_pi(parent)->force_reliable;
+ pi->flushable = l2cap_pi(parent)->flushable;
} else {
pi->imtu = L2CAP_DEFAULT_MTU;
pi->omtu = 0;
@@ -915,6 +928,7 @@ static void l2cap_sock_init(struct sock *sk, struct sock *parent)
pi->sec_level = BT_SECURITY_LOW;
pi->role_switch = 0;
pi->force_reliable = 0;
+ pi->flushable = BT_FLUSHABLE_OFF;
}
/* Default config options */
@@ -1450,10 +1464,17 @@ static void l2cap_drop_acked_frames(struct sock *sk)
static inline void l2cap_do_send(struct sock *sk, struct sk_buff *skb)
{
struct l2cap_pinfo *pi = l2cap_pi(sk);
+ struct hci_conn *hcon = pi->conn->hcon;
+ u16 flags;
BT_DBG("sk %p, skb %p len %d", sk, skb, skb->len);
- hci_send_acl(pi->conn->hcon, skb, 0);
+ if (!pi->flushable && lmp_no_flush_capable(hcon->hdev))
+ flags = ACL_START_NO_FLUSH;
+ else
+ flags = ACL_START;
+
+ hci_send_acl(hcon, skb, flags);
}
static void l2cap_streaming_send(struct sock *sk)
@@ -2098,6 +2119,28 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, ch
bt_sk(sk)->defer_setup = opt;
break;
+ case BT_FLUSHABLE:
+ if (get_user(opt, (u32 __user *) optval)) {
+ err = -EFAULT;
+ break;
+ }
+
+ if (opt > BT_FLUSHABLE_ON) {
+ err = -EINVAL;
+ break;
+ }
+
+ if (opt == BT_FLUSHABLE_OFF) {
+ struct l2cap_conn *conn = l2cap_pi(sk)->conn;
+ if (conn && !lmp_no_flush_capable(conn->hcon->hdev)) {
+ err = -EINVAL;
+ break;
+ }
+ }
+
+ l2cap_pi(sk)->flushable = opt;
+ break;
+
default:
err = -ENOPROTOOPT;
break;
@@ -2237,6 +2280,12 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname, ch
break;
+ case BT_FLUSHABLE:
+ if (put_user(l2cap_pi(sk)->flushable, (u32 __user *) optval))
+ err = -EFAULT;
+
+ break;
+
default:
err = -ENOPROTOOPT;
break;
@@ -4697,7 +4746,7 @@ static int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 fl
BT_DBG("conn %p len %d flags 0x%x", conn, skb->len, flags);
- if (flags & ACL_START) {
+ if (!(flags & ACL_CONT)) {
struct l2cap_hdr *hdr;
struct sock *sk;
u16 cid;
--
1.7.1
^ permalink raw reply related
* Re: [RFCv2] Bluetooth: Use non-flushable by default L2CAP data packets
From: Andrei Emeltchenko @ 2010-12-28 15:22 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: linux-bluetooth
In-Reply-To: <20101223020615.GA26284@vigoh>
Hi Gustavo,
On Thu, Dec 23, 2010 at 4:06 AM, Gustavo F. Padovan
<padovan@profusion.mobi> wrote:
> Hi Andrei,
>
> * Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2010-12-16 16:54:26 +0200]:
>
>> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
>>
>> Modification of Nick Pelly <npelly@google.com> patch.
>>
>> With Bluetooth 2.1 ACL packets can be flushable or non-flushable. This commit
>> makes ACL data packets non-flushable by default on compatible chipsets, and
>> adds the BT_FLUSHABLE socket option to explicitly request flushable ACL
>> data packets for a given L2CAP socket. This is useful for A2DP data which can
>> be safely discarded if it can not be delivered within a short time (while
>> other ACL data should not be discarded).
>>
>> Note that making ACL data flushable has no effect unless the automatic flush
>> timeout for that ACL link is changed from its default of 0 (infinite).
>>
>> Default packet types (for compatible chipsets):
>> Frame 34: 13 bytes on wire (104 bits), 13 bytes captured (104 bits)
>> Bluetooth HCI H4
>> Bluetooth HCI ACL Packet
>> .... 0000 0000 0010 = Connection Handle: 0x0002
>> ..00 .... .... .... = PB Flag: First Non-automatically Flushable Packet (0)
>> 00.. .... .... .... = BC Flag: Point-To-Point (0)
>> Data Total Length: 8
>> Bluetooth L2CAP Packet
>>
>> After setting BT_FLUSHABLE
>> (sock.setsockopt(274 /*SOL_BLUETOOTH*/, 8 /* BT_FLUSHABLE */, 1 /* flush */))
>> Frame 34: 13 bytes on wire (104 bits), 13 bytes captured (104 bits)
>> Bluetooth HCI H4
>> Bluetooth HCI ACL Packet
>> .... 0000 0000 0010 = Connection Handle: 0x0002
>> ..10 .... .... .... = PB Flag: First Automatically Flushable Packet (2)
>> 00.. .... .... .... = BC Flag: Point-To-Point (0)
>> Data Total Length: 8
>> Bluetooth L2CAP Packet
>>
>> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
>> ---
>> include/net/bluetooth/bluetooth.h | 5 ++++
>> include/net/bluetooth/hci.h | 2 +
>> include/net/bluetooth/hci_core.h | 1 +
>> include/net/bluetooth/l2cap.h | 2 +
>> net/bluetooth/hci_core.c | 6 +++-
>> net/bluetooth/l2cap.c | 48 ++++++++++++++++++++++++++++++++++--
>> 6 files changed, 59 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
>> index 0c5e725..ed7d775 100644
>> --- a/include/net/bluetooth/bluetooth.h
>> +++ b/include/net/bluetooth/bluetooth.h
>> @@ -64,6 +64,11 @@ struct bt_security {
>>
>> #define BT_DEFER_SETUP 7
>>
>> +#define BT_FLUSHABLE 8
>> +
>> +#define BT_FLUSHABLE_OFF 0
>> +#define BT_FLUSHABLE_ON 1
>> +
>> #define BT_INFO(fmt, arg...) printk(KERN_INFO "Bluetooth: " fmt "\n" , ## arg)
>> #define BT_ERR(fmt, arg...) printk(KERN_ERR "%s: " fmt "\n" , __func__ , ## arg)
>> #define BT_DBG(fmt, arg...) pr_debug("%s: " fmt "\n" , __func__ , ## arg)
>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>> index 29a7a8c..333d5cb 100644
>> --- a/include/net/bluetooth/hci.h
>> +++ b/include/net/bluetooth/hci.h
>> @@ -150,6 +150,7 @@ enum {
>> #define EDR_ESCO_MASK (ESCO_2EV3 | ESCO_3EV3 | ESCO_2EV5 | ESCO_3EV5)
>>
>> /* ACL flags */
>> +#define ACL_START_NO_FLUSH 0x00
>> #define ACL_CONT 0x01
>> #define ACL_START 0x02
>> #define ACL_ACTIVE_BCAST 0x04
>> @@ -193,6 +194,7 @@ enum {
>> #define LMP_EDR_ESCO_3M 0x40
>> #define LMP_EDR_3S_ESCO 0x80
>>
>> +#define LMP_NO_FLUSH 0x01
>
> Isn't this 0x40? As stated on Core v4.0 (Volume 2, part C, 3.3 Feature Mask
> Definition)
>
>> #define LMP_SIMPLE_PAIR 0x08
>>
>> /* Connection modes */
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> index 1992fac..9778bc8 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -456,6 +456,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
>> #define lmp_sniffsubr_capable(dev) ((dev)->features[5] & LMP_SNIFF_SUBR)
>> #define lmp_esco_capable(dev) ((dev)->features[3] & LMP_ESCO)
>> #define lmp_ssp_capable(dev) ((dev)->features[6] & LMP_SIMPLE_PAIR)
>> +#define lmp_no_flush_capable(dev) ((dev)->features[6] & LMP_NO_FLUSH)
>>
>> /* ----- HCI protocols ----- */
>> struct hci_proto {
>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>> index 7ad25ca..af35711 100644
>> --- a/include/net/bluetooth/l2cap.h
>> +++ b/include/net/bluetooth/l2cap.h
>> @@ -75,6 +75,7 @@ struct l2cap_conninfo {
>> #define L2CAP_LM_TRUSTED 0x0008
>> #define L2CAP_LM_RELIABLE 0x0010
>> #define L2CAP_LM_SECURE 0x0020
>> +#define L2CAP_LM_FLUSHABLE 0x0040
>
> Not using this flag in the code.
>
>>
>> /* L2CAP command codes */
>> #define L2CAP_COMMAND_REJ 0x01
>> @@ -327,6 +328,7 @@ struct l2cap_pinfo {
>> __u8 sec_level;
>> __u8 role_switch;
>> __u8 force_reliable;
>> + __u8 flushable;
>>
>> __u8 conf_req[64];
>> __u8 conf_len;
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index 51c61f7..c0d776b 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -1380,7 +1380,7 @@ void hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags)
>>
>> skb->dev = (void *) hdev;
>> bt_cb(skb)->pkt_type = HCI_ACLDATA_PKT;
>> - hci_add_acl_hdr(skb, conn->handle, flags | ACL_START);
>> + hci_add_acl_hdr(skb, conn->handle, flags);
>>
>> list = skb_shinfo(skb)->frag_list;
>> if (!list) {
>> @@ -1398,12 +1398,14 @@ void hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags)
>> spin_lock_bh(&conn->data_q.lock);
>>
>> __skb_queue_tail(&conn->data_q, skb);
>
> add an empty line here.
>
>> + flags &= ~ACL_START;
>> + flags |= ACL_CONT;
>> do {
>> skb = list; list = list->next;
>>
>> skb->dev = (void *) hdev;
>> bt_cb(skb)->pkt_type = HCI_ACLDATA_PKT;
>> - hci_add_acl_hdr(skb, conn->handle, flags | ACL_CONT);
>> + hci_add_acl_hdr(skb, conn->handle, flags);
>>
>> BT_DBG("%s frag %p len %d", hdev->name, skb, skb->len);
>>
>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
>> index c791fcd..efa60eb 100644
>> --- a/net/bluetooth/l2cap.c
>> +++ b/net/bluetooth/l2cap.c
>> @@ -362,13 +362,19 @@ static inline u8 l2cap_get_ident(struct l2cap_conn *conn)
>> static inline void l2cap_send_cmd(struct l2cap_conn *conn, u8 ident, u8 code, u16 len, void *data)
>> {
>> struct sk_buff *skb = l2cap_build_cmd(conn, code, ident, len, data);
>> + u8 flags;
>>
>> BT_DBG("code 0x%2.2x", code);
>>
>> if (!skb)
>> return;
>>
>> - hci_send_acl(conn->hcon, skb, 0);
>> + if (lmp_no_flush_capable(conn->hcon->hdev))
>> + flags = ACL_START_NO_FLUSH;
>> + else
>> + flags = ACL_START;
>> +
>> + hci_send_acl(conn->hcon, skb, flags);
>> }
>>
>> static inline void l2cap_send_sframe(struct l2cap_pinfo *pi, u16 control)
>> @@ -900,6 +906,7 @@ static void l2cap_sock_init(struct sock *sk, struct sock *parent)
>> pi->sec_level = l2cap_pi(parent)->sec_level;
>> pi->role_switch = l2cap_pi(parent)->role_switch;
>> pi->force_reliable = l2cap_pi(parent)->force_reliable;
>> + pi->flushable = l2cap_pi(parent)->flushable;
>> } else {
>> pi->imtu = L2CAP_DEFAULT_MTU;
>> pi->omtu = 0;
>> @@ -915,6 +922,7 @@ static void l2cap_sock_init(struct sock *sk, struct sock *parent)
>> pi->sec_level = BT_SECURITY_LOW;
>> pi->role_switch = 0;
>> pi->force_reliable = 0;
>> + pi->flushable = BT_FLUSHABLE_OFF;
>
> You need to check for the no_flush feature here, right?
I think at this stage we do not have hci and l2cap connection handlers
added to the socket.
We assign it in l2cap_chan_add later when making connect. Do you think
it makes sense to add the check there?
I am sending the latest patch leaving this unchanged for now.
Regards,
Andrei
>
>> }
>>
>> /* Default config options */
>> @@ -1450,10 +1458,17 @@ static void l2cap_drop_acked_frames(struct sock *sk)
>> static inline void l2cap_do_send(struct sock *sk, struct sk_buff *skb)
>> {
>> struct l2cap_pinfo *pi = l2cap_pi(sk);
>> + struct hci_conn *hcon = pi->conn->hcon;
>> + u16 flags;
>>
>> BT_DBG("sk %p, skb %p len %d", sk, skb, skb->len);
>>
>> - hci_send_acl(pi->conn->hcon, skb, 0);
>> + if (lmp_no_flush_capable(hcon->hdev) && !l2cap_pi(sk)->flushable)
>> + flags = ACL_START_NO_FLUSH;
>> + else
>> + flags = ACL_START;
>> +
>> + hci_send_acl(hcon, skb, flags);
>
> There is another call to hci_send_acl() in l2cap. It is in
> l2cap_send_sframe(), but I'm still wondering if we shall flush those packets
> or not in the case flushable was set. Now I'm thinking that don't.
>
> Then you need to add the same check in l2cap_send_cmd to l2cap_send_sframe().
>
>> }
>>
>> static void l2cap_streaming_send(struct sock *sk)
>> @@ -2045,6 +2060,7 @@ static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, char __us
>> static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, char __user *optval, unsigned int optlen)
>> {
>> struct sock *sk = sock->sk;
>> + struct hci_conn *hcon = l2cap_pi(sk)->conn->hcon;
>> struct bt_security sec;
>> int len, err = 0;
>> u32 opt;
>> @@ -2098,6 +2114,26 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, ch
>> bt_sk(sk)->defer_setup = opt;
>> break;
>>
>> + case BT_FLUSHABLE:
>> + if (get_user(opt, (u32 __user *) optval)) {
>> + err = -EFAULT;
>> + break;
>> + }
>> +
>> + if (opt > BT_FLUSHABLE_ON) {
>> + err = -EINVAL;
>> + break;
>> + }
>> +
>> + if (opt == BT_FLUSHABLE_OFF &&
>> + !lmp_no_flush_capable(hcon->hdev)) {
>
> I'm not really happy with !lmp_no_flush... 2 negatives in the same place, but
> that is the feature name. So we can't do too much here. Btw, the "No" in the
> feature name is making my brain hurt. ;)
>
> regards,
>
> --
> Gustavo F. Padovan
> http://profusion.mobi
>
^ permalink raw reply
* Re: [PATCHv3] Bluetooth: Use non-flushable by default L2CAP data packets
From: Andrei Emeltchenko @ 2010-12-28 15:31 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1293549367-2516-2-git-send-email-Andrei.Emeltchenko.news@gmail.com>
On Tue, Dec 28, 2010 at 5:16 PM, Emeltchenko Andrei
<Andrei.Emeltchenko.news@gmail.com> wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
>
> Modification of Nick Pelly <npelly@google.com> patch.
>
> With Bluetooth 2.1 ACL packets can be flushable or non-flushable. This commit
> makes ACL data packets non-flushable by default on compatible chipsets, and
> adds the BT_FLUSHABLE socket option to explicitly request flushable ACL
> data packets for a given L2CAP socket. This is useful for A2DP data which can
> be safely discarded if it can not be delivered within a short time (while
> other ACL data should not be discarded).
>
> Note that making ACL data flushable has no effect unless the automatic flush
> timeout for that ACL link is changed from its default of 0 (infinite).
>
> Default packet types (for compatible chipsets):
> Frame 34: 13 bytes on wire (104 bits), 13 bytes captured (104 bits)
> Bluetooth HCI H4
> Bluetooth HCI ACL Packet
> .... 0000 0000 0010 = Connection Handle: 0x0002
> ..00 .... .... .... = PB Flag: First Non-automatically Flushable Packet (0)
> 00.. .... .... .... = BC Flag: Point-To-Point (0)
> Data Total Length: 8
> Bluetooth L2CAP Packet
>
> After setting BT_FLUSHABLE
> (sock.setsockopt(274 /*SOL_BLUETOOTH*/, 8 /* BT_FLUSHABLE */, 1 /* flush */))
> Frame 34: 13 bytes on wire (104 bits), 13 bytes captured (104 bits)
> Bluetooth HCI H4
> Bluetooth HCI ACL Packet
> .... 0000 0000 0010 = Connection Handle: 0x0002
> ..10 .... .... .... = PB Flag: First Automatically Flushable Packet (2)
> 00.. .... .... .... = BC Flag: Point-To-Point (0)
> Data Total Length: 8
> Bluetooth L2CAP Packet
>
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> ---
> include/net/bluetooth/bluetooth.h | 5 +++
> include/net/bluetooth/hci.h | 2 +
> include/net/bluetooth/hci_core.h | 1 +
> include/net/bluetooth/l2cap.h | 1 +
> net/bluetooth/hci_core.c | 7 +++-
> net/bluetooth/l2cap.c | 57 ++++++++++++++++++++++++++++++++++--
> 6 files changed, 67 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index 0c5e725..ed7d775 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -64,6 +64,11 @@ struct bt_security {
>
> #define BT_DEFER_SETUP 7
>
> +#define BT_FLUSHABLE 8
> +
> +#define BT_FLUSHABLE_OFF 0
> +#define BT_FLUSHABLE_ON 1
> +
> #define BT_INFO(fmt, arg...) printk(KERN_INFO "Bluetooth: " fmt "\n" , ## arg)
> #define BT_ERR(fmt, arg...) printk(KERN_ERR "%s: " fmt "\n" , __func__ , ## arg)
> #define BT_DBG(fmt, arg...) pr_debug("%s: " fmt "\n" , __func__ , ## arg)
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 29a7a8c..5d033dc 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -150,6 +150,7 @@ enum {
> #define EDR_ESCO_MASK (ESCO_2EV3 | ESCO_3EV3 | ESCO_2EV5 | ESCO_3EV5)
>
> /* ACL flags */
> +#define ACL_START_NO_FLUSH 0x00
> #define ACL_CONT 0x01
> #define ACL_START 0x02
> #define ACL_ACTIVE_BCAST 0x04
> @@ -194,6 +195,7 @@ enum {
> #define LMP_EDR_3S_ESCO 0x80
>
> #define LMP_SIMPLE_PAIR 0x08
> +#define LMP_NO_FLUSH 0x40
>
> /* Connection modes */
> #define HCI_CM_ACTIVE 0x0000
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index a29feb0..59135a6 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -457,6 +457,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
> #define lmp_sniffsubr_capable(dev) ((dev)->features[5] & LMP_SNIFF_SUBR)
> #define lmp_esco_capable(dev) ((dev)->features[3] & LMP_ESCO)
> #define lmp_ssp_capable(dev) ((dev)->features[6] & LMP_SIMPLE_PAIR)
> +#define lmp_no_flush_capable(dev) ((dev)->features[6] & LMP_NO_FLUSH)
>
> /* ----- HCI protocols ----- */
> struct hci_proto {
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 7ad25ca..7f88a87 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -327,6 +327,7 @@ struct l2cap_pinfo {
> __u8 sec_level;
> __u8 role_switch;
> __u8 force_reliable;
> + __u8 flushable;
>
> __u8 conf_req[64];
> __u8 conf_len;
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 8b602d8..3c0990d 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1391,7 +1391,7 @@ void hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags)
>
> skb->dev = (void *) hdev;
> bt_cb(skb)->pkt_type = HCI_ACLDATA_PKT;
> - hci_add_acl_hdr(skb, conn->handle, flags | ACL_START);
> + hci_add_acl_hdr(skb, conn->handle, flags);
>
> list = skb_shinfo(skb)->frag_list;
> if (!list) {
> @@ -1409,12 +1409,15 @@ void hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags)
> spin_lock_bh(&conn->data_q.lock);
>
> __skb_queue_tail(&conn->data_q, skb);
> +
> + flags &= ~ACL_START;
> + flags |= ACL_CONT;
> do {
> skb = list; list = list->next;
>
> skb->dev = (void *) hdev;
> bt_cb(skb)->pkt_type = HCI_ACLDATA_PKT;
> - hci_add_acl_hdr(skb, conn->handle, flags | ACL_CONT);
> + hci_add_acl_hdr(skb, conn->handle, flags);
>
> BT_DBG("%s frag %p len %d", hdev->name, skb, skb->len);
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index c791fcd..bedbb5f 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -362,13 +362,19 @@ static inline u8 l2cap_get_ident(struct l2cap_conn *conn)
> static inline void l2cap_send_cmd(struct l2cap_conn *conn, u8 ident, u8 code, u16 len, void *data)
> {
> struct sk_buff *skb = l2cap_build_cmd(conn, code, ident, len, data);
> + u8 flags;
>
> BT_DBG("code 0x%2.2x", code);
>
> if (!skb)
> return;
>
> - hci_send_acl(conn->hcon, skb, 0);
> + if (lmp_no_flush_capable(conn->hcon->hdev))
> + flags = ACL_START_NO_FLUSH;
> + else
> + flags = ACL_START;
> +
> + hci_send_acl(conn->hcon, skb, flags);
> }
>
> static inline void l2cap_send_sframe(struct l2cap_pinfo *pi, u16 control)
> @@ -378,6 +384,7 @@ static inline void l2cap_send_sframe(struct l2cap_pinfo *pi, u16 control)
> struct l2cap_conn *conn = pi->conn;
> struct sock *sk = (struct sock *)pi;
> int count, hlen = L2CAP_HDR_SIZE + 2;
> + u8 flags;
>
> if (sk->sk_state != BT_CONNECTED)
> return;
> @@ -414,7 +421,12 @@ static inline void l2cap_send_sframe(struct l2cap_pinfo *pi, u16 control)
> put_unaligned_le16(fcs, skb_put(skb, 2));
> }
>
> - hci_send_acl(pi->conn->hcon, skb, 0);
> + if (lmp_no_flush_capable(conn->hcon->hdev))
> + flags = ACL_START_NO_FLUSH;
> + else
> + flags = ACL_START;
> +
> + hci_send_acl(pi->conn->hcon, skb, flags);
> }
>
> static inline void l2cap_send_rr_or_rnr(struct l2cap_pinfo *pi, u16 control)
> @@ -900,6 +912,7 @@ static void l2cap_sock_init(struct sock *sk, struct sock *parent)
> pi->sec_level = l2cap_pi(parent)->sec_level;
> pi->role_switch = l2cap_pi(parent)->role_switch;
> pi->force_reliable = l2cap_pi(parent)->force_reliable;
> + pi->flushable = l2cap_pi(parent)->flushable;
> } else {
> pi->imtu = L2CAP_DEFAULT_MTU;
> pi->omtu = 0;
> @@ -915,6 +928,7 @@ static void l2cap_sock_init(struct sock *sk, struct sock *parent)
> pi->sec_level = BT_SECURITY_LOW;
> pi->role_switch = 0;
> pi->force_reliable = 0;
> + pi->flushable = BT_FLUSHABLE_OFF;
> }
>
> /* Default config options */
> @@ -1450,10 +1464,17 @@ static void l2cap_drop_acked_frames(struct sock *sk)
> static inline void l2cap_do_send(struct sock *sk, struct sk_buff *skb)
> {
> struct l2cap_pinfo *pi = l2cap_pi(sk);
> + struct hci_conn *hcon = pi->conn->hcon;
> + u16 flags;
>
> BT_DBG("sk %p, skb %p len %d", sk, skb, skb->len);
>
> - hci_send_acl(pi->conn->hcon, skb, 0);
> + if (!pi->flushable && lmp_no_flush_capable(hcon->hdev))
> + flags = ACL_START_NO_FLUSH;
> + else
> + flags = ACL_START;
> +
> + hci_send_acl(hcon, skb, flags);
> }
>
> static void l2cap_streaming_send(struct sock *sk)
> @@ -2098,6 +2119,28 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, ch
> bt_sk(sk)->defer_setup = opt;
> break;
>
> + case BT_FLUSHABLE:
> + if (get_user(opt, (u32 __user *) optval)) {
> + err = -EFAULT;
> + break;
> + }
> +
> + if (opt > BT_FLUSHABLE_ON) {
> + err = -EINVAL;
> + break;
> + }
> +
> + if (opt == BT_FLUSHABLE_OFF) {
> + struct l2cap_conn *conn = l2cap_pi(sk)->conn;
> + if (conn && !lmp_no_flush_capable(conn->hcon->hdev)) {
or maybe:
if (!conn ||
!lmp_no_flush_capable(conn->hcon->hdev)) {
> + err = -EINVAL;
> + break;
> + }
> + }
> +
> + l2cap_pi(sk)->flushable = opt;
> + break;
> +
> default:
> err = -ENOPROTOOPT;
> break;
> @@ -2237,6 +2280,12 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname, ch
>
> break;
>
> + case BT_FLUSHABLE:
> + if (put_user(l2cap_pi(sk)->flushable, (u32 __user *) optval))
> + err = -EFAULT;
> +
> + break;
> +
> default:
> err = -ENOPROTOOPT;
> break;
> @@ -4697,7 +4746,7 @@ static int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 fl
>
> BT_DBG("conn %p len %d flags 0x%x", conn, skb->len, flags);
>
> - if (flags & ACL_START) {
> + if (!(flags & ACL_CONT)) {
> struct l2cap_hdr *hdr;
> struct sock *sk;
> u16 cid;
> --
> 1.7.1
>
> --
> 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
* [PATCH] Fix crash due to misplaced parameter in emit_device_found() call
From: Anderson Lizardo @ 2010-12-28 16:52 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
To avoid related mistakes, dev->uuid_count was shortened to uuid_count
and kept on the same line.
---
Note that there is a 82-column line on this patch. I chose to put it
on the same line to avoid a similar mistake in future.
---
src/adapter.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index bfc2b8b..816d842 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2841,9 +2841,9 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
"RSSI", DBUS_TYPE_INT16, &rssi,
"Name", DBUS_TYPE_STRING, &dev->name,
"Paired", DBUS_TYPE_BOOLEAN, &paired,
- "UUIDs", DBUS_TYPE_ARRAY, &dev->uuids,
"Broadcaster", DBUS_TYPE_BOOLEAN, &broadcaster,
- dev->uuid_count, NULL);
+ "UUIDs", DBUS_TYPE_ARRAY, &dev->uuids, uuid_count,
+ NULL);
return;
}
@@ -2867,7 +2867,7 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
"Alias", DBUS_TYPE_STRING, &alias,
"LegacyPairing", DBUS_TYPE_BOOLEAN, &dev->legacy,
"Paired", DBUS_TYPE_BOOLEAN, &paired,
- "UUIDs", DBUS_TYPE_ARRAY, &dev->uuids, dev->uuid_count,
+ "UUIDs", DBUS_TYPE_ARRAY, &dev->uuids, uuid_count,
NULL);
g_free(alias);
--
1.7.0.4
^ permalink raw reply related
* [PATCH v3] Fix leak of EIR data if RSSI does not change
From: Anderson Lizardo @ 2010-12-28 18:10 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1293473827-26418-1-git-send-email-anderson.lizardo@openbossa.org>
If RSSI value does not change, memory used by parsed EIR data would leak
because it would not be assigned to the remote_dev_info structure.
---
src/adapter.c | 20 ++++++++++++++------
src/adapter.h | 6 +++---
src/event.c | 49 ++++++++++++++++++++++++++++---------------------
3 files changed, 45 insertions(+), 30 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 4af11bc..b7a65dc 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2923,10 +2923,18 @@ static void remove_same_uuid(gpointer data, gpointer user_data)
}
}
+static void dev_prepend_uuid(gpointer data, gpointer user_data)
+{
+ struct remote_dev_info *dev = user_data;
+ char *new_uuid = data;
+
+ dev->services = g_slist_prepend(dev->services, g_strdup(new_uuid));
+}
+
void adapter_update_device_from_info(struct btd_adapter *adapter,
- bdaddr_t bdaddr, int8_t rssi,
- uint8_t evt_type, char *name,
- GSList *services, uint8_t flags)
+ bdaddr_t bdaddr, int8_t rssi,
+ uint8_t evt_type, const char *name,
+ GSList *services, uint8_t flags)
{
struct remote_dev_info *dev;
gboolean new_dev;
@@ -2945,13 +2953,13 @@ void adapter_update_device_from_info(struct btd_adapter *adapter,
(GCompareFunc) dev_rssi_cmp);
g_slist_foreach(services, remove_same_uuid, dev);
- dev->services = g_slist_concat(dev->services, services);
+ g_slist_foreach(services, dev_prepend_uuid, dev);
dev->flags = flags;
if (name) {
g_free(dev->name);
- dev->name = name;
+ dev->name = g_strdup(name);
}
/* FIXME: check if other information was changed before emitting the
@@ -2989,7 +2997,7 @@ void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
(GCompareFunc) dev_rssi_cmp);
g_slist_foreach(services, remove_same_uuid, dev);
- dev->services = g_slist_concat(dev->services, services);
+ g_slist_foreach(services, dev_prepend_uuid, dev);
adapter_emit_device_found(adapter, dev);
}
diff --git a/src/adapter.h b/src/adapter.h
index 1fbc221..a655cdb 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -135,9 +135,9 @@ int adapter_get_discover_type(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,
- bdaddr_t bdaddr, int8_t rssi,
- uint8_t evt_type, char *name,
- GSList *services, uint8_t flags);
+ bdaddr_t bdaddr, int8_t rssi,
+ uint8_t evt_type, const 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 178cea8..b1054d7 100644
--- a/src/event.c
+++ b/src/event.c
@@ -431,6 +431,13 @@ static int parse_eir_data(struct eir_data *eir, uint8_t *eir_data,
return 0;
}
+static void free_eir_data(struct eir_data *eir)
+{
+ g_slist_foreach(eir->services, (GFunc) g_free, NULL);
+ g_slist_free(eir->services);
+ g_free(eir->name);
+}
+
void btd_event_advertising_report(bdaddr_t *local, le_advertising_info *info)
{
struct btd_adapter *adapter;
@@ -455,6 +462,8 @@ void btd_event_advertising_report(bdaddr_t *local, le_advertising_info *info)
adapter_update_device_from_info(adapter, info->bdaddr, rssi,
info->evt_type, eir_data.name,
eir_data.services, eir_data.flags);
+
+ free_eir_data(&eir_data);
}
static void update_lastseen(bdaddr_t *sba, bdaddr_t *dba)
@@ -491,6 +500,7 @@ void btd_event_device_found(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
int state, err;
dbus_bool_t legacy;
unsigned char features[8];
+ const char *dev_name;
ba2str(local, local_addr);
ba2str(peer, peer_addr);
@@ -517,11 +527,6 @@ void btd_event_device_found(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
adapter_set_state(adapter, state);
}
- 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);
-
/* the inquiry result can be triggered by NON D-Bus client */
if (adapter_get_discover_type(adapter) & DISC_RESOLVNAME &&
adapter_has_discov_sessions(adapter))
@@ -547,28 +552,30 @@ void btd_event_device_found(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
} else
legacy = TRUE;
- if (eir_data.name) {
+ 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);
+
+ /* Complete EIR names are always used. Shortened EIR names are only
+ * used if there is no name already in storage. */
+ dev_name = name;
+ if (eir_data.name != NULL) {
if (eir_data.name_complete) {
write_device_name(local, peer, eir_data.name);
name_status = NAME_NOT_REQUIRED;
-
- if (name)
- g_free(name);
-
- name = eir_data.name;
- } else {
- if (name)
- free(eir_data.name);
- else
- name = eir_data.name;
- }
+ dev_name = eir_data.name;
+ } else if (name == NULL)
+ dev_name = eir_data.name;
}
- adapter_update_found_devices(adapter, peer, rssi, class, name, alias,
- legacy, eir_data.services, name_status);
+ adapter_update_found_devices(adapter, peer, rssi, class, dev_name,
+ alias, legacy, eir_data.services,
+ name_status);
- g_free(name);
- g_free(alias);
+ free_eir_data(&eir_data);
+ free(name);
+ free(alias);
}
void btd_event_set_legacy_pairing(bdaddr_t *local, bdaddr_t *peer,
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH v2] Fix leak of EIR data if RSSI does not change
From: Anderson Lizardo @ 2010-12-28 18:16 UTC (permalink / raw)
To: Anderson Lizardo, linux-bluetooth
In-Reply-To: <AANLkTi=dsEwvjAOGM7eFdDDZSXUrPuA5wjQw8VcWH8N5@mail.gmail.com>
Hi Johan,
On Mon, Dec 27, 2010 at 6:24 PM, Anderson Lizardo
<anderson.lizardo@openbossa.org> wrote:
> On Mon, Dec 27, 2010 at 5:48 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
>> Beware of words like "also" and "and" in commit messages. Often they are
>> a good indication that the patch shold be split into multiple ones.
>> Though in this case I can't really make up my mind if it's fine since
>> the changes are so strongly related. I'll leave it up to you to think
>> about it and decide if you can cleanly split this into two patches or
>> not.
>
> I can split into two patches without problem.
I just sent a v3 of this patch, which I hope fix all issues. It is
bigger than previous versions, but I assure it is solely to fix the
leak of EIR data. The other benefits are just "side effects":
* That if() clause which handles EIR name has been simplified, by
removing free()/g_free()
* I avoid passing ownership of memory allocated in src/event.c to
src/adapter.c, which would potentially become memory leaks if not
managed carefully.
I hope these side effects are not of a big issue to be kept in a single patch.
Regards,
--
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [PATCH] Fix crash due to misplaced parameter in emit_device_found() call
From: Johan Hedberg @ 2010-12-28 18:42 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1293555150-9979-1-git-send-email-anderson.lizardo@openbossa.org>
Hi,
On Tue, Dec 28, 2010, Anderson Lizardo wrote:
> To avoid related mistakes, dev->uuid_count was shortened to uuid_count
> and kept on the same line.
> ---
> Note that there is a 82-column line on this patch. I chose to put it
> on the same line to avoid a similar mistake in future.
> ---
> src/adapter.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
The patch has been pushed upstream. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH v3] Fix leak of EIR data if RSSI does not change
From: Johan Hedberg @ 2010-12-28 18:49 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1293559820-10738-1-git-send-email-anderson.lizardo@openbossa.org>
Hi Lizardo,
On Tue, Dec 28, 2010, Anderson Lizardo wrote:
> If RSSI value does not change, memory used by parsed EIR data would leak
> because it would not be assigned to the remote_dev_info structure.
> ---
> src/adapter.c | 20 ++++++++++++++------
> src/adapter.h | 6 +++---
> src/event.c | 49 ++++++++++++++++++++++++++++---------------------
> 3 files changed, 45 insertions(+), 30 deletions(-)
Thanks. This latest version looks fine to me. The patch has been pushed
upstream.
Johan
^ permalink raw reply
* Re: [RFC 1/2] Add g_attrib_send_seq()
From: Vinicius Costa Gomes @ 2010-12-28 21:48 UTC (permalink / raw)
To: Brian Gix; +Cc: Claudio Takahasi, rshaffer, padovan, linux-bluetooth
In-Reply-To: <1293059620.7281.43.camel@sealablnx02.qualcomm.com>
Hi Brian,
First of all, sorry for the delay.
On 15:13 Wed 22 Dec, Brian Gix wrote:
> Thanks Claudio,
>
> On Wed, 2010-12-22 at 19:29 -0300, Claudio Takahasi wrote:
> > Hi Brian,
> >
> > On Fri, Dec 17, 2010 at 7:48 PM, Brian Gix <bgix@codeaurora.org> wrote:
> > > 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;
> > Intialization is not necessary.
>
> There is a path to it's usage (a "goto done") just after
> g_io_channel_read_chars call.
>
> >
> > >
> > > 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);
> > > +
> > Any chance to change the logic to avoid duplicated verification?
> > No mater the value of "compound" wake_up_sender() is always called. Is
> > the order important?
>
> This is probably the ugliest part of my additions.
>
> Here is the problem:
> I wanted to delay the call to wake_up_sender() until *after* the
> invocation of the cmd->func() callback, because if the the subsequent
> ATT command is to be sent next, it needs to be added to the head of the
> queue before the sender is woken up. wake_up_sender calls
> g_io_add_watch_full(), and I am unsure how the process threading works
> here, so I don't know if received_data() is going to be preempted (and
> send the current queue head) by that call.
It is really simple, it was not meant to work on multi-threaded enviroments.
receeived_data() is going to be called after control is back to the mainloop.
It is not meant to be reentrant.
>
> I do see that the call to wake_up_sender is called by the g_attrib_send*
> function, but only if the addition was to a previously empty queue (so
> that the count is now 1). It appeared to me that care was taken by the
> original coder to make sure wake_up_sender() was called exactly once for
> each ATT command that was queued, and since only a single ATT command
> can be outstanding at a time, I believe this is the correct
> functionality.
I think that Claudio was referring just to the changes that you made to
received_data(). And considering what I said above (that it's not meant to be
thread-safe) I think that received_data could stay unchanged.
And g_attrib_send_seq() could have a bug when the command at the head was
already sent but is waiting for a response. If the command is always added to
the head of the queue, could it be so the order of the command of a compound
procedure is inverted?
And taking a closer look at the spec, it was clear to me that only a higher
level profile (above GATT) is able to know that a characteristic needs to be
read by using the "Read Long Characteristic Value" procedure -- which I think is
what brought this discussion, right? or you already have another need for
these procedures? -- Which gives me the impression that this should
be dealt by the profile implementation. Which gives us more time to think about
how to implement this correctly ;-) in case the need arrise.
>
> Also, there is the problem that just because a GATT procedure *may* be
> compound, there is no guarantee that it *will* need to queue an
> additional ATT command. In the read characteristic case, for instance,
> if the result is less than 22 bytes, it will not end up being truly
> compound at all. However, we do not know that until we have processed
> the first result.
>
> I could get around the "two code blocks that do almost the same thing"
> problem by the following changes:
>
> 1. In g_attrib_send_seq(), pull "if length == 1 then wake_up_sender"
> block out one level, so that it is invoked for both the
> push_head and push_tail cases.
>
> 2. In received_data(), Check the queue member count prior to invoking
> cmd->func, and then calling wake_up_sender afterwards if the
> prior count was non-zero. We can't check the current count
> at that point, because wake_up_sender may have already been
> called in g_attrib_send_seq.
>
> 3. Then, we can totally eliminate the "compound" local var, and
> structure member, making the whole thing cleaner.
>
>
> Should I try that?
>
> >
> > > }
> > >
> > > 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;
> > > +
> > Is it necessary to check if queue is NULL?
>
> I don't know. What happens if g_queue_push_head or g_queue_push_tail is
> passed a NULL pointer for the queue? I noticed that it is checked by
> g_attrib_cancel, and was unsure as to why it would be different.
>
It would not crash, but I think that GLib would give an warning. Doing the
check is correct.
> >
> > > 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);
> > Coding style issue here.
>
> What is the issue? I saw in g_attrib_new() that a return of a call to
> g_attrib_ref was tail-chain-returned.
I think it was more about indentation, something like this looks better:
return g_attrib_send_seq(attrib, FALSE, 0, opcode, pdu, len, func,
user_data, notify);
>
> >
> > Claudio
>
> Thanks,
>
> --
> 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
Cheers,
--
Vinicius
^ permalink raw reply
* Support for Bluetooth 3.0 HS in BlueZ.
From: Gregory Greenman @ 2010-12-29 15:00 UTC (permalink / raw)
To: linux-bluetooth
Hello all,
Does anybody know if there is support for the High Speed feature of
the Bluetooth 3.0 spec in BlueZ? If not, is there anybody working on
that already?
Thank you,
Gregory
^ permalink raw reply
* Bluez DOA after reboot(s)
From: gene heskett @ 2010-12-29 15:23 UTC (permalink / raw)
To: linux-bluetooth
Greetings;
I had a Broadcom button linked to an a7 brand eb-101 that worked great,
for about a week, then rebooted to a newer 2.6.37-rc8 kernel where it
fails. Rebooting to the old kernel, 2.6.36.1, now fails also.
Starting blueman-manager from a root terminal gets this return.
[root@coyote ~]# blueman-manager
Loading configuration plugins
Using gconf config backend
_________
on_bluez_name_owner_changed (/usr/bin/blueman-manager:104)
org.bluez owner changed to
at which point a small window opens and advises me
Connection to Bluez failed
and only has one option, close.
I have looked at line 104, and its apparent that 'owner' is not set, hence
the apparent null after 'to' above. But I don't know enough about python
to even be dangerous.
Running kde-4.5.latest here, on 32 bit pclos-2010 uptodate except for the
newer, badly broken for amanda's usage, tar.
Fixes anyone, please?
Where is this bluez started in the boot sequence?
I have looked at the logs (dmesg etc) and the device is discovered just
fine. Everything for bluetooth is enabled in the kernel's .config.
>From /var/log/messages
Dec 29 06:30:05 coyote klogd: [ 6.234467] usb 1-10.4.4: new full speed USB device using ehci_hcd and
address 14
Dec 29 06:30:05 coyote klogd: [ 6.327964] usb 1-10.4.4: New USB device found, idVendor=0a5c, idProduct=4500
Dec 29 06:30:05 coyote klogd: [ 6.334330] usb 1-10.4.4: New USB device strings: Mfr=1, Product=2,
SerialNumber=0
Dec 29 06:30:05 coyote klogd: [ 6.340660] usb 1-10.4.4: Product: BCM2046B1
Dec 29 06:30:05 coyote klogd: [ 6.346950] usb 1-10.4.4: Manufacturer: Broadcom
Dec 29 06:30:05 coyote klogd: [ 6.647365] usb 1-10.4.4.1: new full speed USB device using ehci_hcd and
address 16
Dec 29 06:30:05 coyote klogd: [ 6.748240] usb 1-10.4.4.1: New USB device found, idVendor=0a5c,
idProduct=4502
Dec 29 06:30:05 coyote klogd: [ 6.748244] usb 1-10.4.4.1: New USB device strings: Mfr=0, Product=0,
SerialNumber=0
Dec 29 06:30:05 coyote klogd: [ 6.837371] usb 1-10.4.4.2: new full speed USB device using ehci_hcd and
address 17
Dec 29 06:30:05 coyote klogd: [ 6.924252] usb 1-10.4.4.2: New USB device found, idVendor=0a5c,
idProduct=4503
Dec 29 06:30:05 coyote klogd: [ 6.924257] usb 1-10.4.4.2: New USB device strings: Mfr=0, Product=0,
SerialNumber=0
Dec 29 06:30:05 coyote klogd: [ 6.997390] usb 1-10.4.4.3: new full speed USB device using ehci_hcd and
address 18
Dec 29 06:30:05 coyote klogd: [ 7.089511] usb 1-10.4.4.3: New USB device found, idVendor=0a5c,
idProduct=2148
Dec 29 06:30:05 coyote klogd: [ 7.089515] usb 1-10.4.4.3: New USB device strings: Mfr=1, Product=2,
SerialNumber=3
Dec 29 06:30:05 coyote klogd: [ 7.089518] usb 1-10.4.4.3: Product: BCM92046DG-CL1ROM
Dec 29 06:30:05 coyote klogd: [ 7.089520] usb 1-10.4.4.3: Manufacturer: Broadcom Corp
Dec 29 06:30:05 coyote klogd: [ 7.089531] usb 1-10.4.4.3: SerialNumber: 000272A176A4
But now bluez isn't running, and of course /dev/rfcomm0 is not being
created. Even on the 2.6.36.1 kernel it was running well with yesterday.
Thank you.
--
Cheers, Gene
"There are four boxes to be used in defense of liberty:
soap, ballot, jury, and ammo. Please use in that order."
-Ed Howdershelt (Author)
He looked at me as if I were a side dish he hadn't ordered.
-- Ring Lardner
^ 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