* Re: [PATCH] bluetooth: cmtp: fix information leak to userland
From: Marcel Holtmann @ 2010-11-02 15:35 UTC (permalink / raw)
To: Vasiliy Kulikov
Cc: kernel-janitors, Gustavo F. Padovan, David S. Miller,
Eric Dumazet, linux-bluetooth, netdev, linux-kernel
In-Reply-To: <1288448787-5848-1-git-send-email-segooon@gmail.com>
Hi Vasiliy,
> Structure cmtp_conninfo is copied to userland with some padding fields
> unitialized. It leads to leaking of contents of kernel stack memory.
>
> Signed-off-by: Vasiliy Kulikov <segooon@gmail.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH] bluetooth: bnep: fix information leak to userland
From: Marcel Holtmann @ 2010-11-02 15:35 UTC (permalink / raw)
To: Vasiliy Kulikov
Cc: kernel-janitors, Gustavo F. Padovan, David S. Miller,
Eric Dumazet, Thadeu Lima de Souza Cascardo, Tejun Heo,
Jiri Kosina, linux-bluetooth, netdev, linux-kernel
In-Reply-To: <1288448782-5582-1-git-send-email-segooon@gmail.com>
Hi Vasiiy,
> Structure bnep_conninfo is copied to userland with the field "device"
> that has the last elements unitialized. It leads to leaking of
> contents of kernel stack memory.
>
> Signed-off-by: Vasiliy Kulikov <segooon@gmail.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 3/6] MacBookAir3,1(3,2) btusb support
From: Dmitry Torokhov @ 2010-11-02 15:24 UTC (permalink / raw)
To: linux-bluetooth; +Cc: linux-kernel, gimli, Marcel Holtmann
In-Reply-To: <9e7738c0461aa509dd55fe470df3855e@mognix.dark-green.com>
Not sure of you guys monitor LKML...
On Tue, Nov 02, 2010 at 08:19:43AM +0100, gimli wrote:
> This patch add support for the MacBookAir3,1 and MacBookAir3,2 to the btusb
> driver.
>
> Signed-off-by: Edgar (gimli) Hucek <gimli@dark-green.com>
> --- a/drivers/bluetooth/btusb.c 2010-10-30 21:08:45.170492002 +0200
> +++ b/drivers/bluetooth/btusb.c 2010-10-30 21:18:11.820492000 +0200
> @@ -62,6 +62,9 @@
> /* Apple iMac11,1 */
> { USB_DEVICE(0x05ac, 0x8215) },
>
> + /* Apple MacBookAir3,1, MacBookAir3,2 */
> + { USB_DEVICE(0x05ac, 0x821b) },
> +
> /* AVM BlueFRITZ! USB v2.0 */
> { USB_DEVICE(0x057c, 0x3800) },
>
--
Dmitry
^ permalink raw reply
* Re: [PATCHv3 2/2] Bluetooth: timer check sk is not owned before freeing
From: Gustavo F. Padovan @ 2010-11-02 15:15 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <AANLkTiksc7jKPqxkFEZqvgBJ5ATvGh0nurWS9SZcdtit@mail.gmail.com>
Hi Andrei,
* Andrei Emeltchenko <andrei.emeltchenko.news@gmail.com> [2010-11-01 16:20:15 +0200]:
> Hi Gustavo
>
> On Sat, Oct 30, 2010 at 12:17 AM, Gustavo F. Padovan
> <padovan@profusion.mobi> wrote:
> > Hi Andrei,
> >
> > * Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2010-10-29 16:43:01 +0300]:
> >
> >> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> >>
> >> In timer context we might delete l2cap channel used by krfcommd.
> >> The check makes sure that sk is not owned. If sk is owned we
> >> restart timer for HZ/5.
> >>
> >> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> >> ---
> >> net/bluetooth/l2cap.c | 32 ++++++++++++++++++++------------
> >> 1 files changed, 20 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> >> index b1344d8..c67b3c6 100644
> >> --- a/net/bluetooth/l2cap.c
> >> +++ b/net/bluetooth/l2cap.c
> >> @@ -83,6 +83,18 @@ static struct sk_buff *l2cap_build_cmd(struct l2cap_conn *conn,
> >> static int l2cap_ertm_data_rcv(struct sock *sk, struct sk_buff *skb);
> >>
> >> /* ---- L2CAP timers ---- */
> >> +static void l2cap_sock_set_timer(struct sock *sk, long timeout)
> >> +{
> >> + BT_DBG("sk %p state %d timeout %ld", sk, sk->sk_state, timeout);
> >> + sk_reset_timer(sk, &sk->sk_timer, jiffies + timeout);
> >> +}
> >> +
> >> +static void l2cap_sock_clear_timer(struct sock *sk)
> >> +{
> >> + BT_DBG("sock %p state %d", sk, sk->sk_state);
> >> + sk_stop_timer(sk, &sk->sk_timer);
> >> +}
> >> +
> >> static void l2cap_sock_timeout(unsigned long arg)
> >> {
> >> struct sock *sk = (struct sock *) arg;
> >> @@ -92,6 +104,14 @@ static void l2cap_sock_timeout(unsigned long arg)
> >>
> >> bh_lock_sock(sk);
> >>
> >> + if (sock_owned_by_user(sk)) {
> >> + /* sk is owned by user. Try again later */
> >> + l2cap_sock_set_timer(sk, HZ / 5);
> >> + bh_unlock_sock(sk);
> >> + sock_put(sk);
> >
> > You can't do a sock_put() here, you have to keep the referencee to the
> > socket while the timer is enabled.
>
> sk_reset_timer is holding sock when timer restarts. The same way done
> in TCP code in function:
> static void tcp_delack_timer(unsigned long data)
Yes, I got confused, you're right.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* [PATCH 7/7] Bluetooth: Fix not returning proper error in RFCOMM
From: Gustavo F. Padovan @ 2010-11-02 15:03 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1288710198-6108-6-git-send-email-padovan@profusion.mobi>
Return 0 in that situation could lead to errors in the caller.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
net/bluetooth/rfcomm/sock.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index e48fbca..cd7e27a 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -930,7 +930,7 @@ int rfcomm_connect_ind(struct rfcomm_session *s, u8 channel, struct rfcomm_dlc *
/* Check if we have socket listening on channel */
parent = rfcomm_get_sock_by_channel(BT_LISTEN, channel, &src);
if (!parent)
- return 0;
+ return -EINVAL;
bh_lock_sock(parent);
--
1.7.3.1
^ permalink raw reply related
* [PATCH 6/7] Bluetooth: Fix not returning proper error in SCO
From: Gustavo F. Padovan @ 2010-11-02 15:03 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1288710198-6108-5-git-send-email-padovan@profusion.mobi>
Return 0 in that situation could lead to errors in the caller.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
net/bluetooth/sco.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 85b5498..f031b62 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -880,7 +880,7 @@ static int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 type)
int lm = 0;
if (type != SCO_LINK && type != ESCO_LINK)
- return 0;
+ return -EINVAL;
BT_DBG("hdev %s, bdaddr %s", hdev->name, batostr(bdaddr));
@@ -906,7 +906,7 @@ static int sco_connect_cfm(struct hci_conn *hcon, __u8 status)
BT_DBG("hcon %p bdaddr %s status %d", hcon, batostr(&hcon->dst), status);
if (hcon->type != SCO_LINK && hcon->type != ESCO_LINK)
- return 0;
+ return -EINVAL;
if (!status) {
struct sco_conn *conn;
@@ -925,7 +925,7 @@ static int sco_disconn_cfm(struct hci_conn *hcon, __u8 reason)
BT_DBG("hcon %p reason %d", hcon, reason);
if (hcon->type != SCO_LINK && hcon->type != ESCO_LINK)
- return 0;
+ return -EINVAL;
sco_conn_del(hcon, bt_err(reason));
--
1.7.3.1
^ permalink raw reply related
* [PATCH 5/7] Bluetooth: Get ride of __rfcomm_get_sock_by_channel()
From: Gustavo F. Padovan @ 2010-11-02 15:03 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1288710198-6108-4-git-send-email-padovan@profusion.mobi>
rfcomm_get_sock_by_channel() was the only user of this function, so I merged
both into rfcomm_get_sock_by_channel(). The socket lock now should be hold
outside of rfcomm_get_sock_by_channel() once we hold and release it inside the
same function now.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
net/bluetooth/rfcomm/sock.c | 19 +++++++------------
1 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 4ed9499..e48fbca 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -142,11 +142,13 @@ static struct sock *rfcomm_get_sock_by_addr(u8 channel, bdaddr_t *src)
/* Find socket with channel and source bdaddr.
* Returns closest match.
*/
-static struct sock *__rfcomm_get_sock_by_channel(int state, u8 channel, bdaddr_t *src)
+static struct sock *rfcomm_get_sock_by_channel(int state, u8 channel, bdaddr_t *src)
{
struct sock *sk = NULL, *sk1 = NULL;
struct hlist_node *node;
+ read_lock(&rfcomm_sk_list.lock);
+
sk_for_each(sk, node, &rfcomm_sk_list.head) {
if (state && sk->sk_state != state)
continue;
@@ -161,19 +163,10 @@ static struct sock *__rfcomm_get_sock_by_channel(int state, u8 channel, bdaddr_t
sk1 = sk;
}
}
- return node ? sk : sk1;
-}
-/* Find socket with given address (channel, src).
- * Returns locked socket */
-static inline struct sock *rfcomm_get_sock_by_channel(int state, u8 channel, bdaddr_t *src)
-{
- struct sock *s;
- read_lock(&rfcomm_sk_list.lock);
- s = __rfcomm_get_sock_by_channel(state, channel, src);
- if (s) bh_lock_sock(s);
read_unlock(&rfcomm_sk_list.lock);
- return s;
+
+ return node ? sk : sk1;
}
static void rfcomm_sock_destruct(struct sock *sk)
@@ -939,6 +932,8 @@ int rfcomm_connect_ind(struct rfcomm_session *s, u8 channel, struct rfcomm_dlc *
if (!parent)
return 0;
+ bh_lock_sock(parent);
+
/* Check for backlog size */
if (sk_acceptq_is_full(parent)) {
BT_DBG("backlog full %d", parent->sk_ack_backlog);
--
1.7.3.1
^ permalink raw reply related
* [PATCH 4/7] Bluetooth: Get ride of __l2cap_get_sock_by_psm()
From: Gustavo F. Padovan @ 2010-11-02 15:03 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1288710198-6108-3-git-send-email-padovan@profusion.mobi>
l2cap_get_sock_by_psm() was the only user of this function, so I merged
both into l2cap_get_sock_by_psm(). The socket lock now should be hold
outside of l2cap_get_sock_by_psm() once we hold and release it inside the
same function now.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
net/bluetooth/l2cap.c | 22 +++++++++-------------
1 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 3d48867..27199bc 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -746,11 +746,13 @@ found:
/* Find socket with psm and source bdaddr.
* Returns closest match.
*/
-static struct sock *__l2cap_get_sock_by_psm(int state, __le16 psm, bdaddr_t *src)
+static struct sock *l2cap_get_sock_by_psm(int state, __le16 psm, bdaddr_t *src)
{
struct sock *sk = NULL, *sk1 = NULL;
struct hlist_node *node;
+ read_lock(&l2cap_sk_list.lock);
+
sk_for_each(sk, node, &l2cap_sk_list.head) {
if (state && sk->sk_state != state)
continue;
@@ -765,20 +767,10 @@ static struct sock *__l2cap_get_sock_by_psm(int state, __le16 psm, bdaddr_t *src
sk1 = sk;
}
}
- return node ? sk : sk1;
-}
-/* Find socket with given address (psm, src).
- * Returns locked socket */
-static inline struct sock *l2cap_get_sock_by_psm(int state, __le16 psm, bdaddr_t *src)
-{
- struct sock *s;
- read_lock(&l2cap_sk_list.lock);
- s = __l2cap_get_sock_by_psm(state, psm, src);
- if (s)
- bh_lock_sock(s);
read_unlock(&l2cap_sk_list.lock);
- return s;
+
+ return node ? sk : sk1;
}
static void l2cap_sock_destruct(struct sock *sk)
@@ -2921,6 +2913,8 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
goto sendresp;
}
+ bh_lock_sock(parent);
+
/* Check if the ACL is secure enough (if not SDP) */
if (psm != cpu_to_le16(0x0001) &&
!hci_conn_check_link_mode(conn->hcon)) {
@@ -4425,6 +4419,8 @@ static inline int l2cap_conless_channel(struct l2cap_conn *conn, __le16 psm, str
if (!sk)
goto drop;
+ bh_lock_sock(sk);
+
BT_DBG("sk %p, len %d", sk, skb->len);
if (sk->sk_state != BT_BOUND && sk->sk_state != BT_CONNECTED)
--
1.7.3.1
^ permalink raw reply related
* [PATCH 3/7] Bluetooth: Hold the lock inside rfcomm_get_sock_by_addr()
From: Gustavo F. Padovan @ 2010-11-02 15:03 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1288710198-6108-2-git-send-email-padovan@profusion.mobi>
It also have to change the name of the function to
rfcomm_get_sock_by_addr() because we do hold the lock inside it now.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
net/bluetooth/rfcomm/sock.c | 16 +++++-----------
1 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index aec505f..4ed9499 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -123,16 +123,18 @@ static void rfcomm_sk_state_change(struct rfcomm_dlc *d, int err)
}
/* ---- Socket functions ---- */
-static struct sock *__rfcomm_get_sock_by_addr(u8 channel, bdaddr_t *src)
+static struct sock *rfcomm_get_sock_by_addr(u8 channel, bdaddr_t *src)
{
struct sock *sk = NULL;
struct hlist_node *node;
+ write_lock_bh(&rfcomm_sk_list.lock);
sk_for_each(sk, node, &rfcomm_sk_list.head) {
if (rfcomm_pi(sk)->channel == channel &&
!bacmp(&bt_sk(sk)->src, src))
break;
}
+ write_unlock_bh(&rfcomm_sk_list.lock);
return node ? sk : NULL;
}
@@ -374,9 +376,7 @@ static int rfcomm_sock_bind(struct socket *sock, struct sockaddr *addr, int addr
goto done;
}
- write_lock_bh(&rfcomm_sk_list.lock);
-
- if (sa->rc_channel && __rfcomm_get_sock_by_addr(sa->rc_channel, &sa->rc_bdaddr)) {
+ if (sa->rc_channel && rfcomm_get_sock_by_addr(sa->rc_channel, &sa->rc_bdaddr)) {
err = -EADDRINUSE;
} else {
/* Save source address */
@@ -385,8 +385,6 @@ static int rfcomm_sock_bind(struct socket *sock, struct sockaddr *addr, int addr
sk->sk_state = BT_BOUND;
}
- write_unlock_bh(&rfcomm_sk_list.lock);
-
done:
release_sock(sk);
return err;
@@ -459,17 +457,13 @@ static int rfcomm_sock_listen(struct socket *sock, int backlog)
err = -EINVAL;
- write_lock_bh(&rfcomm_sk_list.lock);
-
for (channel = 1; channel < 31; channel++)
- if (!__rfcomm_get_sock_by_addr(channel, src)) {
+ if (!rfcomm_get_sock_by_addr(channel, src)) {
rfcomm_pi(sk)->channel = channel;
err = 0;
break;
}
- write_unlock_bh(&rfcomm_sk_list.lock);
-
if (err < 0)
goto done;
}
--
1.7.3.1
^ permalink raw reply related
* [PATCH 2/7] Bluetooth: Hold the lock inside sco_get_sock_by_addr()
From: Gustavo F. Padovan @ 2010-11-02 15:03 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1288710198-6108-1-git-send-email-padovan@profusion.mobi>
It also have to change the name of the function to
sco_get_sock_by_addr() because we do hold the lock inside it now.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
net/bluetooth/sco.c | 10 ++++------
1 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index d0927d1..85b5498 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -276,16 +276,18 @@ drop:
}
/* -------- Socket interface ---------- */
-static struct sock *__sco_get_sock_by_addr(bdaddr_t *ba)
+static struct sock *sco_get_sock_by_addr(bdaddr_t *ba)
{
struct sock *sk;
struct hlist_node *node;
+ write_lock_bh(&sco_sk_list.lock);
sk_for_each(sk, node, &sco_sk_list.head)
if (!bacmp(&bt_sk(sk)->src, ba))
goto found;
sk = NULL;
found:
+ write_unlock_bh(&sco_sk_list.lock);
return sk;
}
@@ -469,9 +471,7 @@ static int sco_sock_bind(struct socket *sock, struct sockaddr *addr, int addr_le
goto done;
}
- write_lock_bh(&sco_sk_list.lock);
-
- if (bacmp(src, BDADDR_ANY) && __sco_get_sock_by_addr(src)) {
+ if (bacmp(src, BDADDR_ANY) && sco_get_sock_by_addr(src)) {
err = -EADDRINUSE;
} else {
/* Save source address */
@@ -479,8 +479,6 @@ static int sco_sock_bind(struct socket *sock, struct sockaddr *addr, int addr_le
sk->sk_state = BT_BOUND;
}
- write_unlock_bh(&sco_sk_list.lock);
-
done:
release_sock(sk);
return err;
--
1.7.3.1
^ permalink raw reply related
* [PATCH 1/7] Bluetooth: Hold the lock inside l2cap_get_sock_by_addr()
From: Gustavo F. Padovan @ 2010-11-02 15:03 UTC (permalink / raw)
To: linux-bluetooth
It also have to change the name of the function to
l2cap_get_sock_by_addr() because we do hold the lock inside it now.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
net/bluetooth/l2cap.c | 17 ++++++-----------
1 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 6f931cc..3d48867 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -728,15 +728,18 @@ static inline void l2cap_chan_add(struct l2cap_conn *conn, struct sock *sk, stru
}
/* ---- Socket interface ---- */
-static struct sock *__l2cap_get_sock_by_addr(__le16 psm, bdaddr_t *src)
+static struct sock *l2cap_get_sock_by_addr(__le16 psm, bdaddr_t *src)
{
struct sock *sk;
struct hlist_node *node;
+
+ write_lock_bh(&l2cap_sk_list.lock);
sk_for_each(sk, node, &l2cap_sk_list.head)
if (l2cap_pi(sk)->sport == psm && !bacmp(&bt_sk(sk)->src, src))
goto found;
sk = NULL;
found:
+ write_unlock_bh(&l2cap_sk_list.lock);
return sk;
}
@@ -1024,9 +1027,7 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
}
}
- write_lock_bh(&l2cap_sk_list.lock);
-
- if (la.l2_psm && __l2cap_get_sock_by_addr(la.l2_psm, &la.l2_bdaddr)) {
+ if (la.l2_psm && l2cap_get_sock_by_addr(la.l2_psm, &la.l2_bdaddr)) {
err = -EADDRINUSE;
} else {
/* Save source address */
@@ -1040,8 +1041,6 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
l2cap_pi(sk)->sec_level = BT_SECURITY_SDP;
}
- write_unlock_bh(&l2cap_sk_list.lock);
-
done:
release_sock(sk);
return err;
@@ -1257,18 +1256,14 @@ static int l2cap_sock_listen(struct socket *sock, int backlog)
err = -EINVAL;
- write_lock_bh(&l2cap_sk_list.lock);
-
for (psm = 0x1001; psm < 0x1100; psm += 2)
- if (!__l2cap_get_sock_by_addr(cpu_to_le16(psm), src)) {
+ if (!l2cap_get_sock_by_addr(cpu_to_le16(psm), src)) {
l2cap_pi(sk)->psm = cpu_to_le16(psm);
l2cap_pi(sk)->sport = cpu_to_le16(psm);
err = 0;
break;
}
- write_unlock_bh(&l2cap_sk_list.lock);
-
if (err < 0)
goto done;
}
--
1.7.3.1
^ permalink raw reply related
* [PATCH] Cleanup set_mode code
From: Luiz Augusto von Dentz @ 2010-11-02 14:44 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
Now that MODE_LIMITED was removed there is no need to compare string mode
to determine if scan mode will change or not.
---
src/adapter.c | 21 +++++----------------
1 files changed, 5 insertions(+), 16 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 32c74d1..e12d9e5 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -603,22 +603,11 @@ done:
DBG("%s", modestr);
- if (msg != NULL) {
- /* Limited to Discoverable and vice-versa doesn't cause any
- change to scan mode */
- if (g_str_equal(modestr, mode2str(adapter->mode)) == TRUE) {
- DBusMessage *reply;
-
- reply = g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
-
- g_dbus_send_message(connection, reply);
- } else
- /* Wait for mode change to reply */
- adapter->pending_mode = create_session(adapter,
- connection,
- msg, new_mode,
- NULL);
- } else
+ if (msg != NULL)
+ /* Wait for mode change to reply */
+ adapter->pending_mode = create_session(adapter, connection,
+ msg, new_mode, NULL);
+ else
/* Nothing to reply just write the new mode */
adapter->mode = new_mode;
--
1.7.1
^ permalink raw reply related
* Re: [PATCH 2/2] Add support for generating pull response in many parts
From: Luiz Augusto von Dentz @ 2010-11-02 12:48 UTC (permalink / raw)
To: Radoslaw Jablonski; +Cc: linux-bluetooth
In-Reply-To: <AANLkTi=xHO1Ste4+6AEULbs91qeL6NCuYAK8exh63gqy@mail.gmail.com>
Hi,
On Tue, Nov 2, 2010 at 1:56 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Did you actually check if tracker/sparql doesn't support having a byte
> limit instead of contact/row? I know this sounds crazy, but Ive seem
> some other implementations of pbap that does similar things as to
> query a number of contacts and they can cause big pauses when
> generating the responses depending on the size of the MTU being used
> and in fact doesn't completely eliminate the extra buffering on the
> plugin side. Also I think we might need to use the read callback to
> continue the queries and not do it regardless of the speed the client
> can read, otherwise we may run in the same situation as we have now
> but instead of asking all the data at once we do it in parts but we
> still don't care if the client is fetching that data at the same pace
> we buffer.
Radek and I discussed this offline and came to a conclusion that using
a temporary files to buffer the data is probably a better idea, first
because it will be difficult to synchronize the speed of client and
backend which can either cause too much buffering (OOM) or slow
transfer speed, the second reason is that if we start supporting
avatar/image then each contact will probably consume a lot more memory
than it does right now and third caching can probably be done much
more efficiently using temporary files.
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: [PATCH v2] Add support for sending small data through obex
From: Luiz Augusto von Dentz @ 2010-11-02 12:05 UTC (permalink / raw)
To: Radoslaw Jablonski; +Cc: linux-bluetooth
In-Reply-To: <1288689911-18507-1-git-send-email-ext-jablonski.radoslaw@nokia.com>
Hi,
On Tue, Nov 2, 2010 at 11:25 AM, Radoslaw Jablonski
<ext-jablonski.radoslaw@nokia.com> wrote:
> Added handling packets smaller than mtu in obex_write_stream. Now trying
> to read from source until mtu will be filled properly and not sending
> immediately data if it is smaller than mtu.
> ---
> src/obex.c | 51 +++++++++++++++++++++++++++++++++++++++------------
> 1 files changed, 39 insertions(+), 12 deletions(-)
>
> diff --git a/src/obex.c b/src/obex.c
> index 6d4430d..4cbc1f8 100644
> --- a/src/obex.c
> +++ b/src/obex.c
> @@ -622,7 +622,7 @@ static int obex_write_stream(struct obex_session *os,
> {
> obex_headerdata_t hd;
> uint8_t *ptr;
> - ssize_t len;
> + ssize_t len, r_len;
> unsigned int flags;
> uint8_t hi;
>
> @@ -642,18 +642,39 @@ static int obex_write_stream(struct obex_session *os,
> goto add_header;
> }
>
> - len = os->driver->read(os->object, os->buf, os->tx_mtu, &hi);
> - if (len < 0) {
> - error("read(): %s (%zd)", strerror(-len), -len);
> - if (len == -EAGAIN)
> - return len;
> - else if (len == -ENOSTR)
> - return 0;
> + /* Copying data from source until we reach end of the stream. Sending
> + * data only if MTU will be filled in 100% or we reach end of data.
> + * Remaining data in buffer will be sent with next amount of data
> + * from source.*/
> + do {
> + r_len = os->driver->read(os->object, os->buf + os->pending,
> + os->tx_mtu - os->pending, &hi);
>
> - g_free(os->buf);
> - os->buf = NULL;
> - return len;
> - }
> + if (r_len == 0)
> + break;
> + else if (r_len < 0) {
> + error("read(): %s (%zd)", strerror(-r_len), -r_len);
> +
> + switch(r_len) {
> + case -EAGAIN:
> + return r_len;
> + case -EINTR:
> + continue;
> + case -ENOSTR:
> + return 0;
> + default:
> + g_free(os->buf);
> + os->buf = NULL;
> + return r_len;
> + }
> + }
> +
> + /* Saving amount of data accumulated in obex buffer */
> + os->pending += r_len;
> + } while (os->pending < os->tx_mtu);
> +
> + len = os->pending;
> + os->pending = 0;
>
> ptr = os->buf;
>
> @@ -702,6 +723,12 @@ static gboolean handle_async_io(void *object, int flags, int err,
> ret = obex_read_stream(os, os->obex, os->obj);
>
> proceed:
> +
> + /* Returning TRUE to not delete current watcher - it need to be active
> + * to handle next io flag changes (more data will be available later)*/
> + if (ret == -EAGAIN)
> + return TRUE;
> +
> if (ret < 0) {
> os_set_response(os->obj, err);
> OBEX_CancelRequest(os->obex, TRUE);
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Ack
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* [PATCH v2] Add support for sending small data through obex
From: Radoslaw Jablonski @ 2010-11-02 9:25 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Radoslaw Jablonski
Added handling packets smaller than mtu in obex_write_stream. Now trying
to read from source until mtu will be filled properly and not sending
immediately data if it is smaller than mtu.
---
src/obex.c | 51 +++++++++++++++++++++++++++++++++++++++------------
1 files changed, 39 insertions(+), 12 deletions(-)
diff --git a/src/obex.c b/src/obex.c
index 6d4430d..4cbc1f8 100644
--- a/src/obex.c
+++ b/src/obex.c
@@ -622,7 +622,7 @@ static int obex_write_stream(struct obex_session *os,
{
obex_headerdata_t hd;
uint8_t *ptr;
- ssize_t len;
+ ssize_t len, r_len;
unsigned int flags;
uint8_t hi;
@@ -642,18 +642,39 @@ static int obex_write_stream(struct obex_session *os,
goto add_header;
}
- len = os->driver->read(os->object, os->buf, os->tx_mtu, &hi);
- if (len < 0) {
- error("read(): %s (%zd)", strerror(-len), -len);
- if (len == -EAGAIN)
- return len;
- else if (len == -ENOSTR)
- return 0;
+ /* Copying data from source until we reach end of the stream. Sending
+ * data only if MTU will be filled in 100% or we reach end of data.
+ * Remaining data in buffer will be sent with next amount of data
+ * from source.*/
+ do {
+ r_len = os->driver->read(os->object, os->buf + os->pending,
+ os->tx_mtu - os->pending, &hi);
- g_free(os->buf);
- os->buf = NULL;
- return len;
- }
+ if (r_len == 0)
+ break;
+ else if (r_len < 0) {
+ error("read(): %s (%zd)", strerror(-r_len), -r_len);
+
+ switch(r_len) {
+ case -EAGAIN:
+ return r_len;
+ case -EINTR:
+ continue;
+ case -ENOSTR:
+ return 0;
+ default:
+ g_free(os->buf);
+ os->buf = NULL;
+ return r_len;
+ }
+ }
+
+ /* Saving amount of data accumulated in obex buffer */
+ os->pending += r_len;
+ } while (os->pending < os->tx_mtu);
+
+ len = os->pending;
+ os->pending = 0;
ptr = os->buf;
@@ -702,6 +723,12 @@ static gboolean handle_async_io(void *object, int flags, int err,
ret = obex_read_stream(os, os->obex, os->obj);
proceed:
+
+ /* Returning TRUE to not delete current watcher - it need to be active
+ * to handle next io flag changes (more data will be available later)*/
+ if (ret == -EAGAIN)
+ return TRUE;
+
if (ret < 0) {
os_set_response(os->obj, err);
OBEX_CancelRequest(os->obex, TRUE);
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] [RFC] Add Application property to HealthChannel
From: Gustavo F. Padovan @ 2010-11-02 3:35 UTC (permalink / raw)
To: Elvis Pfützenreuter; +Cc: linux-bluetooth
In-Reply-To: <1288635382-2032-1-git-send-email-epx@signove.com>
* Elvis Pfützenreuter <epx@signove.com> [2010-11-01 16:16:22 -0200]:
> This patch adds the Application property to HealthChannel, which
> allows to unambiguously relate a channel to an application.
>
> This property is useful when there are several processes interested
> in accepting HealthChannels but device address is not sufficient
> criteria to engage upon, or ignore, the ChannelConnected signal.
> Having the application path allows to determine role and data type,
> in the context of the process that has created that application.
>
> Also, channel path skeleton is fixed in documentation, in order to
> reflect atual implementation.
So a separeted patch here.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: [PATCH 2/2] Add support for generating pull response in many parts
From: Luiz Augusto von Dentz @ 2010-11-01 23:56 UTC (permalink / raw)
To: Radoslaw Jablonski; +Cc: linux-bluetooth
In-Reply-To: <1288608899-23032-2-git-send-email-ext-jablonski.radoslaw@nokia.com>
Hi,
On Mon, Nov 1, 2010 at 12:54 PM, Radoslaw Jablonski
<ext-jablonski.radoslaw@nokia.com> wrote:
> Now data from tracker is fetched in many smaller parts (instead of one
> big query before). This is needed to save memory and to not overload
> dbus and tracker when generating query result.
> ---
> plugins/phonebook-tracker.c | 71 ++++++++++++++++++++++++++++++++++++-------
> 1 files changed, 60 insertions(+), 11 deletions(-)
>
> diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
> index 58f52ab..46ef5fb 100644
> --- a/plugins/phonebook-tracker.c
> +++ b/plugins/phonebook-tracker.c
> @@ -57,6 +57,8 @@
> #define COL_ANSWERED 37
> #define ADDR_FIELD_AMOUNT 7
> #define CONTACT_ID_PREFIX "contact:"
> +#define QUERY_LIMIT_FORMAT "%s LIMIT %d OFFSET %d"
> +#define QUERY_LIMIT 50
>
> #define CONTACTS_QUERY_ALL \
> "SELECT ?v nco:fullname(?c) " \
> @@ -650,6 +652,9 @@ struct phonebook_data {
> gboolean vcardentry;
> const struct apparam_field *params;
> GSList *contacts;
> + char *name;
> + int offset;
> + int num_row;
> };
>
> struct cache_data {
> @@ -1098,6 +1103,12 @@ static void add_affiliation(char **field, const char *value)
> *field = g_strdup(value);
> }
>
> +static char *gen_partial_query(const char *name, int limit, int offset)
> +{
> + return g_strdup_printf(QUERY_LIMIT_FORMAT, name2query(name),
> + limit, offset);
> +}
> +
> static void pull_contacts(char **reply, int num_fields, void *user_data)
> {
> struct phonebook_data *data = user_data;
> @@ -1107,13 +1118,17 @@ static void pull_contacts(char **reply, int num_fields, void *user_data)
> GString *vcards;
> int last_index, i;
> gboolean cdata_present = FALSE;
> - char *home_addr, *work_addr;
> + gboolean last_resp = FALSE;
> + char *home_addr, *work_addr, *query;
>
> if (num_fields < 0) {
> data->cb(NULL, 0, num_fields, 0, data->user_data);
> goto fail;
> }
>
> + data->num_row++;
> + last_index = params->liststartoffset + params->maxlistcount;
> +
> DBG("reply %p", reply);
>
> if (reply == NULL)
> @@ -1145,8 +1160,6 @@ static void pull_contacts(char **reply, int num_fields, void *user_data)
>
> data->index++;
>
> - last_index = params->liststartoffset + params->maxlistcount;
> -
> if ((data->index <= params->liststartoffset ||
> data->index > last_index) &&
> params->maxlistcount > 0)
> @@ -1222,14 +1235,46 @@ add_numbers:
> done:
> vcards = gen_vcards(data->contacts, params);
>
> - if (num_fields == 0)
> + /* If tracker returned only empty row - all results already returned */
> + if (num_fields == 0 && data->num_row == 1)
> + last_resp = TRUE;
> +
> + /* Check if tracker could return desired number of results - if coldn't,
> + * all results are fetched already and this is last response */
> + if (data->num_row < QUERY_LIMIT)
> + last_resp = TRUE;
> +
> + /* Check needed for 'maxlistcount' and 'liststartoffset' parameters */
> + if (data->index > last_index)
> + last_resp = TRUE;
> +
> + /* Data won't be send if starting offset has not been achieved (unless
> + * now handling last response from tracker) */
> + if (data->index > params->liststartoffset || last_resp)
> + /* 4th parameter of callback is used to mark if stream should be
> + * closed or more data will be sent*/
> data->cb(vcards->str, vcards->len,
> - g_slist_length(data->contacts), 0,
> - data->user_data);
> + g_slist_length(data->contacts), !last_resp,
> + data->user_data);
>
> + g_slist_free(data->contacts);data->contacts = NULL;
> g_string_free(vcards, TRUE);
> + data->num_row = 0;
> +
> + /* Sending query to tracker to get next part of results (only for pull
> + * phonebook queries) */
> + if (!data->vcardentry && !last_resp) {
> + data->offset += QUERY_LIMIT;
> + query = gen_partial_query(data->name, QUERY_LIMIT,
> + data->offset);
> + query_tracker(query, PULL_QUERY_COL_AMOUNT,
> + pull_contacts, data);
> + g_free(query);
> + return;
> + }
> fail:
> g_slist_free(data->contacts);
> + g_free(data->name);
> g_free(data);
> }
>
> @@ -1367,18 +1412,18 @@ int phonebook_pull(const char *name, const struct apparam_field *params,
> phonebook_cb cb, void *user_data)
> {
> struct phonebook_data *data;
> - const char *query;
> + char *query;
> reply_list_foreach_t pull_cb;
> - int col_amount;
> + int col_amount, ret;
>
> DBG("name %s", name);
>
> if (params->maxlistcount == 0) {
> - query = name2count_query(name);
> + query = g_strdup(name2count_query(name));
> col_amount = COUNT_QUERY_COL_AMOUNT;
> pull_cb = pull_contacts_size;
> } else {
> - query = name2query(name);
> + query = gen_partial_query(name, QUERY_LIMIT, 0);
> col_amount = PULL_QUERY_COL_AMOUNT;
> pull_cb = pull_contacts;
> }
> @@ -1390,8 +1435,12 @@ int phonebook_pull(const char *name, const struct apparam_field *params,
> data->params = params;
> data->user_data = user_data;
> data->cb = cb;
> + data->name = g_strdup(name);
> +
> + ret = query_tracker(query, col_amount, pull_cb, data);
> + g_free(query);
>
> - return query_tracker(query, col_amount, pull_cb, data);
> + return ret;
> }
Did you actually check if tracker/sparql doesn't support having a byte
limit instead of contact/row? I know this sounds crazy, but Ive seem
some other implementations of pbap that does similar things as to
query a number of contacts and they can cause big pauses when
generating the responses depending on the size of the MTU being used
and in fact doesn't completely eliminate the extra buffering on the
plugin side. Also I think we might need to use the read callback to
continue the queries and not do it regardless of the speed the client
can read, otherwise we may run in the same situation as we have now
but instead of asking all the data at once we do it in parts but we
still don't care if the client is fetching that data at the same pace
we buffer.
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: [PATCH 1/2] Add support for sending PBAP response in many parts
From: Luiz Augusto von Dentz @ 2010-11-01 23:10 UTC (permalink / raw)
To: Radoslaw Jablonski; +Cc: linux-bluetooth
In-Reply-To: <1288608899-23032-1-git-send-email-ext-jablonski.radoslaw@nokia.com>
Hi,
On Mon, Nov 1, 2010 at 12:54 PM, Radoslaw Jablonski
<ext-jablonski.radoslaw@nokia.com> wrote:
> Previously pull result from PBAP was returned always in one big
> part - a lot of memory was used to store all data. Now it is
> possible to send data from pbap in many smaller chunks.
> ---
> plugins/pbap.c | 29 ++++++++++++++++++++++++++++-
> 1 files changed, 28 insertions(+), 1 deletions(-)
>
> diff --git a/plugins/pbap.c b/plugins/pbap.c
> index 3ea7d6b..b4c1f00 100644
> --- a/plugins/pbap.c
> +++ b/plugins/pbap.c
> @@ -143,6 +143,7 @@ struct pbap_session {
> uint32_t find_handle;
> GString *buffer;
> struct cache cache;
> + gboolean partial_resp;
> };
>
> static const uint8_t PBAP_TARGET[TARGET_SIZE] = {
> @@ -262,6 +263,10 @@ static void query_result(const char *buffer, size_t bufsize, int vcards,
> pbap->buffer = g_string_append_len(pbap->buffer, buffer,
> bufsize);
>
> + /* If partial_resp will be set to TRUE, then we won't end transmission
> + * after sending one part of results to the client via obex*/
> + pbap->partial_resp = missed ? TRUE : FALSE;
> +
> obex_object_set_io_flags(pbap, G_IO_IN, 0);
> }
>
> @@ -829,6 +834,28 @@ fail:
> return NULL;
> }
>
> +static ssize_t string_read_part(void *object, void *buf, size_t count,
> + gboolean partial)
> +{
> + GString *string = object;
> + ssize_t len;
> +
> + if (string->len == 0) {
> + /* if more data will be available later, returning -EAGAIN
> + * instead of 0 (it will suspend request) */
> + if (partial)
> + return -EAGAIN;
> + else
> + return 0;
> + }
> +
> + len = MIN(string->len, count);
> + memcpy(buf, string->str, len);
> + string = g_string_erase(string, 0, len);
> +
> + return len;
> +}
> +
> static ssize_t vobject_pull_read(void *object, void *buf, size_t count,
> uint8_t *hi)
> {
> @@ -847,7 +874,7 @@ static ssize_t vobject_pull_read(void *object, void *buf, size_t count,
> /* Stream data */
> *hi = OBEX_HDR_BODY;
>
> - return string_read(pbap->buffer, buf, count);
> + return string_read_part(pbap->buffer, buf, count, pbap->partial_resp);
> }
Im not sure why you decided to create another string_read copy to
handle this, why not using string_read return and check if it is a
partial response on directly on vobject_pull_read? Sounds simple to me
and don't duplicate any code.
> static ssize_t vobject_list_read(void *object, void *buf, size_t count,
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: [PATCH] Add support for sending small data through obex
From: Luiz Augusto von Dentz @ 2010-11-01 23:01 UTC (permalink / raw)
To: Radoslaw Jablonski; +Cc: linux-bluetooth
In-Reply-To: <1288603499-16456-1-git-send-email-ext-jablonski.radoslaw@nokia.com>
Hi,
On Mon, Nov 1, 2010 at 11:24 AM, Radoslaw Jablonski
<ext-jablonski.radoslaw@nokia.com> wrote:
> Added handling packets smaller than mtu in obex_write_stream. Now trying
> to read from source until mtu will be filled properly and not sending
> immediately data if it is smaller than mtu.
> ---
> src/obex.c | 48 +++++++++++++++++++++++++++++++++++++-----------
> 1 files changed, 37 insertions(+), 11 deletions(-)
>
> diff --git a/src/obex.c b/src/obex.c
> index 6d4430d..d3a6ccd 100644
> --- a/src/obex.c
> +++ b/src/obex.c
> @@ -623,6 +623,7 @@ static int obex_write_stream(struct obex_session *os,
> obex_headerdata_t hd;
> uint8_t *ptr;
> ssize_t len;
> + ssize_t r_len;
You can have len and r_len in the same line above.
> unsigned int flags;
> uint8_t hi;
>
> @@ -642,18 +643,37 @@ static int obex_write_stream(struct obex_session *os,
> goto add_header;
> }
>
> - len = os->driver->read(os->object, os->buf, os->tx_mtu, &hi);
> - if (len < 0) {
> - error("read(): %s (%zd)", strerror(-len), -len);
> - if (len == -EAGAIN)
> - return len;
> - else if (len == -ENOSTR)
> - return 0;
> + /* Copying data from source until we reach end of the stream. Sending
> + * data only if MTU will be filled in 100% or we reach end of data.
> + * Remaining data in buffer will be sent with next amount of data
> + * from source.*/
> + do {
> + r_len = os->driver->read(os->object, os->buf + os->pending,
> + os->tx_mtu - os->pending, &hi);
It looks like you should add one more tab in the line above it that
doesn't go over 80 columns.
> + if (r_len < 0) {
> + error("read(): %s (%zd)", strerror(-r_len), -r_len);
> +
> + switch(r_len) {
> + case -EAGAIN:
> + return r_len;
> + case -EINTR:
> + continue;
> + case -ENOSTR:
> + return 0;
> + default:
> + g_free(os->buf);
> + os->buf = NULL;
> + return r_len;
> + }
> + }
I think it is probably more efficiently to handle rlen == 0 here, like
if (rlen == 0) break; else if (rlen < 0)
> - g_free(os->buf);
> - os->buf = NULL;
> - return len;
> - }
> + /* Saving amount of data accumulated in obex buffer */
> + os->pending += r_len;
> + } while (os->pending < os->tx_mtu && r_len != 0);
That simplify here too, just do while (os->pending < os->txt_mtu) if
you handle rlen == 0 like I said before.
> +
> + len = os->pending;
> + os->pending = 0;
>
> ptr = os->buf;
>
> @@ -702,6 +722,12 @@ static gboolean handle_async_io(void *object, int flags, int err,
> ret = obex_read_stream(os, os->obex, os->obj);
>
> proceed:
> +
> + /* Returning TRUE to not delete current watcher - it need to be active
> + * to handle next io flag changes (more data will be available later)*/
> + if (ret == -EAGAIN)
> + return TRUE;
> +
> if (ret < 0) {
> os_set_response(os->obj, err);
> OBEX_CancelRequest(os->obex, TRUE);
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: [PATCH v4 0/2] Get and Set Feature Reports on HIDRAW (USB and Bluetooth)
From: Jiri Kosina @ 2010-11-01 19:23 UTC (permalink / raw)
To: Alan Ott, Marcel Holtmann, David S. Miller
Cc: Stefan Achatz, Antonio Ospite, Alexey Dobriyan, Tejun Heo,
Alan Stern, Greg Kroah-Hartman, Stephane Chatty, Michael Poole,
Bastien Nocera, Eric Dumazet, linux-input, linux-kernel,
linux-usb, linux-bluetooth, netdev
In-Reply-To: <alpine.LNX.2.00.1009221408330.26813@pobox.suse.cz>
On Wed, 22 Sep 2010, Jiri Kosina wrote:
> > > > This is version 4. Built against 2.6.35+ revision 320b2b8de12698 .
> > > >
> > > > Alan Ott (2):
> > > > HID: Add Support for Setting and Getting Feature Reports from hidraw
> > > > Bluetooth: hidp: Add support for hidraw HIDIOCGFEATURE and
> > > > HIDIOCSFEATURE
> > > >
> > > > drivers/hid/hidraw.c | 105 ++++++++++++++++++++++++++++++++++++--
> > > > drivers/hid/usbhid/hid-core.c | 37 +++++++++++++-
> > > > include/linux/hid.h | 3 +
> > > > include/linux/hidraw.h | 3 +
> > > > net/bluetooth/hidp/core.c | 114 +++++++++++++++++++++++++++++++++++++++--
> > > > net/bluetooth/hidp/hidp.h | 8 +++
> > > > 6 files changed, 260 insertions(+), 10 deletions(-)
> > >
> > > Marcel, as per our previous discussion -- what is your word on this? I'd
> > > be glad taking it once you Ack the bluetooth bits (which, as far as I
> > > understood from your last mail, don't have strong objections against any
> > > more).
> >
> > ... Marcel?
> >
> > I'd really like not to miss 2.6.37 merge window with this.
>
> Seemingly I have not enought powers to get statement from Marcel here
> these days/weeks.
>
> Davem, would you perhaps be able to step in here?
Marcel, any word on this patchset by chance?
Thanks,
--
Jiri Kosina
SUSE Labs, Novell Inc.
^ permalink raw reply
* [PATCH] [RFC] Add Application property to HealthChannel
From: Elvis Pfützenreuter @ 2010-11-01 18:16 UTC (permalink / raw)
To: linux-bluetooth; +Cc: epx
This patch adds the Application property to HealthChannel, which
allows to unambiguously relate a channel to an application.
This property is useful when there are several processes interested
in accepting HealthChannels but device address is not sufficient
criteria to engage upon, or ignore, the ChannelConnected signal.
Having the application path allows to determine role and data type,
in the context of the process that has created that application.
Also, channel path skeleton is fixed in documentation, in order to
reflect atual implementation.
---
doc/health-api.txt | 8 ++++++--
health/hdp.c | 3 +++
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/doc/health-api.txt b/doc/health-api.txt
index 89d5ff9..a0a1685 100644
--- a/doc/health-api.txt
+++ b/doc/health-api.txt
@@ -119,8 +119,7 @@ Properties:
Service org.bluez
Interface org.bluez.HealthChannel
-Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/
- hdp_YYYY/channel_ZZ
+Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/chanZZZ
Only the process that created the data channel or the creator of the
HealthApplication that received it will be able to call this methods.
@@ -160,3 +159,8 @@ Properties:
Identifies the Remote Device that is connected with. Maps with
a HealthDevice object.
+
+ object Application [readonly]
+
+ Identifies the HealthApplication to which this channel is
+ related to (which indirectly defines its role and data type).
diff --git a/health/hdp.c b/health/hdp.c
index be30204..1eba8e1 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -423,6 +423,9 @@ static DBusMessage *channel_get_properties(DBusConnection *conn,
path = device_get_path(chan->dev->dev);
dict_append_entry(&dict, "Device", DBUS_TYPE_OBJECT_PATH, &path);
+ path = chan->app->path;
+ dict_append_entry(&dict, "Application", DBUS_TYPE_OBJECT_PATH, &path);
+
if (chan->config == HDP_RELIABLE_DC)
type = g_strdup("Reliable");
else
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] Fix avoid starting AVDTP disconnect timer twice
From: Johan Hedberg @ 2010-11-01 14:45 UTC (permalink / raw)
To: Daniel Örstadius; +Cc: linux-bluetooth
In-Reply-To: <AANLkTikPhBzAhQ=CeDkQokMK39k=c-VxU3re5GtMdiFZ@mail.gmail.com>
Hi Daniel,
On Mon, Nov 01, 2010, Daniel Örstadius wrote:
> Remove starting the timer when setting the AVDTP state to idle. If
> needed, the timer should probably already have been started in
> avdtp_unref when the reference count goes to one.
>
> Since reference counting is handled in avdtp_ref and avdtp_unref, it
> seems reasonable that not to inspect the count outside of those
> functions.
>
> The issue was found when using Device.Disconnect to disconnect a
> headset. It was revealed by commit
> c72ce0f12a8387a70a6f0109f13bd6f414f32be8.
>
> Before the commit, the timer was removed and then started again.
> After applying it, the idle callback (disconnect_timeout) is called
> twice, causing a crash.
Thanks for investigating and fixing this. avdtp_unref indeed does
already take care of the timer so the call in set_state seems redundant
(in addition to being in a questionable place to begin with). The patch
has been pushed upstream.
Johan
^ permalink raw reply
* Re: [PATCHv3 2/2] Bluetooth: timer check sk is not owned before freeing
From: Andrei Emeltchenko @ 2010-11-01 14:20 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: linux-bluetooth
In-Reply-To: <20101029211718.GC14961@vigoh>
Hi Gustavo
On Sat, Oct 30, 2010 at 12:17 AM, Gustavo F. Padovan
<padovan@profusion.mobi> wrote:
> Hi Andrei,
>
> * Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2010-10-29 16:43:01 +0300]:
>
>> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
>>
>> In timer context we might delete l2cap channel used by krfcommd.
>> The check makes sure that sk is not owned. If sk is owned we
>> restart timer for HZ/5.
>>
>> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
>> ---
>> net/bluetooth/l2cap.c | 32 ++++++++++++++++++++------------
>> 1 files changed, 20 insertions(+), 12 deletions(-)
>>
>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
>> index b1344d8..c67b3c6 100644
>> --- a/net/bluetooth/l2cap.c
>> +++ b/net/bluetooth/l2cap.c
>> @@ -83,6 +83,18 @@ static struct sk_buff *l2cap_build_cmd(struct l2cap_conn *conn,
>> static int l2cap_ertm_data_rcv(struct sock *sk, struct sk_buff *skb);
>>
>> /* ---- L2CAP timers ---- */
>> +static void l2cap_sock_set_timer(struct sock *sk, long timeout)
>> +{
>> + BT_DBG("sk %p state %d timeout %ld", sk, sk->sk_state, timeout);
>> + sk_reset_timer(sk, &sk->sk_timer, jiffies + timeout);
>> +}
>> +
>> +static void l2cap_sock_clear_timer(struct sock *sk)
>> +{
>> + BT_DBG("sock %p state %d", sk, sk->sk_state);
>> + sk_stop_timer(sk, &sk->sk_timer);
>> +}
>> +
>> static void l2cap_sock_timeout(unsigned long arg)
>> {
>> struct sock *sk = (struct sock *) arg;
>> @@ -92,6 +104,14 @@ static void l2cap_sock_timeout(unsigned long arg)
>>
>> bh_lock_sock(sk);
>>
>> + if (sock_owned_by_user(sk)) {
>> + /* sk is owned by user. Try again later */
>> + l2cap_sock_set_timer(sk, HZ / 5);
>> + bh_unlock_sock(sk);
>> + sock_put(sk);
>
> You can't do a sock_put() here, you have to keep the referencee to the
> socket while the timer is enabled.
sk_reset_timer is holding sock when timer restarts. The same way done
in TCP code in function:
static void tcp_delack_timer(unsigned long data)
Regards,
Andrei
^ permalink raw reply
* [PATCH] Fix avoid starting AVDTP disconnect timer twice
From: Daniel Örstadius @ 2010-11-01 13:36 UTC (permalink / raw)
To: linux-bluetooth
[-- Attachment #1: Type: text/plain, Size: 36 bytes --]
Patch proposal for review.
/Daniel
[-- Attachment #2: 0001-Fix-avoid-starting-AVDTP-disconnect-timer-twice.patch --]
[-- Type: text/x-patch, Size: 1371 bytes --]
From 0aad44e90585e6a7c9a59cee2cdeff00316a527c Mon Sep 17 00:00:00 2001
From: Daniel Orstadius <daniel.orstadius@nokia.com>
Date: Mon, 1 Nov 2010 12:44:25 +0200
Subject: [PATCH] Fix avoid starting AVDTP disconnect timer twice
Remove starting the timer when setting the AVDTP state to idle. If
needed, the timer should probably already have been started in
avdtp_unref when the reference count goes to one.
Since reference counting is handled in avdtp_ref and avdtp_unref, it
seems reasonable that not to inspect the count outside of those
functions.
The issue was found when using Device.Disconnect to disconnect a
headset. It was revealed by commit
c72ce0f12a8387a70a6f0109f13bd6f414f32be8.
Before the commit, the timer was removed and then started again.
After applying it, the idle callback (disconnect_timeout) is called
twice, causing a crash.
---
audio/avdtp.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/audio/avdtp.c b/audio/avdtp.c
index 5e84a45..611fd7b 100644
--- a/audio/avdtp.c
+++ b/audio/avdtp.c
@@ -1034,8 +1034,6 @@ static void avdtp_sep_set_state(struct avdtp *session,
/* Remove pending commands for this stream from the queue */
cleanup_queue(session, stream);
stream_free(stream);
- if (session->ref == 1 && !session->streams)
- set_disconnect_timer(session);
break;
default:
break;
--
1.6.0.4
^ permalink raw reply related
* [PATCH 2/2] Add support for generating pull response in many parts
From: Radoslaw Jablonski @ 2010-11-01 10:54 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Radoslaw Jablonski
In-Reply-To: <1288608899-23032-1-git-send-email-ext-jablonski.radoslaw@nokia.com>
Now data from tracker is fetched in many smaller parts (instead of one
big query before). This is needed to save memory and to not overload
dbus and tracker when generating query result.
---
plugins/phonebook-tracker.c | 71 ++++++++++++++++++++++++++++++++++++-------
1 files changed, 60 insertions(+), 11 deletions(-)
diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index 58f52ab..46ef5fb 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -57,6 +57,8 @@
#define COL_ANSWERED 37
#define ADDR_FIELD_AMOUNT 7
#define CONTACT_ID_PREFIX "contact:"
+#define QUERY_LIMIT_FORMAT "%s LIMIT %d OFFSET %d"
+#define QUERY_LIMIT 50
#define CONTACTS_QUERY_ALL \
"SELECT ?v nco:fullname(?c) " \
@@ -650,6 +652,9 @@ struct phonebook_data {
gboolean vcardentry;
const struct apparam_field *params;
GSList *contacts;
+ char *name;
+ int offset;
+ int num_row;
};
struct cache_data {
@@ -1098,6 +1103,12 @@ static void add_affiliation(char **field, const char *value)
*field = g_strdup(value);
}
+static char *gen_partial_query(const char *name, int limit, int offset)
+{
+ return g_strdup_printf(QUERY_LIMIT_FORMAT, name2query(name),
+ limit, offset);
+}
+
static void pull_contacts(char **reply, int num_fields, void *user_data)
{
struct phonebook_data *data = user_data;
@@ -1107,13 +1118,17 @@ static void pull_contacts(char **reply, int num_fields, void *user_data)
GString *vcards;
int last_index, i;
gboolean cdata_present = FALSE;
- char *home_addr, *work_addr;
+ gboolean last_resp = FALSE;
+ char *home_addr, *work_addr, *query;
if (num_fields < 0) {
data->cb(NULL, 0, num_fields, 0, data->user_data);
goto fail;
}
+ data->num_row++;
+ last_index = params->liststartoffset + params->maxlistcount;
+
DBG("reply %p", reply);
if (reply == NULL)
@@ -1145,8 +1160,6 @@ static void pull_contacts(char **reply, int num_fields, void *user_data)
data->index++;
- last_index = params->liststartoffset + params->maxlistcount;
-
if ((data->index <= params->liststartoffset ||
data->index > last_index) &&
params->maxlistcount > 0)
@@ -1222,14 +1235,46 @@ add_numbers:
done:
vcards = gen_vcards(data->contacts, params);
- if (num_fields == 0)
+ /* If tracker returned only empty row - all results already returned */
+ if (num_fields == 0 && data->num_row == 1)
+ last_resp = TRUE;
+
+ /* Check if tracker could return desired number of results - if coldn't,
+ * all results are fetched already and this is last response */
+ if (data->num_row < QUERY_LIMIT)
+ last_resp = TRUE;
+
+ /* Check needed for 'maxlistcount' and 'liststartoffset' parameters */
+ if (data->index > last_index)
+ last_resp = TRUE;
+
+ /* Data won't be send if starting offset has not been achieved (unless
+ * now handling last response from tracker) */
+ if (data->index > params->liststartoffset || last_resp)
+ /* 4th parameter of callback is used to mark if stream should be
+ * closed or more data will be sent*/
data->cb(vcards->str, vcards->len,
- g_slist_length(data->contacts), 0,
- data->user_data);
+ g_slist_length(data->contacts), !last_resp,
+ data->user_data);
+ g_slist_free(data->contacts);data->contacts = NULL;
g_string_free(vcards, TRUE);
+ data->num_row = 0;
+
+ /* Sending query to tracker to get next part of results (only for pull
+ * phonebook queries) */
+ if (!data->vcardentry && !last_resp) {
+ data->offset += QUERY_LIMIT;
+ query = gen_partial_query(data->name, QUERY_LIMIT,
+ data->offset);
+ query_tracker(query, PULL_QUERY_COL_AMOUNT,
+ pull_contacts, data);
+ g_free(query);
+ return;
+ }
fail:
g_slist_free(data->contacts);
+ g_free(data->name);
g_free(data);
}
@@ -1367,18 +1412,18 @@ int phonebook_pull(const char *name, const struct apparam_field *params,
phonebook_cb cb, void *user_data)
{
struct phonebook_data *data;
- const char *query;
+ char *query;
reply_list_foreach_t pull_cb;
- int col_amount;
+ int col_amount, ret;
DBG("name %s", name);
if (params->maxlistcount == 0) {
- query = name2count_query(name);
+ query = g_strdup(name2count_query(name));
col_amount = COUNT_QUERY_COL_AMOUNT;
pull_cb = pull_contacts_size;
} else {
- query = name2query(name);
+ query = gen_partial_query(name, QUERY_LIMIT, 0);
col_amount = PULL_QUERY_COL_AMOUNT;
pull_cb = pull_contacts;
}
@@ -1390,8 +1435,12 @@ int phonebook_pull(const char *name, const struct apparam_field *params,
data->params = params;
data->user_data = user_data;
data->cb = cb;
+ data->name = g_strdup(name);
+
+ ret = query_tracker(query, col_amount, pull_cb, data);
+ g_free(query);
- return query_tracker(query, col_amount, pull_cb, data);
+ return ret;
}
int phonebook_get_entry(const char *folder, const char *id,
--
1.7.0.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox