* [PATCH v3 1/2] Bluetooth: Introduce a new HCI_RFKILLED flag
From: johan.hedberg @ 2013-09-13 5:58 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1379051898-2630-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
This makes it more convenient to check for rfkill (no need to check for
dev->rfkill before calling rfkill_blocked()) and also avoids potential
races if the RFKILL state needs to be checked from within the rfkill
callback.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
Cc: stable@vger.kernel.org
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
v3: no changes
v2: Explicitly intialize HCI_RFKILLED flag after calling rfkill_register
include/net/bluetooth/hci.h | 1 +
net/bluetooth/hci_core.c | 15 ++++++++++-----
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 30c88b5..ba008d5 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -104,6 +104,7 @@ enum {
enum {
HCI_SETUP,
HCI_AUTO_OFF,
+ HCI_RFKILLED,
HCI_MGMT,
HCI_PAIRABLE,
HCI_SERVICE_CACHE,
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 4dbb6cb..d0d6cf8 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1148,7 +1148,7 @@ int hci_dev_open(__u16 dev)
goto done;
}
- if (hdev->rfkill && rfkill_blocked(hdev->rfkill)) {
+ if (test_bit(HCI_RFKILLED, &hdev->dev_flags)) {
ret = -ERFKILL;
goto done;
}
@@ -1597,10 +1597,12 @@ static int hci_rfkill_set_block(void *data, bool blocked)
if (test_bit(HCI_USER_CHANNEL, &hdev->dev_flags))
return -EBUSY;
- if (!blocked)
- return 0;
-
- hci_dev_do_close(hdev);
+ if (blocked) {
+ set_bit(HCI_RFKILLED, &hdev->dev_flags);
+ hci_dev_do_close(hdev);
+ } else {
+ clear_bit(HCI_RFKILLED, &hdev->dev_flags);
+ }
return 0;
}
@@ -2244,6 +2246,9 @@ int hci_register_dev(struct hci_dev *hdev)
}
}
+ if (hdev->rfkill && rfkill_blocked(hdev->rfkill))
+ set_bit(HCI_RFKILLED, &hdev->dev_flags);
+
set_bit(HCI_SETUP, &hdev->dev_flags);
if (hdev->dev_type != HCI_AMP)
--
1.8.4.rc3
^ permalink raw reply related
* [PATCH v3 2/2] Bluetooth: Fix rfkill functionality during the HCI setup stage
From: johan.hedberg @ 2013-09-13 5:58 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1379051898-2630-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
We need to let the setup stage complete cleanly even when the HCI device
is rfkilled. Otherwise the HCI device will stay in an undefined state
and never get notified to user space through mgmt (even when it gets
unblocked through rfkill).
This patch makes sure that hci_dev_open() can be called in the HCI_SETUP
stage, that blocking the device doesn't abort the setup stage, and that
the device gets proper powered down as soon as the setup stage completes
in case it was blocked meanwhile.
The bug that this patch fixed can be very easily reproduced using e.g.
the rfkill command line too. By running "rfkill block all" before
inserting a Bluetooth dongle the resulting HCI device goes into a state
where it is never announced over mgmt, not even when "rfkill unblock all"
is run.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
Cc: stable@vger.kernel.org
---
v3: Use "else if" instead of separate if-statement
net/bluetooth/hci_core.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index d0d6cf8..12b017d 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1148,7 +1148,11 @@ int hci_dev_open(__u16 dev)
goto done;
}
- if (test_bit(HCI_RFKILLED, &hdev->dev_flags)) {
+ /* Check for rfkill but allow the HCI setup stage to proceed
+ * (which in itself doesn't cause any RF activity).
+ */
+ if (test_bit(HCI_RFKILLED, &hdev->dev_flags) &&
+ !test_bit(HCI_SETUP, &hdev->dev_flags)) {
ret = -ERFKILL;
goto done;
}
@@ -1599,7 +1603,8 @@ static int hci_rfkill_set_block(void *data, bool blocked)
if (blocked) {
set_bit(HCI_RFKILLED, &hdev->dev_flags);
- hci_dev_do_close(hdev);
+ if (!test_bit(HCI_SETUP, &hdev->dev_flags))
+ hci_dev_do_close(hdev);
} else {
clear_bit(HCI_RFKILLED, &hdev->dev_flags);
}
@@ -1624,9 +1629,13 @@ static void hci_power_on(struct work_struct *work)
return;
}
- if (test_bit(HCI_AUTO_OFF, &hdev->dev_flags))
+ if (test_bit(HCI_RFKILLED, &hdev->dev_flags)) {
+ clear_bit(HCI_AUTO_OFF, &hdev->dev_flags);
+ hci_dev_do_close(hdev);
+ } else if (test_bit(HCI_AUTO_OFF, &hdev->dev_flags)) {
queue_delayed_work(hdev->req_workqueue, &hdev->power_off,
HCI_AUTO_OFF_TIMEOUT);
+ }
if (test_and_clear_bit(HCI_SETUP, &hdev->dev_flags))
mgmt_index_added(hdev);
--
1.8.4.rc3
^ permalink raw reply related
* Re: [PATCH v3 2/2] Bluetooth: Fix rfkill functionality during the HCI setup stage
From: Marcel Holtmann @ 2013-09-13 6:23 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <1379051898-2630-3-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> We need to let the setup stage complete cleanly even when the HCI device
> is rfkilled. Otherwise the HCI device will stay in an undefined state
> and never get notified to user space through mgmt (even when it gets
> unblocked through rfkill).
>
> This patch makes sure that hci_dev_open() can be called in the HCI_SETUP
> stage, that blocking the device doesn't abort the setup stage, and that
> the device gets proper powered down as soon as the setup stage completes
> in case it was blocked meanwhile.
>
> The bug that this patch fixed can be very easily reproduced using e.g.
> the rfkill command line too. By running "rfkill block all" before
> inserting a Bluetooth dongle the resulting HCI device goes into a state
> where it is never announced over mgmt, not even when "rfkill unblock all"
> is run.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> Cc: stable@vger.kernel.org
> ---
> v3: Use "else if" instead of separate if-statement
>
> net/bluetooth/hci_core.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: Device discovery/pairing issue w/ BlueZ on Debian
From: Szymon Janc @ 2013-09-13 6:50 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: Clemens Vasters, BlueZ development
In-Reply-To: <CAJdJm_PpQoQeJ5SGrzD3RuA+7mOg-mAQgzSx5WV29528hU3stA@mail.gmail.com>
Hi,
> On Tue, Sep 10, 2013 at 2:47 PM, Clemens Vasters <clemensv@gmail.com> wrote:
> > I’ve got a cheap Bluetooth car diagnostics dongle (OBD-2, ELM327) that
> > gets discovered by and pairs with Windows 8, Windows Phone 8, OSX
> > Mountain Lion, iOS 6, and Android Jelly Bean.
> >
> > I can’t get it to be either discovered or paired with BlueZ on Debian
> > Wheezy on my Notebook (bare metal, ThinkPad W510) or Raspbian Wheezy
> > on the Raspberry Pi. On the latter I even went so far to pull the
> > source for BlueZ 5.8 and install it from source. No dice.
>
> Your description of the problem is a little vague. Which tools did you
> use on BlueZ 5.8 to attempt to discover/pair with the device? Which
> errors did you see (if any)?
Wheezy comes with kernel 3.2 so BlueZ 5 is no option there (at least on stock
kernel).
--
BR
Szymon Janc
^ permalink raw reply
* [PATCH 0/2] Bluetooth: Add new HCI commands to the init sequence
From: johan.hedberg @ 2013-09-13 8:40 UTC (permalink / raw)
To: linux-bluetooth
Hi,
These two patches add a couple of new(ish) HCI commands to the init
sequence. Event mask page 2 has been available already since core spec
4.0 whereas the sync train parameters were introduced in CSA4.
Johan
----------------------------------------------------------------
Johan Hedberg (2):
Bluetooth: Add synchronization train parameters reading support
Bluetooth: Add event mask page 2 setting support
include/net/bluetooth/hci.h | 4 ++++
net/bluetooth/hci_core.c | 47 +++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 50 insertions(+), 1 deletion(-)
^ permalink raw reply
* [PATCH 1/2] Bluetooth: Add synchronization train parameters reading support
From: johan.hedberg @ 2013-09-13 8:40 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1379061602-8290-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
This patch adds support for reading the synchronization train parameters
for controllers that support the feature. Since the feature is
detectable through the local features page 2, which is retreived only in
stage 3 of the HCI init sequence, there is no other option than to add a
fourth stage to the init sequence.
For now the patch doesn't yet add storing of the parameters, but it is
nevertheless convenient to have around to see what kind of parameters
various controllers use by default (analyzable e.g. with the btmon user
space tool).
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
include/net/bluetooth/hci.h | 2 ++
net/bluetooth/hci_core.c | 15 ++++++++++++++-
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index ba008d5..63cce97 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -839,6 +839,8 @@ struct hci_cp_write_le_host_supported {
__u8 simul;
} __packed;
+#define HCI_OP_READ_SYNC_TRAIN_PARAMS 0x0c77
+
#define HCI_OP_READ_LOCAL_VERSION 0x1001
struct hci_rp_read_local_version {
__u8 status;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 12b017d..8f73b1f 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -645,6 +645,15 @@ static void hci_init3_req(struct hci_request *req, unsigned long opt)
}
}
+static void hci_init4_req(struct hci_request *req, unsigned long opt)
+{
+ struct hci_dev *hdev = req->hdev;
+
+ /* Check for Synchronization Train support */
+ if (hdev->features[2][0] & 0x04)
+ hci_req_add(req, HCI_OP_READ_SYNC_TRAIN_PARAMS, 0, NULL);
+}
+
static int __hci_init(struct hci_dev *hdev)
{
int err;
@@ -664,7 +673,11 @@ static int __hci_init(struct hci_dev *hdev)
if (err < 0)
return err;
- return __hci_req_sync(hdev, hci_init3_req, 0, HCI_INIT_TIMEOUT);
+ err = __hci_req_sync(hdev, hci_init3_req, 0, HCI_INIT_TIMEOUT);
+ if (err < 0)
+ return err;
+
+ return __hci_req_sync(hdev, hci_init4_req, 0, HCI_INIT_TIMEOUT);
}
static void hci_scan_req(struct hci_request *req, unsigned long opt)
--
1.8.4.rc3
^ permalink raw reply related
* [PATCH 2/2] Bluetooth: Add event mask page 2 setting support
From: johan.hedberg @ 2013-09-13 8:40 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1379061602-8290-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
For those controller that support the HCI_Set_Event_Mask_Page_2 command
we should include it in the init sequence. This patch implements sending
of the command and enables the events in it based on supported features
(currently only CSB is checked).
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
include/net/bluetooth/hci.h | 2 ++
net/bluetooth/hci_core.c | 32 ++++++++++++++++++++++++++++++++
2 files changed, 34 insertions(+)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 63cce97..57aa408 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -827,6 +827,8 @@ struct hci_rp_read_inq_rsp_tx_power {
__s8 tx_power;
} __packed;
+#define HCI_OP_SET_EVENT_MASK_PAGE_2 0x0c63
+
#define HCI_OP_READ_FLOW_CONTROL_MODE 0x0c66
struct hci_rp_read_flow_control_mode {
__u8 status;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 8f73b1f..35550d3 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -604,6 +604,34 @@ static void hci_set_le_support(struct hci_request *req)
&cp);
}
+static void hci_set_event_mask_page_2(struct hci_request *req)
+{
+ struct hci_dev *hdev = req->hdev;
+ u8 events[8] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };
+
+ /* If Connectionless Slave Broadcast master role is supported
+ * enable all necessary events for it.
+ */
+ if (hdev->features[2][0] & 0x01) {
+ events[1] |= 0x40; /* Triggered Clock Capture */
+ events[1] |= 0x80; /* Synchronization Train Complete */
+ events[2] |= 0x10; /* Slave Page Response Timeout */
+ events[2] |= 0x20; /* CSB Channel Map Change */
+ }
+
+ /* If Connectionless Slave Broadcast slave role is supported
+ * enable all necessary events for it.
+ */
+ if (hdev->features[2][0] & 0x02) {
+ events[2] |= 0x01; /* Synchronization Train Received */
+ events[2] |= 0x02; /* CSB Receive */
+ events[2] |= 0x04; /* CSB Timeout */
+ events[2] |= 0x08; /* Truncated Page Complete */
+ }
+
+ hci_req_add(req, HCI_OP_SET_EVENT_MASK_PAGE_2, sizeof(events), events);
+}
+
static void hci_init3_req(struct hci_request *req, unsigned long opt)
{
struct hci_dev *hdev = req->hdev;
@@ -649,6 +677,10 @@ static void hci_init4_req(struct hci_request *req, unsigned long opt)
{
struct hci_dev *hdev = req->hdev;
+ /* Set event mask page 2 if the HCI command for it is supported */
+ if (hdev->commands[22] & 0x04)
+ hci_set_event_mask_page_2(req);
+
/* Check for Synchronization Train support */
if (hdev->features[2][0] & 0x04)
hci_req_add(req, HCI_OP_READ_SYNC_TRAIN_PARAMS, 0, NULL);
--
1.8.4.rc3
^ permalink raw reply related
* [PATCH 0/3] Bluetooth: Various L2CAP fixes
From: johan.hedberg @ 2013-09-13 8:43 UTC (permalink / raw)
To: linux-bluetooth
Hi,
This patch set includes various fixes to L2CAP signaling and socket
handling.
Johan
----------------------------------------------------------------
Johan Hedberg (3):
Bluetooth: Fix L2CAP Disconnect response for unknown CID
Bluetooth: Fix responding to invalid L2CAP signaling commands
Bluetooth: Fix waiting for clearing of BT_SK_SUSPEND flag
include/net/bluetooth/bluetooth.h | 1 +
net/bluetooth/af_bluetooth.c | 38 +++++++++++++++++++++++++++++++++++++
net/bluetooth/l2cap_core.c | 10 +++++++++-
net/bluetooth/l2cap_sock.c | 6 ++++++
net/bluetooth/rfcomm/sock.c | 7 +++++--
5 files changed, 59 insertions(+), 3 deletions(-)
^ permalink raw reply
* [PATCH 1/3] Bluetooth: Fix L2CAP Disconnect response for unknown CID
From: johan.hedberg @ 2013-09-13 8:43 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1379061827-8595-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
If we receive an L2CAP Disconnect Request for an unknown CID we should
not just silently drop it but reply with a proper Command Reject
response. This patch fixes this by ensuring that the disconnect handler
returns a proper error instead of 0 and will cause the function caller
to send the right response.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/l2cap_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index b3bb7bc..ea3792f 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4206,7 +4206,7 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn,
chan = __l2cap_get_chan_by_scid(conn, dcid);
if (!chan) {
mutex_unlock(&conn->chan_lock);
- return 0;
+ return -EINVAL;
}
l2cap_chan_lock(chan);
--
1.8.4.rc3
^ permalink raw reply related
* [PATCH 2/3] Bluetooth: Fix responding to invalid L2CAP signaling commands
From: johan.hedberg @ 2013-09-13 8:43 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1379061827-8595-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
When we have an LE link we should not respond to any data on the BR/EDR
L2CAP signaling channel (0x0001) and vice-versa when we have a BR/EDR
link we should not respond to LE L2CAP (CID 0x0005) signaling commands.
This patch fixes this issue by checking for a valid link type and
ignores data if it is wrong.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/l2cap_core.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index ea3792f..1d03644 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -5297,6 +5297,7 @@ static inline int l2cap_le_sig_cmd(struct l2cap_conn *conn,
static inline void l2cap_le_sig_channel(struct l2cap_conn *conn,
struct sk_buff *skb)
{
+ struct hci_conn *hcon = conn->hcon;
u8 *data = skb->data;
int len = skb->len;
struct l2cap_cmd_hdr cmd;
@@ -5304,6 +5305,9 @@ static inline void l2cap_le_sig_channel(struct l2cap_conn *conn,
l2cap_raw_recv(conn, skb);
+ if (hcon->type != LE_LINK)
+ return;
+
while (len >= L2CAP_CMD_HDR_SIZE) {
u16 cmd_len;
memcpy(&cmd, data, L2CAP_CMD_HDR_SIZE);
@@ -5342,6 +5346,7 @@ static inline void l2cap_le_sig_channel(struct l2cap_conn *conn,
static inline void l2cap_sig_channel(struct l2cap_conn *conn,
struct sk_buff *skb)
{
+ struct hci_conn *hcon = conn->hcon;
u8 *data = skb->data;
int len = skb->len;
struct l2cap_cmd_hdr cmd;
@@ -5349,6 +5354,9 @@ static inline void l2cap_sig_channel(struct l2cap_conn *conn,
l2cap_raw_recv(conn, skb);
+ if (hcon->type != ACL_LINK)
+ return;
+
while (len >= L2CAP_CMD_HDR_SIZE) {
u16 cmd_len;
memcpy(&cmd, data, L2CAP_CMD_HDR_SIZE);
--
1.8.4.rc3
^ permalink raw reply related
* [PATCH 3/3] Bluetooth: Fix waiting for clearing of BT_SK_SUSPEND flag
From: johan.hedberg @ 2013-09-13 8:43 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1379061827-8595-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
In the case of blocking sockets we should not proceed with sendmsg() if
the socket has the BT_SK_SUSPEND flag set. So far the code was only
ensuring that POLLOUT doesn't get set for non-blocking sockets using
poll() but there was no code in place to ensure that blocking sockets do
the right thing when writing to them.
This patch adds a new bt_sock_wait_unsuspend helper function to sleep in
the sendmsg call if the BT_SK_SUSPEND flag is set, and wake up as soon
as it is unset. It also updates the L2CAP and RFCOMM sendmsg callbacks
to take advantage of this new helper function.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
include/net/bluetooth/bluetooth.h | 1 +
net/bluetooth/af_bluetooth.c | 38 ++++++++++++++++++++++++++++++++++++++
net/bluetooth/l2cap_sock.c | 6 ++++++
net/bluetooth/rfcomm/sock.c | 7 +++++--
4 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 10d43d8..3299d42 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -249,6 +249,7 @@ int bt_sock_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
uint bt_sock_poll(struct file *file, struct socket *sock, poll_table *wait);
int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo);
+int bt_sock_wait_unsuspend(struct sock *sk, unsigned long flags);
void bt_accept_enqueue(struct sock *parent, struct sock *sk);
void bt_accept_unlink(struct sock *sk);
diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index 9096137..624f866 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -525,6 +525,44 @@ int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo)
}
EXPORT_SYMBOL(bt_sock_wait_state);
+int bt_sock_wait_unsuspend(struct sock *sk, unsigned long flags)
+{
+ DECLARE_WAITQUEUE(wait, current);
+ unsigned long timeo;
+ int err = 0;
+
+ BT_DBG("sk %p", sk);
+
+ timeo = sock_sndtimeo(sk, flags & O_NONBLOCK);
+
+ add_wait_queue(sk_sleep(sk), &wait);
+ set_current_state(TASK_INTERRUPTIBLE);
+ while (test_bit(BT_SK_SUSPEND, &bt_sk(sk)->flags)) {
+ if (!timeo) {
+ err = -EAGAIN;
+ break;
+ }
+
+ if (signal_pending(current)) {
+ err = sock_intr_errno(timeo);
+ break;
+ }
+
+ release_sock(sk);
+ timeo = schedule_timeout(timeo);
+ lock_sock(sk);
+ set_current_state(TASK_INTERRUPTIBLE);
+
+ err = sock_error(sk);
+ if (err)
+ break;
+ }
+ __set_current_state(TASK_RUNNING);
+ remove_wait_queue(sk_sleep(sk), &wait);
+ return err;
+}
+EXPORT_SYMBOL(bt_sock_wait_unsuspend);
+
#ifdef CONFIG_PROC_FS
struct bt_seq_state {
struct bt_sock_list *l;
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 0098af8..1ddbc1a 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -777,6 +777,12 @@ static int l2cap_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
if (sk->sk_state != BT_CONNECTED)
return -ENOTCONN;
+ lock_sock(sk);
+ err = bt_sock_wait_unsuspend(sk, msg->msg_flags);
+ release_sock(sk);
+ if (err)
+ return err;
+
l2cap_chan_lock(chan);
err = l2cap_chan_send(chan, msg, len, sk->sk_priority);
l2cap_chan_unlock(chan);
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 30b3721..4203ec4 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -544,7 +544,7 @@ static int rfcomm_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
struct sock *sk = sock->sk;
struct rfcomm_dlc *d = rfcomm_pi(sk)->dlc;
struct sk_buff *skb;
- int sent = 0;
+ int err, sent = 0;
if (test_bit(RFCOMM_DEFER_SETUP, &d->flags))
return -ENOTCONN;
@@ -559,9 +559,12 @@ static int rfcomm_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
lock_sock(sk);
+ err = bt_sock_wait_unsuspend(sk, msg->msg_flags);
+ if (err)
+ return err;
+
while (len) {
size_t size = min_t(size_t, len, d->mtu);
- int err;
skb = sock_alloc_send_skb(sk, size + RFCOMM_SKB_RESERVE,
msg->msg_flags & MSG_DONTWAIT, &err);
--
1.8.4.rc3
^ permalink raw reply related
* [PATCH 0/4] obexd: Fix setting message folder
From: Christian Fetzer @ 2013-09-13 9:23 UTC (permalink / raw)
To: linux-bluetooth
From: Christian Fetzer <christian.fetzer@bmw-carit.de>
This fixes an issue that the folder property for messages was set incorrectly
when ListMessages got called with a subfolder parameter.
Calling ListMessages within /telecom/msg for the subfolder inbox would set the
folder property to /telecom/msg instead of /telecom/msg/inbox.
The patchset therefore changes map_msg_create to not set the folder property
to the current folder, but to a folder given as parameter. This change will be
needed as well for new message notifications (that can indicate new messages
for any folder).
In addition I've updated the documentation to clarify that the folder property
for ListMessages and PushMessage can be used only for a subfolder of the
current folder. It's not possible to specify an absolute or relative path.
Christian Fetzer (4):
obexd: Add folder property to map_msg_create
obexd: Fix setting message folder for relative folder in ListMessages
obexd: Clarify the folder property of ListMessages
obexd: Clarify the folder property of PushMessage
doc/obex-api.txt | 8 +++++---
obexd/client/map.c | 23 ++++++++++++++++++++---
2 files changed, 25 insertions(+), 6 deletions(-)
--
1.8.3.4
^ permalink raw reply
* [PATCH 1/4] obexd: Add folder property to map_msg_create
From: Christian Fetzer @ 2013-09-13 9:23 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1379064212-30056-1-git-send-email-christian.fetzer@oss.bmw-carit.de>
From: Christian Fetzer <christian.fetzer@bmw-carit.de>
Message interfaces are not necessarily created for the current folder,
therefore the folder needs to be specified in a parameter.
For example, messages can be created for sub folders when using the folder
parameter in ListMessages.
---
obexd/client/map.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/obexd/client/map.c b/obexd/client/map.c
index f0dcf72..9a1b140 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -779,7 +779,8 @@ static const GDBusPropertyTable map_msg_properties[] = {
{ }
};
-static struct map_msg *map_msg_create(struct map_data *data, const char *handle)
+static struct map_msg *map_msg_create(struct map_data *data, const char *handle,
+ const char *folder)
{
struct map_msg *msg;
@@ -788,7 +789,7 @@ static struct map_msg *map_msg_create(struct map_data *data, const char *handle)
msg->path = g_strdup_printf("%s/message%s",
obc_session_get_path(data->session),
handle);
- msg->folder = g_strdup(obc_session_get_folder(data->session));
+ msg->folder = g_strdup(folder);
if (!g_dbus_register_interface(conn, msg->path, MAP_MSG_INTERFACE,
map_msg_methods, NULL,
@@ -1057,7 +1058,8 @@ static void msg_element(GMarkupParseContext *ctxt, const char *element,
msg = g_hash_table_lookup(data->messages, values[i]);
if (msg == NULL) {
- msg = map_msg_create(data, values[i]);
+ msg = map_msg_create(data, values[i],
+ obc_session_get_folder(data->session));
if (msg == NULL)
return;
}
--
1.8.3.4
^ permalink raw reply related
* [PATCH 2/4] obexd: Fix setting message folder for relative folder in ListMessages
From: Christian Fetzer @ 2013-09-13 9:23 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1379064212-30056-1-git-send-email-christian.fetzer@oss.bmw-carit.de>
From: Christian Fetzer <christian.fetzer@bmw-carit.de>
The method ListMessages allows to specify a relative subfolder.
This subfolder needs to be added to the current path when registering
a new message interface.
---
obexd/client/map.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/obexd/client/map.c b/obexd/client/map.c
index 9a1b140..54011d8 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -96,6 +96,7 @@ static const char * const filter_list[] = {
struct map_data {
struct obc_session *session;
DBusMessage *msg;
+ char *folder;
GHashTable *messages;
int16_t mas_instance_id;
uint8_t supported_message_types;
@@ -1058,8 +1059,7 @@ static void msg_element(GMarkupParseContext *ctxt, const char *element,
msg = g_hash_table_lookup(data->messages, values[i]);
if (msg == NULL) {
- msg = map_msg_create(data, values[i],
- obc_session_get_folder(data->session));
+ msg = map_msg_create(data, values[i], data->folder);
if (msg == NULL)
return;
}
@@ -1153,6 +1153,19 @@ static void message_listing_cb(struct obc_session *session,
done:
g_dbus_send_message(conn, reply);
dbus_message_unref(map->msg);
+ g_free(map->folder);
+ map->folder = NULL;
+}
+
+static char *get_absolute_folder(const char *root, const char *subfolder)
+{
+ if (!subfolder || strlen(subfolder) == 0)
+ return g_strdup(root);
+ else
+ if (g_str_has_suffix(root, "/"))
+ return g_strconcat(root, subfolder, NULL);
+ else
+ return g_strconcat(root, "/", subfolder, NULL);
}
static DBusMessage *get_message_listing(struct map_data *map,
@@ -1175,6 +1188,8 @@ static DBusMessage *get_message_listing(struct map_data *map,
if (obc_session_queue(map->session, transfer, message_listing_cb, map,
&err)) {
map->msg = dbus_message_ref(message);
+ map->folder = get_absolute_folder(obc_session_get_folder(
+ map->session), folder);
return NULL;
}
--
1.8.3.4
^ permalink raw reply related
* [PATCH 3/4] obexd: Clarify the folder property of ListMessages
From: Christian Fetzer @ 2013-09-13 9:23 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1379064212-30056-1-git-send-email-christian.fetzer@oss.bmw-carit.de>
From: Christian Fetzer <christian.fetzer@bmw-carit.de>
The folder property of ListMessages does not accept path information,
it allows only to request the messages of a subfolder of the current
folder.
---
doc/obex-api.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/doc/obex-api.txt b/doc/obex-api.txt
index 0a8e632..28343f6 100644
--- a/doc/obex-api.txt
+++ b/doc/obex-api.txt
@@ -538,7 +538,8 @@ Methods void SetFolder(string name)
array{object, dict} ListMessages(string folder, dict filter)
Returns an array containing the messages found in the
- given folder.
+ given subfolder of the current folder, or in the
+ current folder if folder is empty.
Possible Filters: Offset, MaxCount, SubjectLength, Fields,
Type, PeriodStart, PeriodEnd, Status, Recipient, Sender,
--
1.8.3.4
^ permalink raw reply related
* [PATCH 4/4] obexd: Clarify the folder property of PushMessage
From: Christian Fetzer @ 2013-09-13 9:23 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1379064212-30056-1-git-send-email-christian.fetzer@oss.bmw-carit.de>
From: Christian Fetzer <christian.fetzer@bmw-carit.de>
The folder property of PushMessages does not accept path information,
it allows only to request the messages to be added to a subfolder of the
current folder.
---
doc/obex-api.txt | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/doc/obex-api.txt b/doc/obex-api.txt
index 28343f6..eda7cb9 100644
--- a/doc/obex-api.txt
+++ b/doc/obex-api.txt
@@ -638,8 +638,9 @@ Methods void SetFolder(string name)
Transfer a message (in bMessage format) to the
remote device.
- The message is transferred either to the given folder,
- or to the current folder if folder is omitted.
+ The message is transferred either to the given
+ subfolder of the current folder, or to the current
+ folder if folder is empty.
Possible args: Transparent, Retry, Charset
--
1.8.3.4
^ permalink raw reply related
* Re: [PATCH bluez] bnep: don't error() if kernel lacks bnep module
From: Johan Hedberg @ 2013-09-13 10:01 UTC (permalink / raw)
To: David Herrmann; +Cc: linux-bluetooth, Marcel Holtmann
In-Reply-To: <1378477387-13186-1-git-send-email-dh.herrmann@gmail.com>
Hi David,
On Fri, Sep 06, 2013, David Herrmann wrote:
> If a user disables bnep kernel support, they normally do that on purpose.
> It's misleading to print an error during module-startup in this situation.
> Therefore, only print a hint that bnep-support is missing if
> EPROTONOSUPPORT is returned by the kernel.
>
> Furthermore, allow modules to forward ENOSYS as error to bluetoothd core
> to handle it as "module is not compatible with system setup" instead of an
> error. ENOSYS is commonly used to signal "missing kernel infrastructure"
> so it seems appropriate here.
> ---
> profiles/network/common.c | 7 ++++++-
> profiles/network/manager.c | 17 ++++++++++++++---
> src/plugin.c | 11 +++++++++--
> 3 files changed, 29 insertions(+), 6 deletions(-)
First of all, sorry for the delay with reviewing this. I forgot about it
and only bumped into it now when checking what I'd missed from the last
month in my linux-bluetooth folder.
> diff --git a/profiles/network/common.c b/profiles/network/common.c
> index e069892..17bff30 100644
> --- a/profiles/network/common.c
> +++ b/profiles/network/common.c
> @@ -110,8 +110,13 @@ int bnep_init(void)
>
> if (ctl < 0) {
> int err = -errno;
> - error("Failed to open control socket: %s (%d)",
> +
> + if (err == -EPROTONOSUPPORT)
> + info("kernel lacks bnep-protocol support");
I think warn() might be more appropriate here.
> diff --git a/profiles/network/manager.c b/profiles/network/manager.c
> index 03b1b3d..7697766 100644
> --- a/profiles/network/manager.c
> +++ b/profiles/network/manager.c
> @@ -25,6 +25,7 @@
> #include <config.h>
> #endif
>
> +#include <errno.h>
> #include <stdbool.h>
>
> #include <bluetooth/bluetooth.h>
> @@ -169,11 +170,21 @@ static struct btd_profile nap_profile = {
>
> static int network_init(void)
> {
> + int r;
Please use "err". That's the convention for integer error variables.
> + r = bnep_init();
> + if (r) {
> + if (r == -EPROTONOSUPPORT) {
> + info("bnep module not available, disabling plugin");
> + r = -ENOSYS;
> + } else {
> + error("Can't init bnep module");
> + r = -1;
I think it'd make sense to just pass forward whatever error bnep_init()
returned instead of mapping to -1 (which isn't even a valid error code -
we should fix this in several places of the existing code base).
> diff --git a/src/plugin.c b/src/plugin.c
> index 51c98bc..396c8af 100644
> --- a/src/plugin.c
> +++ b/src/plugin.c
> @@ -119,6 +119,7 @@ gboolean plugin_init(const char *enable, const char *disable)
> const char *file;
> char **cli_disabled, **cli_enabled;
> unsigned int i;
> + int r;
Again, please use "err".
> /* Make a call to BtIO API so its symbols got resolved before the
> * plugins are loaded. */
> @@ -196,8 +197,14 @@ start:
> for (list = plugins; list; list = list->next) {
> struct bluetooth_plugin *plugin = list->data;
>
> - if (plugin->desc->init() < 0) {
> - error("Failed to init %s plugin", plugin->desc->name);
> + r = plugin->desc->init();
> + if (r < 0) {
> + if (r == -ENOSYS)
> + info("System does not support %s plugin",
> + plugin->desc->name);
warn() might be more appropriate here.
> + else
> + error("Failed to init %s plugin",
> + plugin->desc->name);
> continue;
> }
In general this plugin.c update should probably go into its own patch
that precedes the network plugin patch.
Johan
^ permalink raw reply
* Re: [PATCH 1/4] obexd: Add folder property to map_msg_create
From: Luiz Augusto von Dentz @ 2013-09-13 10:11 UTC (permalink / raw)
To: Christian Fetzer; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1379064212-30056-2-git-send-email-christian.fetzer@oss.bmw-carit.de>
Hi Christian,
On Fri, Sep 13, 2013 at 12:23 PM, Christian Fetzer
<christian.fetzer@oss.bmw-carit.de> wrote:
> From: Christian Fetzer <christian.fetzer@bmw-carit.de>
>
> Message interfaces are not necessarily created for the current folder,
> therefore the folder needs to be specified in a parameter.
>
> For example, messages can be created for sub folders when using the folder
> parameter in ListMessages.
> ---
> obexd/client/map.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/obexd/client/map.c b/obexd/client/map.c
> index f0dcf72..9a1b140 100644
> --- a/obexd/client/map.c
> +++ b/obexd/client/map.c
> @@ -779,7 +779,8 @@ static const GDBusPropertyTable map_msg_properties[] = {
> { }
> };
>
> -static struct map_msg *map_msg_create(struct map_data *data, const char *handle)
> +static struct map_msg *map_msg_create(struct map_data *data, const char *handle,
> + const char *folder)
> {
> struct map_msg *msg;
>
> @@ -788,7 +789,7 @@ static struct map_msg *map_msg_create(struct map_data *data, const char *handle)
> msg->path = g_strdup_printf("%s/message%s",
> obc_session_get_path(data->session),
> handle);
> - msg->folder = g_strdup(obc_session_get_folder(data->session));
> + msg->folder = g_strdup(folder);
>
> if (!g_dbus_register_interface(conn, msg->path, MAP_MSG_INTERFACE,
> map_msg_methods, NULL,
> @@ -1057,7 +1058,8 @@ static void msg_element(GMarkupParseContext *ctxt, const char *element,
>
> msg = g_hash_table_lookup(data->messages, values[i]);
> if (msg == NULL) {
> - msg = map_msg_create(data, values[i]);
> + msg = map_msg_create(data, values[i],
> + obc_session_get_folder(data->session));
Is this really fixing anything? Because it seems it is just changing
places where obc_session_get_folder is called when what you should
probably be doing is to store the folder parameter given to
ListMessages. I actually regret to have this parameter as it makes
things a little bit more complicated just to avoid SetFolder.
Probably Session.SetPath would make more sense than having each
profile interface treating it differently but that would require to
break APIs so it is better not to do it right now.
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH bluez] bnep: don't error() if kernel lacks bnep module
From: David Herrmann @ 2013-09-13 10:38 UTC (permalink / raw)
To: David Herrmann, linux-bluetooth@vger.kernel.org, Marcel Holtmann,
Johan Hedberg
In-Reply-To: <20130913100142.GB10346@x220.p-661hnu-f1>
Hi
On Fri, Sep 13, 2013 at 12:01 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi David,
>
> On Fri, Sep 06, 2013, David Herrmann wrote:
>> If a user disables bnep kernel support, they normally do that on purpose.
>> It's misleading to print an error during module-startup in this situation.
>> Therefore, only print a hint that bnep-support is missing if
>> EPROTONOSUPPORT is returned by the kernel.
>>
>> Furthermore, allow modules to forward ENOSYS as error to bluetoothd core
>> to handle it as "module is not compatible with system setup" instead of an
>> error. ENOSYS is commonly used to signal "missing kernel infrastructure"
>> so it seems appropriate here.
>> ---
>> profiles/network/common.c | 7 ++++++-
>> profiles/network/manager.c | 17 ++++++++++++++---
>> src/plugin.c | 11 +++++++++--
>> 3 files changed, 29 insertions(+), 6 deletions(-)
>
> First of all, sorry for the delay with reviewing this. I forgot about it
> and only bumped into it now when checking what I'd missed from the last
> month in my linux-bluetooth folder.
>
>> diff --git a/profiles/network/common.c b/profiles/network/common.c
>> index e069892..17bff30 100644
>> --- a/profiles/network/common.c
>> +++ b/profiles/network/common.c
>> @@ -110,8 +110,13 @@ int bnep_init(void)
>>
>> if (ctl < 0) {
>> int err = -errno;
>> - error("Failed to open control socket: %s (%d)",
>> +
>> + if (err == -EPROTONOSUPPORT)
>> + info("kernel lacks bnep-protocol support");
>
> I think warn() might be more appropriate here.
Using warn() defeats the whole purpose of this patch. I want to run
bluetoothd without getting a warning or error, and obviously, there is
nothing to warn _me_ about. I'm ok if you want err() or warn() here so
other users get warned, but in that case we should just drop the
patch.
So it's up to you to decide. If info() is ok, I will fix the issues
you mentioned below, otherwise just drop it.
Cheers
David
>> diff --git a/profiles/network/manager.c b/profiles/network/manager.c
>> index 03b1b3d..7697766 100644
>> --- a/profiles/network/manager.c
>> +++ b/profiles/network/manager.c
>> @@ -25,6 +25,7 @@
>> #include <config.h>
>> #endif
>>
>> +#include <errno.h>
>> #include <stdbool.h>
>>
>> #include <bluetooth/bluetooth.h>
>> @@ -169,11 +170,21 @@ static struct btd_profile nap_profile = {
>>
>> static int network_init(void)
>> {
>> + int r;
>
> Please use "err". That's the convention for integer error variables.
>
>> + r = bnep_init();
>> + if (r) {
>> + if (r == -EPROTONOSUPPORT) {
>> + info("bnep module not available, disabling plugin");
>> + r = -ENOSYS;
>> + } else {
>> + error("Can't init bnep module");
>> + r = -1;
>
> I think it'd make sense to just pass forward whatever error bnep_init()
> returned instead of mapping to -1 (which isn't even a valid error code -
> we should fix this in several places of the existing code base).
>
>> diff --git a/src/plugin.c b/src/plugin.c
>> index 51c98bc..396c8af 100644
>> --- a/src/plugin.c
>> +++ b/src/plugin.c
>> @@ -119,6 +119,7 @@ gboolean plugin_init(const char *enable, const char *disable)
>> const char *file;
>> char **cli_disabled, **cli_enabled;
>> unsigned int i;
>> + int r;
>
> Again, please use "err".
>
>> /* Make a call to BtIO API so its symbols got resolved before the
>> * plugins are loaded. */
>> @@ -196,8 +197,14 @@ start:
>> for (list = plugins; list; list = list->next) {
>> struct bluetooth_plugin *plugin = list->data;
>>
>> - if (plugin->desc->init() < 0) {
>> - error("Failed to init %s plugin", plugin->desc->name);
>> + r = plugin->desc->init();
>> + if (r < 0) {
>> + if (r == -ENOSYS)
>> + info("System does not support %s plugin",
>> + plugin->desc->name);
>
> warn() might be more appropriate here.
>
>> + else
>> + error("Failed to init %s plugin",
>> + plugin->desc->name);
>> continue;
>> }
>
> In general this plugin.c update should probably go into its own patch
> that precedes the network plugin patch.
>
> Johan
>
>
^ permalink raw reply
* Re: [PATCH 3/3] Bluetooth: Fix waiting for clearing of BT_SK_SUSPEND flag
From: Anderson Lizardo @ 2013-09-13 10:41 UTC (permalink / raw)
To: Johan Hedberg; +Cc: BlueZ development
In-Reply-To: <1379061827-8595-4-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
On Fri, Sep 13, 2013 at 4:43 AM, <johan.hedberg@gmail.com> wrote:
> +int bt_sock_wait_unsuspend(struct sock *sk, unsigned long flags)
> +{
> + DECLARE_WAITQUEUE(wait, current);
> + unsigned long timeo;
> + int err = 0;
> +
> + BT_DBG("sk %p", sk);
> +
> + timeo = sock_sndtimeo(sk, flags & O_NONBLOCK);
> +
> + add_wait_queue(sk_sleep(sk), &wait);
> + set_current_state(TASK_INTERRUPTIBLE);
> + while (test_bit(BT_SK_SUSPEND, &bt_sk(sk)->flags)) {
> + if (!timeo) {
> + err = -EAGAIN;
> + break;
> + }
> +
> + if (signal_pending(current)) {
> + err = sock_intr_errno(timeo);
> + break;
> + }
> +
> + release_sock(sk);
> + timeo = schedule_timeout(timeo);
> + lock_sock(sk);
> + set_current_state(TASK_INTERRUPTIBLE);
> +
> + err = sock_error(sk);
> + if (err)
> + break;
> + }
> + __set_current_state(TASK_RUNNING);
> + remove_wait_queue(sk_sleep(sk), &wait);
> + return err;
> +}
You can add a blank line before "return err" for slightly better legibility.
> @@ -559,9 +559,12 @@ static int rfcomm_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
>
> lock_sock(sk);
>
> + err = bt_sock_wait_unsuspend(sk, msg->msg_flags);
> + if (err)
> + return err;
> +
Is it okay to return without releasing the lock?
Best Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [PATCH 1/1] obexd: Use documented values in org.bluez.obex.Message1 Type property
From: Luiz Augusto von Dentz @ 2013-09-13 10:43 UTC (permalink / raw)
To: Christian Fetzer; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1378744257-648-1-git-send-email-christian.fetzer@oss.bmw-carit.de>
Hi Christian,
On Mon, Sep 9, 2013 at 7:30 PM, Christian Fetzer
<christian.fetzer@oss.bmw-carit.de> wrote:
> From: Christian Fetzer <christian.fetzer@bmw-carit.de>
>
> This updates the values that are presented in the Type property to use
> the values from the documentation ("email", "sms-gsm", "sms-cdma", "mms").
> The existing code directly used the values as received in the messages
> listing object ("EMAIL", "SMS_GSM", "SMS_CDMA", "MMS").
> ---
> obexd/client/map.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/obexd/client/map.c b/obexd/client/map.c
> index f0dcf72..95f0334 100644
> --- a/obexd/client/map.c
> +++ b/obexd/client/map.c
> @@ -894,7 +894,17 @@ static void parse_type(struct map_msg *msg, const char *value)
> return;
>
> g_free(msg->type);
> - msg->type = g_strdup(value);
> +
> + if (strcasecmp(value, "SMS_GSM") == 0)
> + msg->type = g_strdup("sms-gsm");
> + else if (strcasecmp(value, "SMS_CDMA") == 0)
> + msg->type = g_strdup("sms-cdma");
> + else if (strcasecmp(value, "EMAIL") == 0)
> + msg->type = g_strdup("email");
> + else if (strcasecmp(value, "MMS") == 0)
> + msg->type = g_strdup("mms");
> + else
> + msg->type = NULL;
>
> g_dbus_emit_property_changed(conn, msg->path,
> MAP_MSG_INTERFACE, "Type");
> --
> 1.8.3.4
Applied, thanks.
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH 2/4] obexd: Fix setting message folder for relative folder in ListMessages
From: Luiz Augusto von Dentz @ 2013-09-13 10:47 UTC (permalink / raw)
To: Christian Fetzer; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1379064212-30056-3-git-send-email-christian.fetzer@oss.bmw-carit.de>
Hi Christian,
On Fri, Sep 13, 2013 at 12:23 PM, Christian Fetzer
<christian.fetzer@oss.bmw-carit.de> wrote:
> From: Christian Fetzer <christian.fetzer@bmw-carit.de>
>
> The method ListMessages allows to specify a relative subfolder.
> This subfolder needs to be added to the current path when registering
> a new message interface.
> ---
> obexd/client/map.c | 19 +++++++++++++++++--
> 1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/obexd/client/map.c b/obexd/client/map.c
> index 9a1b140..54011d8 100644
> --- a/obexd/client/map.c
> +++ b/obexd/client/map.c
> @@ -96,6 +96,7 @@ static const char * const filter_list[] = {
> struct map_data {
> struct obc_session *session;
> DBusMessage *msg;
> + char *folder;
> GHashTable *messages;
> int16_t mas_instance_id;
> uint8_t supported_message_types;
> @@ -1058,8 +1059,7 @@ static void msg_element(GMarkupParseContext *ctxt, const char *element,
>
> msg = g_hash_table_lookup(data->messages, values[i]);
> if (msg == NULL) {
> - msg = map_msg_create(data, values[i],
> - obc_session_get_folder(data->session));
> + msg = map_msg_create(data, values[i], data->folder);
> if (msg == NULL)
> return;
> }
> @@ -1153,6 +1153,19 @@ static void message_listing_cb(struct obc_session *session,
> done:
> g_dbus_send_message(conn, reply);
> dbus_message_unref(map->msg);
> + g_free(map->folder);
> + map->folder = NULL;
> +}
> +
> +static char *get_absolute_folder(const char *root, const char *subfolder)
> +{
> + if (!subfolder || strlen(subfolder) == 0)
> + return g_strdup(root);
> + else
> + if (g_str_has_suffix(root, "/"))
> + return g_strconcat(root, subfolder, NULL);
> + else
> + return g_strconcat(root, "/", subfolder, NULL);
> }
>
> static DBusMessage *get_message_listing(struct map_data *map,
> @@ -1175,6 +1188,8 @@ static DBusMessage *get_message_listing(struct map_data *map,
> if (obc_session_queue(map->session, transfer, message_listing_cb, map,
> &err)) {
> map->msg = dbus_message_ref(message);
> + map->folder = get_absolute_folder(obc_session_get_folder(
> + map->session), folder);
> return NULL;
> }
>
> --
> 1.8.3.4
This will probably not work in case of multiple outstanding requests
the last will always overwrite the folder, which btw will leak, so
probably we need a per request data.
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH bluez] bnep: don't error() if kernel lacks bnep module
From: Johan Hedberg @ 2013-09-13 10:58 UTC (permalink / raw)
To: David Herrmann; +Cc: linux-bluetooth@vger.kernel.org, Marcel Holtmann
In-Reply-To: <CANq1E4QwzGfc0FMN19fLDpEz3GuJ7GeW60aKudWCtPpj-kxcUA@mail.gmail.com>
Hi David,
On Fri, Sep 13, 2013, David Herrmann wrote:
> On Fri, Sep 13, 2013 at 12:01 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> > Hi David,
> >
> > On Fri, Sep 06, 2013, David Herrmann wrote:
> >> If a user disables bnep kernel support, they normally do that on purpose.
> >> It's misleading to print an error during module-startup in this situation.
> >> Therefore, only print a hint that bnep-support is missing if
> >> EPROTONOSUPPORT is returned by the kernel.
> >>
> >> Furthermore, allow modules to forward ENOSYS as error to bluetoothd core
> >> to handle it as "module is not compatible with system setup" instead of an
> >> error. ENOSYS is commonly used to signal "missing kernel infrastructure"
> >> so it seems appropriate here.
> >> ---
> >> profiles/network/common.c | 7 ++++++-
> >> profiles/network/manager.c | 17 ++++++++++++++---
> >> src/plugin.c | 11 +++++++++--
> >> 3 files changed, 29 insertions(+), 6 deletions(-)
> >
> > First of all, sorry for the delay with reviewing this. I forgot about it
> > and only bumped into it now when checking what I'd missed from the last
> > month in my linux-bluetooth folder.
> >
> >> diff --git a/profiles/network/common.c b/profiles/network/common.c
> >> index e069892..17bff30 100644
> >> --- a/profiles/network/common.c
> >> +++ b/profiles/network/common.c
> >> @@ -110,8 +110,13 @@ int bnep_init(void)
> >>
> >> if (ctl < 0) {
> >> int err = -errno;
> >> - error("Failed to open control socket: %s (%d)",
> >> +
> >> + if (err == -EPROTONOSUPPORT)
> >> + info("kernel lacks bnep-protocol support");
> >
> > I think warn() might be more appropriate here.
>
> Using warn() defeats the whole purpose of this patch. I want to run
> bluetoothd without getting a warning or error, and obviously, there is
> nothing to warn _me_ about. I'm ok if you want err() or warn() here so
> other users get warned, but in that case we should just drop the
> patch.
I thought the main idea was to have a clearer log message when the
module is not there. None of the warn/error/info messages can be
suppressed (unlike DBG which is off by default). To me warn() seems more
appropriate than info() since it's meaning is "this is something that
may or may not be of concern to the user".
> So it's up to you to decide. If info() is ok, I will fix the issues
> you mentioned below, otherwise just drop it.
I do think the patch has value even though info() is not used by having
an understandable log message for what's going on.
Johan
^ permalink raw reply
* Re: [PATCH 3/3] Bluetooth: Fix waiting for clearing of BT_SK_SUSPEND flag
From: Johan Hedberg @ 2013-09-13 11:01 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: BlueZ development
In-Reply-To: <CAJdJm_Pp87adXrnBEysFd99V+=cyXZmc65cF4MGJTQNHMyz90g@mail.gmail.com>
Hi Lizardo,
On Fri, Sep 13, 2013, Anderson Lizardo wrote:
> On Fri, Sep 13, 2013 at 4:43 AM, <johan.hedberg@gmail.com> wrote:
> > +int bt_sock_wait_unsuspend(struct sock *sk, unsigned long flags)
> > +{
> > + DECLARE_WAITQUEUE(wait, current);
> > + unsigned long timeo;
> > + int err = 0;
> > +
> > + BT_DBG("sk %p", sk);
> > +
> > + timeo = sock_sndtimeo(sk, flags & O_NONBLOCK);
> > +
> > + add_wait_queue(sk_sleep(sk), &wait);
> > + set_current_state(TASK_INTERRUPTIBLE);
> > + while (test_bit(BT_SK_SUSPEND, &bt_sk(sk)->flags)) {
> > + if (!timeo) {
> > + err = -EAGAIN;
> > + break;
> > + }
> > +
> > + if (signal_pending(current)) {
> > + err = sock_intr_errno(timeo);
> > + break;
> > + }
> > +
> > + release_sock(sk);
> > + timeo = schedule_timeout(timeo);
> > + lock_sock(sk);
> > + set_current_state(TASK_INTERRUPTIBLE);
> > +
> > + err = sock_error(sk);
> > + if (err)
> > + break;
> > + }
> > + __set_current_state(TASK_RUNNING);
> > + remove_wait_queue(sk_sleep(sk), &wait);
> > + return err;
> > +}
>
> You can add a blank line before "return err" for slightly better legibility.
Sure. The reason it's not there is that I followed the existing similar
bt_sock_wait_state function implementation.
> > @@ -559,9 +559,12 @@ static int rfcomm_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
> >
> > lock_sock(sk);
> >
> > + err = bt_sock_wait_unsuspend(sk, msg->msg_flags);
> > + if (err)
> > + return err;
> > +
>
> Is it okay to return without releasing the lock?
Nope, I'm pretty sure that's a bug. Good catch, I'll fix in a v2.
Johan
^ permalink raw reply
* Re: [PATCH bluez] bnep: don't error() if kernel lacks bnep module
From: David Herrmann @ 2013-09-13 11:07 UTC (permalink / raw)
To: David Herrmann, linux-bluetooth@vger.kernel.org, Marcel Holtmann
In-Reply-To: <20130913105816.GA15267@x220.p-661hnu-f1>
Hi
On Fri, Sep 13, 2013 at 12:58 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi David,
>
> On Fri, Sep 13, 2013, David Herrmann wrote:
>> On Fri, Sep 13, 2013 at 12:01 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
>> > Hi David,
>> >
>> > On Fri, Sep 06, 2013, David Herrmann wrote:
>> >> If a user disables bnep kernel support, they normally do that on purpose.
>> >> It's misleading to print an error during module-startup in this situation.
>> >> Therefore, only print a hint that bnep-support is missing if
>> >> EPROTONOSUPPORT is returned by the kernel.
>> >>
>> >> Furthermore, allow modules to forward ENOSYS as error to bluetoothd core
>> >> to handle it as "module is not compatible with system setup" instead of an
>> >> error. ENOSYS is commonly used to signal "missing kernel infrastructure"
>> >> so it seems appropriate here.
>> >> ---
>> >> profiles/network/common.c | 7 ++++++-
>> >> profiles/network/manager.c | 17 ++++++++++++++---
>> >> src/plugin.c | 11 +++++++++--
>> >> 3 files changed, 29 insertions(+), 6 deletions(-)
>> >
>> > First of all, sorry for the delay with reviewing this. I forgot about it
>> > and only bumped into it now when checking what I'd missed from the last
>> > month in my linux-bluetooth folder.
>> >
>> >> diff --git a/profiles/network/common.c b/profiles/network/common.c
>> >> index e069892..17bff30 100644
>> >> --- a/profiles/network/common.c
>> >> +++ b/profiles/network/common.c
>> >> @@ -110,8 +110,13 @@ int bnep_init(void)
>> >>
>> >> if (ctl < 0) {
>> >> int err = -errno;
>> >> - error("Failed to open control socket: %s (%d)",
>> >> +
>> >> + if (err == -EPROTONOSUPPORT)
>> >> + info("kernel lacks bnep-protocol support");
>> >
>> > I think warn() might be more appropriate here.
>>
>> Using warn() defeats the whole purpose of this patch. I want to run
>> bluetoothd without getting a warning or error, and obviously, there is
>> nothing to warn _me_ about. I'm ok if you want err() or warn() here so
>> other users get warned, but in that case we should just drop the
>> patch.
>
> I thought the main idea was to have a clearer log message when the
> module is not there. None of the warn/error/info messages can be
> suppressed (unlike DBG which is off by default). To me warn() seems more
> appropriate than info() since it's meaning is "this is something that
> may or may not be of concern to the user".
It's not about suppressing messages. It's about putting proper
attributes on them. If I look at my system-log, I am usually only
interested in errors and warnings. And each of them needs my attention
to get fixed. Otherwise, the purpose of an error or warning is missed.
If every running daemon spits warnings to the log which I cannot
"fix", warnings will no longer be any special. Same for errors,
obviously.
So imho every warning should be "fixable" by the administrator. And in
this case I don't think enabling BNEP in the kernel is the appropriate
fix. Instead, if an administrator disables BNEP, bluetoothd should
interpret that as a "configuration-choice".
>> So it's up to you to decide. If info() is ok, I will fix the issues
>> you mentioned below, otherwise just drop it.
>
> I do think the patch has value even though info() is not used by having
> an understandable log message for what's going on.
In that case I will resend it. Please let me know whether to use
info() or warn().
Thanks
David
^ 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