* [PATCH v3 1/3] Implements hci_reassembly to reassemble Rx packets
From: suraj @ 2010-06-09 13:05 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, Luis.Rodriguez, Jothikumar.Mothilal
Implements feature to reassemble received HCI frames from any input stream.
Signed-off-by: Suraj Sumangala <suraj@atheros.com>
---
net/bluetooth/hci_core.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 115 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 5e83f8e..631a185 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1030,6 +1030,121 @@ int hci_recv_frame(struct sk_buff *skb)
}
EXPORT_SYMBOL(hci_recv_frame);
+static int hci_reassembly(struct hci_dev *hdev, int type, void *data,
+ int count, struct sk_buff **skb_ptr, int *remain)
+{
+ int len = 0;
+ int header_len = 0;
+ struct sk_buff *skb = *skb_ptr;
+ struct { __u8 pkt_type; int expect; } *scb;
+
+ *remain = count;
+
+ if (type < HCI_ACLDATA_PKT || type > HCI_EVENT_PKT)
+ return -EILSEQ;
+
+ if (!skb) {
+
+ switch (type) {
+ case HCI_ACLDATA_PKT:
+ len = HCI_MAX_FRAME_SIZE;
+ header_len = HCI_ACL_HDR_SIZE;
+ break;
+ case HCI_EVENT_PKT:
+ len = HCI_MAX_EVENT_SIZE;
+ header_len = HCI_EVENT_HDR_SIZE;
+ break;
+ case HCI_SCODATA_PKT:
+ len = HCI_MAX_SCO_SIZE;
+ header_len = HCI_SCO_HDR_SIZE;
+ break;
+ }
+
+ skb = bt_skb_alloc(len, GFP_ATOMIC);
+
+ if (!skb)
+ return -ENOMEM;
+
+ scb = (void *) skb->cb;
+ scb->expect = header_len;
+ scb->pkt_type = (__u8)type;
+
+ skb->dev = (void *) hdev;
+ *skb_ptr = skb;
+
+ }
+
+ while (count) {
+
+ scb = (void *) skb->cb;
+ len = min(scb->expect, count);
+
+ memcpy(skb_put(skb, len), data, len);
+
+ count -= len;
+ data += len;
+ scb->expect -= len;
+ *remain = count;
+
+ switch (type) {
+ case HCI_EVENT_PKT:
+ if (skb->len == HCI_EVENT_HDR_SIZE) {
+ struct hci_event_hdr *h = hci_event_hdr(skb);
+ scb->expect = h->plen;
+
+ if (skb_tailroom(skb) < scb->expect) {
+ kfree_skb(skb);
+ *skb_ptr = NULL;
+
+ return -ENOMEM;
+ }
+ }
+ break;
+
+ case HCI_ACLDATA_PKT:
+ if (skb->len == HCI_ACL_HDR_SIZE) {
+ struct hci_acl_hdr *h = hci_acl_hdr(skb);
+ scb->expect = __le16_to_cpu(h->dlen);
+
+ if (skb_tailroom(skb) < scb->expect) {
+ kfree_skb(skb);
+ *skb_ptr = NULL;
+
+ return -ENOMEM;
+ }
+ }
+ break;
+
+ case HCI_SCODATA_PKT:
+ if (skb->len == HCI_SCO_HDR_SIZE) {
+ struct hci_sco_hdr *h = hci_sco_hdr(skb);
+ scb->expect = h->dlen;
+
+ if (skb_tailroom(skb) < scb->expect) {
+ kfree_skb(skb);
+ *skb_ptr = NULL;
+
+ return -ENOMEM;
+ }
+ }
+ break;
+ }
+
+ if (scb->expect == 0) {
+
+ /* Complete frame */
+ bt_cb(skb)->pkt_type = type;
+ hci_recv_frame(skb);
+
+ *skb_ptr = NULL;
+
+ return 0;
+ }
+
+ }
+
+ return 0;
+}
/* Receive packet type fragment */
#define __reassembly(hdev, type) ((hdev)->reassembly[(type) - 2])
--
1.7.0
^ permalink raw reply related
* Re: L2CAP ERTM compatibility problem
From: Mat Martineau @ 2010-06-08 21:13 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: Nathan Holstein, linux-bluetooth
In-Reply-To: <20100608205125.GA24245@vigoh>
On Tue, 8 Jun 2010, Gustavo F. Padovan wrote:
> Hi Nathan,
>
> * Nathan Holstein <ngh@isomerica.net> [2010-06-08 13:20:37 -0400]:
>
>> I'm currently testing an older set of Gustavo's ERTM fixes backported
>> onto a 2.6.32 kernel. I'm seeing a compatibility problem while
>> attempting to establish a connection using ERTM. This appears to be
>> caused by commit 277ffbe in Gustavo's testing tree which adds minimum
>> length checks to received ERTM and Streaming Mode packets.
>>
>> Here's the incoming packet (hcidump -Xt):
>>
>> 1276011301.453633 > ACL data: handle 12 flags 0x02 dlen 11
>> L2CAP(d): cid 0x0040 len 7 [psm 0]
>> 0000: 02 01 07 00 06 2e 0b
>>
>> This appears to be a valid packet containing a three byte sequence which
>> should be passed up to the application. However, the check added by
>> commit 277ffbe causes the kernel to disconnect the socket since the data
>> length is less than 4.
>>
>> I don't see the point of the minimum length at this point in the code.
>> The len variable already subtracts for the control, FCS and SAR fields
>> if necessary. It seems to me that the remaining length of the skb is
>> purely socket data, and as such should be passed on to the application.
>
> The checks comes from the section 3.3.7 of the L2CAP spec.
>
>>
>> Here's my proposed fix:
>>
>> @@ -4189,19 +4193,23 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
>>
>>
>> if (__is_iframe(control)) {
>> - if (len < 4) {
>> + if (len < 0) {
>> l2cap_send_disconn_req(pi->conn, sk);
>> goto drop;
>> }
>> @@ -4222,7 +4230,7 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
>> if (pi->fcs == L2CAP_FCS_CRC16)
>> len -= 2;
>>
>> - if (len > pi->mps || len < 4 || __is_sframe(control))
>> + if (len > pi->mps || len < 0 || __is_sframe(control))
>> goto drop;
>>
>> if (l2cap_check_fcs(pi, skb))
>>
>> This lowers the minimum-packet length of I-frame and streaming mode
>> packets put in place by commit 277ffbe. It maintains the length check
>> for S-frames. I believe ensuring a non-negative length is necessary,
>> but this check may need to be performed prior to the call to
>> l2cap_check_fcs().
>
> I agree with you that we should lower the value to 0. Please, send a proper
> git-formatted patch so I can pick it.
However, len is an unsigned value that will never be less than 0. The
easy fix is probably to make len an int instead of a u16.
--
Mat Martineau
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* Re: L2CAP ERTM compatibility problem
From: Gustavo F. Padovan @ 2010-06-08 20:51 UTC (permalink / raw)
To: Nathan Holstein; +Cc: linux-bluetooth
In-Reply-To: <1276017637.3546.519.camel@strawberry>
Hi Nathan,
* Nathan Holstein <ngh@isomerica.net> [2010-06-08 13:20:37 -0400]:
> I'm currently testing an older set of Gustavo's ERTM fixes backported
> onto a 2.6.32 kernel. I'm seeing a compatibility problem while
> attempting to establish a connection using ERTM. This appears to be
> caused by commit 277ffbe in Gustavo's testing tree which adds minimum
> length checks to received ERTM and Streaming Mode packets.
>
> Here's the incoming packet (hcidump -Xt):
>
> 1276011301.453633 > ACL data: handle 12 flags 0x02 dlen 11
> L2CAP(d): cid 0x0040 len 7 [psm 0]
> 0000: 02 01 07 00 06 2e 0b
>
> This appears to be a valid packet containing a three byte sequence which
> should be passed up to the application. However, the check added by
> commit 277ffbe causes the kernel to disconnect the socket since the data
> length is less than 4.
>
> I don't see the point of the minimum length at this point in the code.
> The len variable already subtracts for the control, FCS and SAR fields
> if necessary. It seems to me that the remaining length of the skb is
> purely socket data, and as such should be passed on to the application.
The checks comes from the section 3.3.7 of the L2CAP spec.
>
> Here's my proposed fix:
>
> @@ -4189,19 +4193,23 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
>
>
> if (__is_iframe(control)) {
> - if (len < 4) {
> + if (len < 0) {
> l2cap_send_disconn_req(pi->conn, sk);
> goto drop;
> }
> @@ -4222,7 +4230,7 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
> if (pi->fcs == L2CAP_FCS_CRC16)
> len -= 2;
>
> - if (len > pi->mps || len < 4 || __is_sframe(control))
> + if (len > pi->mps || len < 0 || __is_sframe(control))
> goto drop;
>
> if (l2cap_check_fcs(pi, skb))
>
> This lowers the minimum-packet length of I-frame and streaming mode
> packets put in place by commit 277ffbe. It maintains the length check
> for S-frames. I believe ensuring a non-negative length is necessary,
> but this check may need to be performed prior to the call to
> l2cap_check_fcs().
I agree with you that we should lower the value to 0. Please, send a proper
git-formatted patch so I can pick it.
Regards,
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* L2CAP ERTM compatibility problem
From: Nathan Holstein @ 2010-06-08 17:20 UTC (permalink / raw)
To: linux-bluetooth
I'm currently testing an older set of Gustavo's ERTM fixes backported
onto a 2.6.32 kernel. I'm seeing a compatibility problem while
attempting to establish a connection using ERTM. This appears to be
caused by commit 277ffbe in Gustavo's testing tree which adds minimum
length checks to received ERTM and Streaming Mode packets.
Here's the incoming packet (hcidump -Xt):
1276011301.453633 > ACL data: handle 12 flags 0x02 dlen 11
L2CAP(d): cid 0x0040 len 7 [psm 0]
0000: 02 01 07 00 06 2e 0b
This appears to be a valid packet containing a three byte sequence which
should be passed up to the application. However, the check added by
commit 277ffbe causes the kernel to disconnect the socket since the data
length is less than 4.
I don't see the point of the minimum length at this point in the code.
The len variable already subtracts for the control, FCS and SAR fields
if necessary. It seems to me that the remaining length of the skb is
purely socket data, and as such should be passed on to the application.
Here's my proposed fix:
@@ -4189,19 +4193,23 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
if (__is_iframe(control)) {
- if (len < 4) {
+ if (len < 0) {
l2cap_send_disconn_req(pi->conn, sk);
goto drop;
}
@@ -4222,7 +4230,7 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
if (pi->fcs == L2CAP_FCS_CRC16)
len -= 2;
- if (len > pi->mps || len < 4 || __is_sframe(control))
+ if (len > pi->mps || len < 0 || __is_sframe(control))
goto drop;
if (l2cap_check_fcs(pi, skb))
This lowers the minimum-packet length of I-frame and streaming mode
packets put in place by commit 277ffbe. It maintains the length check
for S-frames. I believe ensuring a non-negative length is necessary,
but this check may need to be performed prior to the call to
l2cap_check_fcs().
Thoughts?
--nathan
^ permalink raw reply
* Re: [PATCH] Get IEEE1284 for a single printer
From: Johan Hedberg @ 2010-06-08 9:26 UTC (permalink / raw)
To: Bastien Nocera; +Cc: linux-bluetooth
In-Reply-To: <1275988881.2458.12112.camel@localhost.localdomain>
Hi Bastien,
On Tue, Jun 08, 2010, Bastien Nocera wrote:
> > I have nothing against pushing the patch upstream as long as its coding
> > style issues are fixed:
> <snip>
> > Mixed tabs and spaces.
>
> You do realise that all those are cut'n'paste from another function in
> the same source file?
Nope, didn't realize that. So the whole file needs coding style cleanups
then.
> > > + fprintf(stderr, "Invalid Bluetooth address '%s'\n", argv[2]);
> >
> > Too long line.
>
> Right, I'll fix that.
Thanks.
Johan
^ permalink raw reply
* Re: [PATCH] Get IEEE1284 for a single printer
From: Bastien Nocera @ 2010-06-08 9:21 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <20100608070935.GA22068@jh-x301>
On Tue, 2010-06-08 at 15:09 +0800, Johan Hedberg wrote:
> Hi Bastien,
>
> On Sun, Jun 06, 2010, Bastien Nocera wrote:
> > Let me know what you think.
>
> Sounds ok'ish to me, but how exactly would udev-configure-printer be
> called? If it's directly executed (as opposed to e.g. using D-Bus) then
> privilege and environment inheritance needs to be considered (i.e. is it
> fine that it gets run with the same privileges and environment as
> bluetoothd itself).
>
> > The patch below could still be useful for debugging (in the worst
> > case).
>
> I have nothing against pushing the patch upstream as long as its coding
> style issues are fixed:
<snip>
> Mixed tabs and spaces.
You do realise that all those are cut'n'paste from another function in
the same source file?
> > + fprintf(stderr, "Invalid Bluetooth address '%s'\n", argv[2]);
>
> Too long line.
Right, I'll fix that.
^ permalink raw reply
* Re: [PATCH] audio: fix leak on gateway
From: Johan Hedberg @ 2010-06-08 8:07 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: linux-bluetooth
In-Reply-To: <1275981279-18175-1-git-send-email-gustavo@padovan.org>
Hi Gustavo,
On Tue, Jun 08, 2010, Gustavo F. Padovan wrote:
> ---
> audio/gateway.c | 15 ++++++++++++++-
> 1 files changed, 14 insertions(+), 1 deletions(-)
This patch is now upstream.
Johan
^ permalink raw reply
* Re: [PATCH 3/8] Fix regression with debug via SIGUSR2
From: Johan Hedberg @ 2010-06-08 8:05 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: linux-bluetooth
In-Reply-To: <1275981114-18019-3-git-send-email-gustavo@padovan.org>
Hi Gustavo,
On Tue, Jun 08, 2010, Gustavo F. Padovan wrote:
> The new dynamic debug feature was not using the SIGUSR2 signal so this was
> causing bluetoothd to crash when one tries to toggle debug via SIGUSR2.
> This patch brings back such compatibility andadds debug_string and
> debug_enabled vars.
> ---
> src/log.c | 17 +++++++++++++++--
> src/log.h | 6 +++++-
> src/main.c | 8 ++++++++
> 3 files changed, 28 insertions(+), 3 deletions(-)
I've commited the debug patches this far, i.e. the D-Bus changes aren't
in yet. I'd like to get a quick ok from Marcel before pushing (both on
the concept itself as well as the names for the new Manager properties).
Johan
^ permalink raw reply
* Re: [PATCH 2/3] Fix redundant null check on calling free()
From: Johan Hedberg @ 2010-06-08 8:02 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: linux-bluetooth
In-Reply-To: <1275732870-15889-3-git-send-email-gustavo@padovan.org>
Hi Gustavo,
On Sat, Jun 05, 2010, Gustavo F. Padovan wrote:
> Issues found by smatch static check: http://smatch.sourceforge.net/
> ---
> input/device.c | 6 +--
> lib/sdp.c | 89 +++++++++++++++++----------------------------------
> src/sdp-xml.c | 8 +---
> src/sdpd-request.c | 12 ++-----
> src/storage.c | 6 +--
> test/ipctest.c | 6 +--
> 6 files changed, 42 insertions(+), 85 deletions(-)
Both NULL-check removal patches have been pushed upstream.
Johan
^ permalink raw reply
* [PATCH] audio: fix leak on gateway
From: Gustavo F. Padovan @ 2010-06-08 7:14 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo
---
audio/gateway.c | 15 ++++++++++++++-
1 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/audio/gateway.c b/audio/gateway.c
index 1dd3f5b..07ebdd4 100644
--- a/audio/gateway.c
+++ b/audio/gateway.c
@@ -561,6 +561,19 @@ static GDBusSignalTable gateway_signals[] = {
{ NULL, NULL }
};
+static void path_unregister(void *data)
+{
+ struct audio_device *dev = data;
+
+ DBG("Unregistered interface %s on path %s",
+ AUDIO_GATEWAY_INTERFACE, dev->path);
+
+ gateway_close(dev);
+
+ g_free(dev->gateway);
+ dev->gateway = NULL;
+}
+
void gateway_unregister(struct audio_device *dev)
{
if (dev->gateway->agent)
@@ -578,7 +591,7 @@ struct gateway *gateway_init(struct audio_device *dev)
if (!g_dbus_register_interface(dev->conn, dev->path,
AUDIO_GATEWAY_INTERFACE,
gateway_methods, gateway_signals,
- NULL, dev, NULL))
+ NULL, dev, path_unregister))
return NULL;
return g_new0(struct gateway, 1);
--
1.7.1
^ permalink raw reply related
* Re: [PATCH 2/3] Remove ifndef barrier from log.h and btio.h
From: Marcel Holtmann @ 2010-06-08 7:14 UTC (permalink / raw)
To: Johan Hedberg; +Cc: Gustavo F. Padovan, linux-bluetooth
In-Reply-To: <20100608061954.GA16335@jh-x301>
Hi Johan,
> > src/btio.h | 3 ---
> > src/log.h | 4 +---
> > 2 files changed, 1 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/btio.h b/src/btio.h
> > index 00d743e..fa6ff69 100644
> > --- a/src/btio.h
> > +++ b/src/btio.h
> > @@ -21,8 +21,6 @@
> > * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> > *
> > */
> > -#ifndef BT_IO_H
> > -#define BT_IO_H
>
> I think it'd be good to leave this for BtIO since it might become a
> public library soon.
>
> > -#ifndef __LOGGING_H
> > -#define __LOGGING_H
> > +extern int debug_enabled;
>
> This doesn't match the commit message. To me it seems like the extern
> int addition should be part of the third patch and not this one. Also,
> Marcel might have had a reason for putting the safeguard for log.h so
> just leave these safeguards as they are (ofono and connman has them
> too).
that safeguard can be removed. That was some leftover from when it
looked like a good idea to make this more generic. It never worked out
so lets just remove this.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/3] sbc: Fix redundant null check on calling free()
From: Marcel Holtmann @ 2010-06-08 7:13 UTC (permalink / raw)
To: Johan Hedberg; +Cc: Gustavo F. Padovan, linux-bluetooth
In-Reply-To: <20100608062116.GA16634@jh-x301>
Hi Johan,
> > Issues found by smatch static check: http://smatch.sourceforge.net/
> > ---
> > sbc/sbc.c | 3 +--
> > sbc/sbcdec.c | 9 +++------
> > 2 files changed, 4 insertions(+), 8 deletions(-)
> >
> > diff --git a/sbc/sbc.c b/sbc/sbc.c
> > index edd152f..86399dd 100644
> > --- a/sbc/sbc.c
> > +++ b/sbc/sbc.c
> > @@ -1138,8 +1138,7 @@ void sbc_finish(sbc_t *sbc)
> > if (!sbc)
> > return;
> >
> > - if (sbc->priv_alloc_base)
> > - free(sbc->priv_alloc_base);
> > + free(sbc->priv_alloc_base);
>
> I'm a bit hesitant at removing these NULL checks since older libc
> versions might need them. Any comments from Marcel?
this is just fine. If you have such an old libc version that doesn't do
it, then you have a problem anyway. I am also not sure that this is a
problem with any libc version at all.
The only argument that might hold this is if you wanna compile this for
a non GNU platform. I don't even know if that is possible since I
actually don't care about non Linux platforms.
Regards
Marcel
^ permalink raw reply
* [PATCH 8/8] Document the new Debug and DebugString property
From: Gustavo F. Padovan @ 2010-06-08 7:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo
In-Reply-To: <1275981114-18019-7-git-send-email-gustavo@padovan.org>
---
doc/manager-api.txt | 19 +++++++++++++++++++
1 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/doc/manager-api.txt b/doc/manager-api.txt
index d2c1caf..07d4fde 100644
--- a/doc/manager-api.txt
+++ b/doc/manager-api.txt
@@ -22,6 +22,15 @@ Methods dict GetProperties()
Possible Errors: org.bluez.Error.DoesNotExist
org.bluez.Error.InvalidArguments
+ void SetProperty(string name, variant value)
+
+ Changes the value of the specified property. Only
+ properties that are listed a read-write are changeable.
+ On success this will emit a PropertyChanged signal.
+
+ Possible Errors: org.bluez.Error.DoesNotExist
+ org.bluez.Error.InvalidArguments
+
object DefaultAdapter()
Returns object path for the default adapter.
@@ -72,3 +81,13 @@ Signals PropertyChanged(string name, variant value)
Properties array{object} Adapters [readonly]
List of adapter object paths.
+
+ boolean Debug [readwrite]
+
+ Switch dynamic debug on or off.
+
+ string DebugString [readwrite]
+
+ Change the dynamic debug match string. Use it to
+ restrict debug to specific files. The default value is
+ "*" to debug all files.
--
1.7.1
^ permalink raw reply related
* [PATCH 7/8] Add DebugString Property to Manager
From: Gustavo F. Padovan @ 2010-06-08 7:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo
In-Reply-To: <1275981114-18019-6-git-send-email-gustavo@padovan.org>
---
src/manager.c | 38 ++++++++++++++++++++++++++++++++++++++
1 files changed, 38 insertions(+), 0 deletions(-)
diff --git a/src/manager.c b/src/manager.c
index 66e1511..220a8fe 100644
--- a/src/manager.c
+++ b/src/manager.c
@@ -96,6 +96,31 @@ static DBusMessage *set_debug(DBusConnection *conn, DBusMessage *msg,
return dbus_message_new_method_return(msg);
}
+static DBusMessage *set_debug_string(DBusConnection *conn, DBusMessage *msg,
+ const char *debug_string, void *data)
+{
+ char *old_string;
+
+ if (!g_utf8_validate(debug_string, -1, NULL)) {
+ error("DebugString change failed: supplied string "
+ "isn't valid UTF-8");
+ return invalid_args(msg);
+ }
+
+ old_string = __btd_get_debug_string();
+
+ if (strcmp((char *)debug_string, old_string) == 0)
+ return dbus_message_new_method_return(msg);
+
+ if (!__btd_set_debug_string((char *)debug_string))
+ return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
+ strerror(ENOMEM));
+ emit_property_changed(connection, "/", MANAGER_INTERFACE,
+ "DebugString", DBUS_TYPE_STRING, &debug_string);
+
+ return dbus_message_new_method_return(msg);
+}
+
static DBusMessage *default_adapter(DBusConnection *conn,
DBusMessage *msg, void *data)
{
@@ -200,6 +225,7 @@ static DBusMessage *get_properties(DBusConnection *conn,
GSList *list;
char **array;
gboolean debug_value;
+ char *debug_string;
int i;
reply = dbus_message_new_method_return(msg);
@@ -228,6 +254,10 @@ static DBusMessage *get_properties(DBusConnection *conn,
debug_value = __btd_debug_enabled();
dict_append_entry(&dict, "Debug", DBUS_TYPE_BOOLEAN, &debug_value);
+ debug_string = __btd_get_debug_string();
+ dict_append_entry(&dict, "DebugString", DBUS_TYPE_STRING,
+ &debug_string);
+
dbus_message_iter_close_container(&iter, &dict);
return reply;
@@ -262,6 +292,14 @@ static DBusMessage *set_property(DBusConnection *conn,
dbus_message_iter_get_basic(&sub, &debug);
return set_debug(conn, msg, debug, data);
+ } else if (g_str_equal("DebugString", property)) {
+ const char *string;
+
+ if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_STRING)
+ return invalid_args(msg);
+ dbus_message_iter_get_basic(&sub, &string);
+
+ return set_debug_string(conn, msg, string, data);
}
return invalid_args(msg);
--
1.7.1
^ permalink raw reply related
* [PATCH 6/8] log: Add function to get/set debug_string
From: Gustavo F. Padovan @ 2010-06-08 7:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo
In-Reply-To: <1275981114-18019-5-git-send-email-gustavo@padovan.org>
---
src/log.c | 56 +++++++++++++++++++++++++++++++++++++++++---------------
src/log.h | 2 ++
2 files changed, 43 insertions(+), 15 deletions(-)
diff --git a/src/log.c b/src/log.c
index 4043ee3..f0f2465 100644
--- a/src/log.c
+++ b/src/log.c
@@ -72,6 +72,7 @@ extern struct btd_debug_desc __stop___debug[];
static gchar **enabled = NULL;
int debug_enabled = FALSE;
+static char *debug_string = NULL;
static gboolean is_enabled(struct btd_debug_desc *desc)
{
@@ -92,6 +93,44 @@ static gboolean is_enabled(struct btd_debug_desc *desc)
return 0;
}
+static void debug_on()
+{
+ struct btd_debug_desc *desc;
+ const char *name = NULL, *file = NULL;
+
+ enabled = g_strsplit_set(debug_string, ":, ", 0);
+
+ for (desc = __start___debug; desc < __stop___debug; desc++) {
+ if (file != NULL || name != NULL) {
+ if (g_strcmp0(desc->file, file) == 0) {
+ if (desc->name == NULL)
+ desc->name = name;
+ } else
+ file = NULL;
+ }
+
+ if (is_enabled(desc))
+ desc->flags |= BTD_DEBUG_FLAG_PRINT;
+ else
+ desc->flags &= ~BTD_DEBUG_FLAG_PRINT;
+ }
+}
+
+char *__btd_get_debug_string()
+{
+ return debug_string;
+}
+
+char *__btd_set_debug_string(char *str)
+{
+ g_free(debug_string);
+ debug_string = g_strdup(str);
+
+ debug_on();
+
+ return debug_string;
+}
+
void __btd_toggle_debug()
{
debug_enabled = !debug_enabled;
@@ -105,8 +144,6 @@ int __btd_debug_enabled()
void __btd_log_init(const char *debug, int detach)
{
int option = LOG_NDELAY | LOG_PID;
- struct btd_debug_desc *desc;
- const char *name = NULL, *file = NULL;
if (debug != NULL) {
debug_enabled = TRUE;
@@ -115,20 +152,9 @@ void __btd_log_init(const char *debug, int detach)
debug_enabled = FALSE;
}
- enabled = g_strsplit_set(debug, ":, ", 0);
-
- for (desc = __start___debug; desc < __stop___debug; desc++) {
- if (file != NULL || name != NULL) {
- if (g_strcmp0(desc->file, file) == 0) {
- if (desc->name == NULL)
- desc->name = name;
- } else
- file = NULL;
- }
+ debug_string = (char *)debug;
- if (is_enabled(desc))
- desc->flags |= BTD_DEBUG_FLAG_PRINT;
- }
+ debug_on();
if (!detach)
option |= LOG_PERROR;
diff --git a/src/log.h b/src/log.h
index 5d1db48..81c53df 100644
--- a/src/log.h
+++ b/src/log.h
@@ -27,6 +27,8 @@ void debug(const char *format, ...) __attribute__((format(printf, 1, 2)));
void __btd_log_init(const char *debug, int detach);
void __btd_log_cleanup(void);
+char *__btd_get_debug_string();
+char *__btd_set_debug_string(char *str);
void __btd_toggle_debug();
int __btd_debug_enabled();
--
1.7.1
^ permalink raw reply related
* [PATCH 5/8] Add SetProperty method and Debug read/write property
From: Gustavo F. Padovan @ 2010-06-08 7:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo
In-Reply-To: <1275981114-18019-4-git-send-email-gustavo@padovan.org>
---
src/manager.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 52 insertions(+), 0 deletions(-)
diff --git a/src/manager.c b/src/manager.c
index eab7e80..66e1511 100644
--- a/src/manager.c
+++ b/src/manager.c
@@ -80,6 +80,22 @@ static inline DBusMessage *no_such_adapter(DBusMessage *msg)
"No such adapter");
}
+static DBusMessage *set_debug(DBusConnection *conn, DBusMessage *msg,
+ gboolean debug, void *data)
+{
+ gboolean old_debug = __btd_debug_enabled();
+
+ if (debug == old_debug)
+ return dbus_message_new_method_return(msg);
+
+ __btd_toggle_debug(debug);
+
+ emit_property_changed(connection, "/" , MANAGER_INTERFACE, "Debug",
+ DBUS_TYPE_BOOLEAN, &debug);
+
+ return dbus_message_new_method_return(msg);
+}
+
static DBusMessage *default_adapter(DBusConnection *conn,
DBusMessage *msg, void *data)
{
@@ -217,8 +233,44 @@ static DBusMessage *get_properties(DBusConnection *conn,
return reply;
}
+static DBusMessage *set_property(DBusConnection *conn,
+ DBusMessage *msg, void *data)
+{
+ DBusMessageIter iter;
+ DBusMessageIter sub;
+ const char *property;
+
+ if (!dbus_message_iter_init(msg, &iter))
+ return invalid_args(msg);
+
+ if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+ return invalid_args(msg);
+
+ dbus_message_iter_get_basic(&iter, &property);
+ dbus_message_iter_next(&iter);
+
+ if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
+ return invalid_args(msg);
+ dbus_message_iter_recurse(&iter, &sub);
+
+ if (g_str_equal("Debug", property)) {
+ gboolean debug;
+
+ if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_BOOLEAN)
+ return invalid_args(msg);
+
+ dbus_message_iter_get_basic(&sub, &debug);
+
+ return set_debug(conn, msg, debug, data);
+ }
+
+ return invalid_args(msg);
+}
+
static GDBusMethodTable manager_methods[] = {
{ "GetProperties", "", "a{sv}",get_properties },
+ { "SetProperty", "sv", "", set_property,
+ G_DBUS_METHOD_FLAG_ASYNC},
{ "DefaultAdapter", "", "o", default_adapter },
{ "FindAdapter", "s", "o", find_adapter },
{ "ListAdapters", "", "ao", list_adapters,
--
1.7.1
^ permalink raw reply related
* [PATCH 4/8] Add Debug property to Manager interface
From: Gustavo F. Padovan @ 2010-06-08 7:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo
In-Reply-To: <1275981114-18019-3-git-send-email-gustavo@padovan.org>
---
src/log.c | 5 +++++
src/log.h | 1 +
src/manager.c | 4 ++++
3 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/src/log.c b/src/log.c
index 1bc0a42..4043ee3 100644
--- a/src/log.c
+++ b/src/log.c
@@ -97,6 +97,11 @@ void __btd_toggle_debug()
debug_enabled = !debug_enabled;
}
+int __btd_debug_enabled()
+{
+ return debug_enabled;
+}
+
void __btd_log_init(const char *debug, int detach)
{
int option = LOG_NDELAY | LOG_PID;
diff --git a/src/log.h b/src/log.h
index 0afd739..5d1db48 100644
--- a/src/log.h
+++ b/src/log.h
@@ -28,6 +28,7 @@ void debug(const char *format, ...) __attribute__((format(printf, 1, 2)));
void __btd_log_init(const char *debug, int detach);
void __btd_log_cleanup(void);
void __btd_toggle_debug();
+int __btd_debug_enabled();
extern int debug_enabled;
diff --git a/src/manager.c b/src/manager.c
index cbbca1e..eab7e80 100644
--- a/src/manager.c
+++ b/src/manager.c
@@ -183,6 +183,7 @@ static DBusMessage *get_properties(DBusConnection *conn,
DBusMessageIter dict;
GSList *list;
char **array;
+ gboolean debug_value;
int i;
reply = dbus_message_new_method_return(msg);
@@ -208,6 +209,9 @@ static DBusMessage *get_properties(DBusConnection *conn,
dict_append_array(&dict, "Adapters", DBUS_TYPE_OBJECT_PATH, &array, i);
g_free(array);
+ debug_value = __btd_debug_enabled();
+ dict_append_entry(&dict, "Debug", DBUS_TYPE_BOOLEAN, &debug_value);
+
dbus_message_iter_close_container(&iter, &dict);
return reply;
--
1.7.1
^ permalink raw reply related
* [PATCH 3/8] Fix regression with debug via SIGUSR2
From: Gustavo F. Padovan @ 2010-06-08 7:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo
In-Reply-To: <1275981114-18019-2-git-send-email-gustavo@padovan.org>
The new dynamic debug feature was not using the SIGUSR2 signal so this was
causing bluetoothd to crash when one tries to toggle debug via SIGUSR2.
This patch brings back such compatibility andadds debug_string and
debug_enabled vars.
---
src/log.c | 17 +++++++++++++++--
src/log.h | 6 +++++-
src/main.c | 8 ++++++++
3 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/src/log.c b/src/log.c
index cb02aad..1bc0a42 100644
--- a/src/log.c
+++ b/src/log.c
@@ -71,6 +71,8 @@ extern struct btd_debug_desc __stop___debug[];
static gchar **enabled = NULL;
+int debug_enabled = FALSE;
+
static gboolean is_enabled(struct btd_debug_desc *desc)
{
int i;
@@ -90,14 +92,25 @@ static gboolean is_enabled(struct btd_debug_desc *desc)
return 0;
}
+void __btd_toggle_debug()
+{
+ debug_enabled = !debug_enabled;
+}
+
void __btd_log_init(const char *debug, int detach)
{
int option = LOG_NDELAY | LOG_PID;
struct btd_debug_desc *desc;
const char *name = NULL, *file = NULL;
- if (debug != NULL)
- enabled = g_strsplit_set(debug, ":, ", 0);
+ if (debug != NULL) {
+ debug_enabled = TRUE;
+ } else {
+ debug = g_strdup("*");
+ debug_enabled = FALSE;
+ }
+
+ enabled = g_strsplit_set(debug, ":, ", 0);
for (desc = __start___debug; desc < __stop___debug; desc++) {
if (file != NULL || name != NULL) {
diff --git a/src/log.h b/src/log.h
index f78ecac..0afd739 100644
--- a/src/log.h
+++ b/src/log.h
@@ -27,6 +27,9 @@ void debug(const char *format, ...) __attribute__((format(printf, 1, 2)));
void __btd_log_init(const char *debug, int detach);
void __btd_log_cleanup(void);
+void __btd_toggle_debug();
+
+extern int debug_enabled;
struct btd_debug_desc {
const char *name;
@@ -49,7 +52,8 @@ struct btd_debug_desc {
__attribute__((used, section("__debug"), aligned(8))) = { \
.file = __FILE__, .flags = BTD_DEBUG_FLAG_DEFAULT, \
}; \
- if (__btd_debug_desc.flags & BTD_DEBUG_FLAG_PRINT) \
+ if (debug_enabled && \
+ __btd_debug_desc.flags & BTD_DEBUG_FLAG_PRINT) \
debug("%s:%s() " fmt, \
__FILE__, __FUNCTION__ , ## arg); \
} while (0)
diff --git a/src/main.c b/src/main.c
index 3118a34..ba18523 100644
--- a/src/main.c
+++ b/src/main.c
@@ -288,6 +288,11 @@ static void sig_term(int sig)
g_main_loop_quit(event_loop);
}
+static void sig_debug(int sig)
+{
+ __btd_toggle_debug();
+}
+
static gchar *option_debug = NULL;
static gboolean option_detach = TRUE;
static gboolean option_udev = FALSE;
@@ -406,6 +411,9 @@ int main(int argc, char *argv[])
sigaction(SIGTERM, &sa, NULL);
sigaction(SIGINT, &sa, NULL);
+ sa.sa_handler = sig_debug;
+ sigaction(SIGUSR2, &sa, NULL);
+
sa.sa_handler = SIG_IGN;
sigaction(SIGPIPE, &sa, NULL);
--
1.7.1
^ permalink raw reply related
* [PATCH 2/8] log: Remove vinfo function
From: Gustavo F. Padovan @ 2010-06-08 7:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo
In-Reply-To: <1275981114-18019-1-git-send-email-gustavo@padovan.org>
It was used only once.
---
src/log.c | 7 +------
1 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/src/log.c b/src/log.c
index 29e2d7d..cb02aad 100644
--- a/src/log.c
+++ b/src/log.c
@@ -33,18 +33,13 @@
#include "log.h"
-static inline void vinfo(const char *format, va_list ap)
-{
- vsyslog(LOG_INFO, format, ap);
-}
-
void info(const char *format, ...)
{
va_list ap;
va_start(ap, format);
- vinfo(format, ap);
+ vsyslog(LOG_INFO, format, ap);
va_end(ap);
}
--
1.7.1
^ permalink raw reply related
* [PATCH 1/8] Remove ifndef barrier from log.h and btio.h
From: Gustavo F. Padovan @ 2010-06-08 7:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo
---
src/btio.h | 3 ---
src/log.h | 4 ----
2 files changed, 0 insertions(+), 7 deletions(-)
diff --git a/src/btio.h b/src/btio.h
index 00d743e..fa6ff69 100644
--- a/src/btio.h
+++ b/src/btio.h
@@ -21,8 +21,6 @@
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*
*/
-#ifndef BT_IO_H
-#define BT_IO_H
#include <glib.h>
@@ -93,4 +91,3 @@ GIOChannel *bt_io_listen(BtIOType type, BtIOConnect connect,
GDestroyNotify destroy, GError **err,
BtIOOption opt1, ...);
-#endif
diff --git a/src/log.h b/src/log.h
index 9af51e7..f78ecac 100644
--- a/src/log.h
+++ b/src/log.h
@@ -21,9 +21,6 @@
*
*/
-#ifndef __LOGGING_H
-#define __LOGGING_H
-
void info(const char *format, ...) __attribute__((format(printf, 1, 2)));
void error(const char *format, ...) __attribute__((format(printf, 1, 2)));
void debug(const char *format, ...) __attribute__((format(printf, 1, 2)));
@@ -57,4 +54,3 @@ struct btd_debug_desc {
__FILE__, __FUNCTION__ , ## arg); \
} while (0)
-#endif /* __LOGGING_H */
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] Get IEEE1284 for a single printer
From: Johan Hedberg @ 2010-06-08 7:09 UTC (permalink / raw)
To: Bastien Nocera; +Cc: linux-bluetooth
In-Reply-To: <1275849744.7994.38.camel@localhost.localdomain>
Hi Bastien,
On Sun, Jun 06, 2010, Bastien Nocera wrote:
> Let me know what you think.
Sounds ok'ish to me, but how exactly would udev-configure-printer be
called? If it's directly executed (as opposed to e.g. using D-Bus) then
privilege and environment inheritance needs to be considered (i.e. is it
fine that it gets run with the same privileges and environment as
bluetoothd itself).
> The patch below could still be useful for debugging (in the worst
> case).
I have nothing against pushing the patch upstream as long as its coding
style issues are fixed:
> + message = dbus_message_new_method_call("org.bluez", "/",
> + "org.bluez.Manager",
> + "DefaultAdapter");
Mixed tabs and spaces for indentation.
> + adapter_reply = dbus_connection_send_with_reply_and_block(conn,
> + message, -1, NULL);
Same.
> + if (dbus_message_get_args(adapter_reply, NULL, DBUS_TYPE_OBJECT_PATH, &adapter, DBUS_TYPE_INVALID) == FALSE)
Longer line than 79 characters.
> + message = dbus_message_new_method_call("org.bluez", adapter,
> + "org.bluez.Adapter",
> + "FindDevice");
Mixed tabs and spaces.
> + message = dbus_message_new_method_call("org.bluez", adapter,
> + "org.bluez.Adapter",
> + "CreateDevice");
Mixed tabs and spaces.
> + dbus_message_iter_init_append(message, &iter);
> + dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING, &bdaddr);
Longer line than 79 characters.
> + reply = dbus_connection_send_with_reply_and_block(conn,
> + message, -1, NULL);
Mixed tabs and spaces as well as overlong line.
> + if (!reply)
> + return FALSE;
> + }
> + if (dbus_message_get_args(reply, NULL, DBUS_TYPE_OBJECT_PATH, &object_path,
Missing empty line after } as well as too long line.
> + DBUS_TYPE_INVALID) == FALSE) {
Mixed tabs and spaces.
> + fprintf(stderr, "Invalid Bluetooth address '%s'\n", argv[2]);
Too long line.
Johan
^ permalink raw reply
* Re: [PATCH 3/3] sdptool: Fix 2 possible NULL dereference
From: Johan Hedberg @ 2010-06-08 6:27 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: linux-bluetooth
In-Reply-To: <1275732870-15889-4-git-send-email-gustavo@padovan.org>
Hi Gustavo,
On Sat, Jun 05, 2010, Gustavo F. Padovan wrote:
> Issues found by smatch static check:
> http://smatch.sourceforge.net/
> ---
> tools/sdptool.c | 19 +++++++++----------
> 1 files changed, 9 insertions(+), 10 deletions(-)
This patch is now also upstream. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH] audio: Fix typo
From: Johan Hedberg @ 2010-06-08 6:23 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: linux-bluetooth
In-Reply-To: <1275792403-5535-1-git-send-email-gustavo@padovan.org>
Hi Gustavo,
On Sat, Jun 05, 2010, Gustavo F. Padovan wrote:
> audio/gateway.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/audio/gateway.c b/audio/gateway.c
> index dfe7145..1dd3f5b 100644
> --- a/audio/gateway.c
> +++ b/audio/gateway.c
> @@ -204,7 +204,7 @@ static void newconnection_reply(DBusPendingCall *call, void *data)
>
> dbus_error_init(&derr);
> if (!dbus_set_error_from_message(&derr, reply)) {
> - DBG("Agent reply: file descriptor passed successfuly");
> + DBG("Agent reply: file descriptor passed successfully");
> change_state(dev, GATEWAY_STATE_CONNECTED);
> goto done;
> }
> @@ -234,7 +234,7 @@ static void rfcomm_connect_cb(GIOChannel *chan, GError *err,
> }
>
> if (!gw->agent) {
> - error("Handfree Agent not registered");
> + error("Handsfree Agent not registered");
> goto fail;
> }
This patch is now upstream. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH 1/3] sbc: Fix redundant null check on calling free()
From: Johan Hedberg @ 2010-06-08 6:21 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: linux-bluetooth
In-Reply-To: <1275732870-15889-2-git-send-email-gustavo@padovan.org>
Hi Gustavo,
On Sat, Jun 05, 2010, Gustavo F. Padovan wrote:
> Issues found by smatch static check: http://smatch.sourceforge.net/
> ---
> sbc/sbc.c | 3 +--
> sbc/sbcdec.c | 9 +++------
> 2 files changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/sbc/sbc.c b/sbc/sbc.c
> index edd152f..86399dd 100644
> --- a/sbc/sbc.c
> +++ b/sbc/sbc.c
> @@ -1138,8 +1138,7 @@ void sbc_finish(sbc_t *sbc)
> if (!sbc)
> return;
>
> - if (sbc->priv_alloc_base)
> - free(sbc->priv_alloc_base);
> + free(sbc->priv_alloc_base);
I'm a bit hesitant at removing these NULL checks since older libc
versions might need them. Any comments from Marcel?
Johan
^ permalink raw reply
* Re: [PATCH 2/3] Remove ifndef barrier from log.h and btio.h
From: Johan Hedberg @ 2010-06-08 6:19 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: linux-bluetooth
In-Reply-To: <1275712886-20127-2-git-send-email-gustavo@padovan.org>
Hi Gustavo,
On Sat, Jun 05, 2010, Gustavo F. Padovan wrote:
> ---
> src/btio.h | 3 ---
> src/log.h | 4 +---
> 2 files changed, 1 insertions(+), 6 deletions(-)
>
> diff --git a/src/btio.h b/src/btio.h
> index 00d743e..fa6ff69 100644
> --- a/src/btio.h
> +++ b/src/btio.h
> @@ -21,8 +21,6 @@
> * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> *
> */
> -#ifndef BT_IO_H
> -#define BT_IO_H
I think it'd be good to leave this for BtIO since it might become a
public library soon.
> -#ifndef __LOGGING_H
> -#define __LOGGING_H
> +extern int debug_enabled;
This doesn't match the commit message. To me it seems like the extern
int addition should be part of the third patch and not this one. Also,
Marcel might have had a reason for putting the safeguard for log.h so
just leave these safeguards as they are (ofono and connman has them
too).
Johan
^ 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