Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH] Extend discover characteristic by UUID in gatttool to fetch all values
From: Johan Hedberg @ 2010-11-25 12:21 UTC (permalink / raw)
  To: Sheldon Demario; +Cc: linux-bluetooth
In-Reply-To: <1290515839-11867-1-git-send-email-sheldon.demario@openbossa.org>

Hi Sheldon,

On Tue, Nov 23, 2010, Sheldon Demario wrote:
> If the number of characteristics returned exceeds the MTU size, it will
> be needed to ask for more data until the handle range is entirely
> searched.
> ---
>  attrib/gatttool.c |   24 +++++++++++++++++++++++-
>  1 files changed, 23 insertions(+), 1 deletions(-)

Thanks.

> +	if ((status == ATT_ECODE_ATTR_NOT_FOUND) &&
> +			(char_data->start != opt_start))
> +		goto done;

No need for the extra () here. However, since it was the only issue I
found I fixed it myself and pushed the patch upstream.

Johan

^ permalink raw reply

* [PATCH] Fix crash after simultaneous authentication requests
From: Rafal Michalski @ 2010-11-25 10:06 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Rafal Michalski

Previously simultaneous authentication requests to the same device caused
bluetoothd crash. Now if ongoing authentication occurs error is returned,
preventing from simultaneous requests to the same device.
---
 src/device.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/src/device.c b/src/device.c
index 7c421e3..4427cd4 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2178,6 +2178,11 @@ int device_request_authentication(struct btd_device *device, auth_type_t type,
 	struct agent *agent;
 	int err;
 
+	if (device->authr) {
+		error("%s: authentication already requested", device->path);
+		return -EALREADY;
+	}
+
 	DBG("%s: requesting agent authentication", device->path);
 
 	agent = device_get_agent(device);
-- 
1.6.3.3


^ permalink raw reply related

* [PATCH 1/1] bluetooth: Fix NULL pointer dereference issue
From: Yuri Ershov @ 2010-11-25  9:55 UTC (permalink / raw)
  To: marcel, davem, padovan, jprvita
  Cc: linux-bluetooth, ville.tervo, andrei.emeltchenko, Yuri Ershov

This patch is an addition to my previous patch for this issue.
The problem is in resynchronization between two loops:
1. Main controlling loop (l2cap_connect_req, l2cap_config_req,
l2cap_config_rsp, l2cap_disconnect_req, etc.)
2. Loop waiting of BT_CONNECTED state of socket (l2cap_sock_accept,
bt_accept_dequeue, etc.).
In case of fast sequence of connect/disconnect operations the loop #1
makes several cycles, while the loop #2 only has time to make one
cycle and it results crash.
The aim of the patch is to skeep handling of sockets queued for
deletion.

Signed-off-by: Yuri Ershov <ext-yuri.ershov@nokia.com>
---
 net/bluetooth/af_bluetooth.c |    2 ++
 net/bluetooth/l2cap.c        |    6 ++++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index c4cf3f5..f9389da 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -200,6 +200,8 @@ struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock)
 	BT_DBG("parent %p", parent);
 
 	list_for_each_safe(p, n, &bt_sk(parent)->accept_q) {
+		if (n == p)
+			break;
 		sk = (struct sock *) list_entry(p, struct bt_sock, accept_q);
 
 		lock_sock(sk);
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 12b4aa2..29f30b0 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -133,7 +133,8 @@ static struct sock *__l2cap_get_chan_by_dcid(struct l2cap_chan_list *l, u16 cid)
 {
 	struct sock *s;
 	for (s = l->head; s; s = l2cap_pi(s)->next_c) {
-		if (l2cap_pi(s)->dcid == cid)
+		if ((l2cap_pi(s)->dcid == cid) &&
+		    (sk->sk_state != BT_DISCONN) && (sk->sk_state != BT_CLOSED))
 			break;
 	}
 	return s;
@@ -143,7 +144,8 @@ static struct sock *__l2cap_get_chan_by_scid(struct l2cap_chan_list *l, u16 cid)
 {
 	struct sock *s;
 	for (s = l->head; s; s = l2cap_pi(s)->next_c) {
-		if (l2cap_pi(s)->scid == cid)
+		if ((l2cap_pi(s)->scid == cid) &&
+		    (sk->sk_state != BT_DISCONN) && (sk->sk_state != BT_CLOSED))
 			break;
 	}
 	return s;
-- 
1.6.3.3

^ permalink raw reply related

* Re: sdptool hanging remote
From: Vinicius Gomes @ 2010-11-24 22:55 UTC (permalink / raw)
  To: Grahame Jordan; +Cc: linux-bluetooth
In-Reply-To: <4CED942E.8080204@theforce.com.au>

Hi,

On Wed, Nov 24, 2010 at 7:39 PM, Grahame Jordan <gbj@theforce.com.au> wrote:
> Hi,
>
> I have tested 4.77 4.80 and git. All appear to have the same issue.
>
>
> 2000-01-02 07:35:12.360814 < ACL data: handle 256 flags 0x02 dlen 18
>    L2CAP(s): Config rsp: scid 0x0041 flags 0x00 result 0 clen 4
>      MTU 672
> 2000-01-02 07:35:12.360884 < ACL data: handle 256 flags 0x02 dlen 12
>    L2CAP(s): Config req: dcid 0x0041 flags 0x00 clen 0
> 2000-01-02 07:35:12.370559 > HCI Event: Number of Completed Packets (0x13)
> plen 5
>    handle 256 packets 1
> 2000-01-02 07:35:12.380596 > HCI Event: Number of Completed Packets (0x13)
> plen 5
>    handle 256 packets 1
> 2000-01-02 07:35:12.380628 > ACL data: handle 256 flags 0x02 dlen 18
>    L2CAP(s): Config rsp: scid 0x0041 flags 0x00 result 0 clen 4
>      MTU 672
> 2000-01-02 07:35:12.410670 > ACL data: handle 256 flags 0x02 dlen 23
>    L2CAP(d): cid 0x0041 len 19 [psm 1]
>        SDP SA Req: tid 0x0 len 0xe
>          handle 0x1001e
>          max 65535
>          aid(s) 0x0000 - 0xffff
>          cont 00
> 2000-01-02 07:35:12.412008 < ACL data: handle 256 flags 0x02 dlen 11
>    L2CAP(d): cid 0x0041 len 7 [psm 1]
>        SDP Error Rsp: tid 0x0 len 0x2
>          code 0x2 info none
> 2000-01-02 07:35:12.430720 > HCI Event: Number of Completed Packets (0x13)
> plen 5
>    handle 256 packets 1
> 2000-01-02 07:35:12.430756 > ACL data: handle 256 flags 0x02 dlen 12
>    L2CAP(s): Disconn req: dcid 0x0041 scid 0x0041
> 2000-01-02 07:35:12.430979 < ACL data: handle 256 flags 0x02 dlen 12
>    L2CAP(s): Disconn rsp: dcid 0x0041 scid 0x0041
> 2000-01-02 07:35:12.440583 > HCI Event: Number of Completed Packets (0x13)
> plen 5
>    handle 256 packets 1
> 2000-01-02 07:35:12.460651 > ACL data: ha
> # Hang here
>
> # Resumed after about 10 seconds
> ndle 256 flags 0x02 dlen 12
>    L2CAP(s): Connect req: psm 1 scid 0x0042
> 2000-01-02 07:35:12.460904 < ACL data: handle 256 flags 0x02 dlen 16
>    L2CAP(s)i2c: error: timeout

This ("i2c: error: timeout") looks strange. But could be unrelated.

Another thing that could be interesting to have is the hcidump of an
attempt using Ubuntu 8.04 (that you said that had no problems).

The output of "sdptool browse local", from the gumstix would be also useful.

> : Connect rsp: dcid 0x0041 scid 0x0042 result 0 status 0
>      i2c: msg_num: 0 msg_idx: 1 msg_ptr: 0
> Connection successful
> 2000-01-02 07:35:12.480629 > HCI Event: Ni2c: ICR: 000097e0 ISR: 00000000
> umber of Completed Packets (0x13) plen 5
>    handle 256 packets 1
> 2000-01-02 07:35:12.510724 > ACL data: handle 256 flags 0x02 dlen 12
>    L2CAP(s): Config req: dcid 0x0041 flags 0x00 clen 0
> 2000-01-02 07:35:12.510946 < ACL data: handle 256 flags 0x02 dlen 18
>    L2CAP(s): Config rsp: scid 0x0042 flags 0x00 result 0 clen 4
>      MTU 672
> ....
>
> 2000-01-02 07:35:34.220696 > ACL data: handle 256 flags 0x02 dlen 23
>    L2CAP(d): cid 0x0041 len 19 [psm 1]
>        SDP SA Req: tid 0x0 len 0xe
>          handle 0x1030a
>          max 65535
>          aid(s) 0x0000 - 0xffff
>          cont 00
> 2000-01-02 07:35:34.222038 < ACL data: handle 256 flags 0x02 dlen 11
>    L2CAP(d): cid 0x0041 len 7 [psm 1]
>        SDP Error Rsp: tid 0x0 len 0x2
>          code 0x2 info none
> 2000-01-02 07:35:34.250696 > HCI Event: Number of Completed Packets (0x13)
> plen 5
>    handle 256 packets 1
> 2000-01-02 07:35:34.250733 > ACL data: handle 256 flags 0x02 dlen 12
>    L2CAP(s): Disconn req: dcid 0x0041 scid 0x0041
> 2000-01-02 07:35:34.250962 < ACL data: handle 256 flags 0x02 dlen 12
>    L2CAP(s): Disconn rsp: dcid 0x0041 scid 0x0041
> 2000-01-02
> # Hang here
>
> # Pressed CTL-C on remote "sdptool records 00:80:37:2F:5A:77"
> 07:35:34.260581 > HCI Event: Number of Completed Packets (0x13) plen 5
>    handle 256 packets 1
> 2000-01-02 07:35:34.280679 > ACL data: handle 256 flags 0x02 dlen 12
>    L2CAP(s): Connect req: psm 1 scid 0x0042
> 2000-01-02 07:35:34.280930 < ACL data: handle 256 flags 0x02 dlen 16
>    L2CAP(s): Connect rsp: dcid 0x0041 scid 0x0042 result 0 status 0
>      Connection successful
> 2000-01-02 07:35:34.300522 > HCI Event: Number of Completed Packets (0x13)
> plen 5
>    handle 256 packets 1
>
> ...
>
>
> Thanks
>
> Grahame Jordan
>
>
> On 24/11/10 04:51, Vinicius Costa Gomes wrote:
>>
>> On 16:37 Tue 23 Nov     , Grahame Jordan wrote:
>>
>>> Hi,
>>>
>>> I am using bluez-4.77 on a Gumstix Verdex kernel 2.4.32 patch 21
>>>
>>> When I run:
>>> sdptool records 00:80:37:2F:06:08"
>>> from the Workstation Ubuntu 10.04 Bluez-4.6? to the Gumstix the Gumstix
>>> hangs.
>>> It does not hang completely but is very busy. It hangs for about 20
>>> seconds and then releases for about 1 second
>>> before it locks up again.
>>>
>> I couldn't find anything that could solve this kind of problem in the logs
>> between 4.6 and the git head. But anyway, could you give a try to 4.80, or
>> better yet, the git head[1]? and see if the problem still happens?
>>
>>
>>> I have tried this on several different Gumstix with the same issue.
>>>
>>> Changing workstations from Ubuntu 10.04 to Ubuntu 8.04 does make a
>>> difference.
>>>  From Ubuntu 8.04 there seems to be no problem. blue-utils 3.26?
>>>
>>>
>> A really helpful piece of information that you could provide is the
>> hcidump[2]
>> logs of both cases, just be sure to give hcidump the options "-V" (verbose
>> output) and "-t" (timestamps, to see where the hang happens).
>>
>>
>>> Appreciate help
>>>
>>>
>>> Thanks
>>>
>>> Grahame Jordan
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe
>>> linux-bluetooth" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
>> Cheers,
>>
>
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


Cheers,
-- 
Vinicius

^ permalink raw reply

* Re: sdptool hanging remote
From: Grahame Jordan @ 2010-11-24 22:39 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <20101123175156.GA30860@eris.indt.org>

Hi,

I have tested 4.77 4.80 and git. All appear to have the same issue.


2000-01-02 07:35:12.360814 < ACL data: handle 256 flags 0x02 dlen 18
     L2CAP(s): Config rsp: scid 0x0041 flags 0x00 result 0 clen 4
       MTU 672
2000-01-02 07:35:12.360884 < ACL data: handle 256 flags 0x02 dlen 12
     L2CAP(s): Config req: dcid 0x0041 flags 0x00 clen 0
2000-01-02 07:35:12.370559 > HCI Event: Number of Completed Packets 
(0x13) plen 5
     handle 256 packets 1
2000-01-02 07:35:12.380596 > HCI Event: Number of Completed Packets 
(0x13) plen 5
     handle 256 packets 1
2000-01-02 07:35:12.380628 > ACL data: handle 256 flags 0x02 dlen 18
     L2CAP(s): Config rsp: scid 0x0041 flags 0x00 result 0 clen 4
       MTU 672
2000-01-02 07:35:12.410670 > ACL data: handle 256 flags 0x02 dlen 23
     L2CAP(d): cid 0x0041 len 19 [psm 1]
         SDP SA Req: tid 0x0 len 0xe
           handle 0x1001e
           max 65535
           aid(s) 0x0000 - 0xffff
           cont 00
2000-01-02 07:35:12.412008 < ACL data: handle 256 flags 0x02 dlen 11
     L2CAP(d): cid 0x0041 len 7 [psm 1]
         SDP Error Rsp: tid 0x0 len 0x2
           code 0x2 info none
2000-01-02 07:35:12.430720 > HCI Event: Number of Completed Packets 
(0x13) plen 5
     handle 256 packets 1
2000-01-02 07:35:12.430756 > ACL data: handle 256 flags 0x02 dlen 12
     L2CAP(s): Disconn req: dcid 0x0041 scid 0x0041
2000-01-02 07:35:12.430979 < ACL data: handle 256 flags 0x02 dlen 12
     L2CAP(s): Disconn rsp: dcid 0x0041 scid 0x0041
2000-01-02 07:35:12.440583 > HCI Event: Number of Completed Packets 
(0x13) plen 5
     handle 256 packets 1
2000-01-02 07:35:12.460651 > ACL data: ha
# Hang here

# Resumed after about 10 seconds
ndle 256 flags 0x02 dlen 12
     L2CAP(s): Connect req: psm 1 scid 0x0042
2000-01-02 07:35:12.460904 < ACL data: handle 256 flags 0x02 dlen 16
     L2CAP(s)i2c: error: timeout
: Connect rsp: dcid 0x0041 scid 0x0042 result 0 status 0
       i2c: msg_num: 0 msg_idx: 1 msg_ptr: 0
Connection successful
2000-01-02 07:35:12.480629 > HCI Event: Ni2c: ICR: 000097e0 ISR: 00000000
umber of Completed Packets (0x13) plen 5
     handle 256 packets 1
2000-01-02 07:35:12.510724 > ACL data: handle 256 flags 0x02 dlen 12
     L2CAP(s): Config req: dcid 0x0041 flags 0x00 clen 0
2000-01-02 07:35:12.510946 < ACL data: handle 256 flags 0x02 dlen 18
     L2CAP(s): Config rsp: scid 0x0042 flags 0x00 result 0 clen 4
       MTU 672
....

2000-01-02 07:35:34.220696 > ACL data: handle 256 flags 0x02 dlen 23
     L2CAP(d): cid 0x0041 len 19 [psm 1]
         SDP SA Req: tid 0x0 len 0xe
           handle 0x1030a
           max 65535
           aid(s) 0x0000 - 0xffff
           cont 00
2000-01-02 07:35:34.222038 < ACL data: handle 256 flags 0x02 dlen 11
     L2CAP(d): cid 0x0041 len 7 [psm 1]
         SDP Error Rsp: tid 0x0 len 0x2
           code 0x2 info none
2000-01-02 07:35:34.250696 > HCI Event: Number of Completed Packets 
(0x13) plen 5
     handle 256 packets 1
2000-01-02 07:35:34.250733 > ACL data: handle 256 flags 0x02 dlen 12
     L2CAP(s): Disconn req: dcid 0x0041 scid 0x0041
2000-01-02 07:35:34.250962 < ACL data: handle 256 flags 0x02 dlen 12
     L2CAP(s): Disconn rsp: dcid 0x0041 scid 0x0041
2000-01-02
# Hang here

# Pressed CTL-C on remote "sdptool records 00:80:37:2F:5A:77"
07:35:34.260581 > HCI Event: Number of Completed Packets (0x13) plen 5
     handle 256 packets 1
2000-01-02 07:35:34.280679 > ACL data: handle 256 flags 0x02 dlen 12
     L2CAP(s): Connect req: psm 1 scid 0x0042
2000-01-02 07:35:34.280930 < ACL data: handle 256 flags 0x02 dlen 16
     L2CAP(s): Connect rsp: dcid 0x0041 scid 0x0042 result 0 status 0
       Connection successful
2000-01-02 07:35:34.300522 > HCI Event: Number of Completed Packets 
(0x13) plen 5
     handle 256 packets 1

...


Thanks

Grahame Jordan


On 24/11/10 04:51, Vinicius Costa Gomes wrote:
> On 16:37 Tue 23 Nov     , Grahame Jordan wrote:
>
>> Hi,
>>
>> I am using bluez-4.77 on a Gumstix Verdex kernel 2.4.32 patch 21
>>
>> When I run:
>> sdptool records 00:80:37:2F:06:08"
>> from the Workstation Ubuntu 10.04 Bluez-4.6? to the Gumstix the Gumstix
>> hangs.
>> It does not hang completely but is very busy. It hangs for about 20
>> seconds and then releases for about 1 second
>> before it locks up again.
>>
> I couldn't find anything that could solve this kind of problem in the logs
> between 4.6 and the git head. But anyway, could you give a try to 4.80, or
> better yet, the git head[1]? and see if the problem still happens?
>
>
>> I have tried this on several different Gumstix with the same issue.
>>
>> Changing workstations from Ubuntu 10.04 to Ubuntu 8.04 does make a
>> difference.
>>   From Ubuntu 8.04 there seems to be no problem. blue-utils 3.26?
>>
>>
> A really helpful piece of information that you could provide is the hcidump[2]
> logs of both cases, just be sure to give hcidump the options "-V" (verbose
> output) and "-t" (timestamps, to see where the hang happens).
>
>
>> Appreciate help
>>
>>
>> Thanks
>>
>> Grahame Jordan
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
> Cheers,
>





^ permalink raw reply

* Re: [PATCH 2/3] Bluetooth: Add initial Bluetooth Management interface callbacks
From: Gustavo F. Padovan @ 2010-11-24 21:58 UTC (permalink / raw)
  To: Andrei Emeltchenko, linux-bluetooth
In-Reply-To: <20101124214747.GA8626@jh-x301>

* Johan Hedberg <johan.hedberg@gmail.com> [2010-11-24 23:47:47 +0200]:

> Hi Andrei,
> 
> On Wed, Nov 24, 2010, Andrei Emeltchenko wrote:
> > > -       if (haddr->hci_dev != HCI_DEV_NONE) {
> > > -               if (!(hdev = hci_dev_get(haddr->hci_dev))) {
> > > +       if (haddr.hci_dev != HCI_DEV_NONE) {
> > > +               if (!(hdev = hci_dev_get(haddr.hci_dev))) {
> > 
> > doesn't checkpatch give errors here?
> 
> Probably, but I've understood that it's ok if it's the existing code
> that contains the coding style issue.
> 
> > Would be more clean like:
> > ...
> > hdev = hci_dev_get(haddr.hci_dev);
> > if (!hdev)
> > ...
> > 
> > At some point shall be fixed in the old code also
> 
> Agreed. A separate code cleanup patch would be nice. I've intentionally
> kept the old style to not mix coding style and functional changes into
> the same patch and to make it clear that I'm not introducing any changes
> to the code logic at this place.

Yes, that should be a separated patch.

Johan, your patches are fine, but I have to wait the wireless-next-2.6
be synced with the net-next-2.6 tree. Then I'll able to apply it.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [PATCH 2/3] Bluetooth: Add initial Bluetooth Management interface callbacks
From: Johan Hedberg @ 2010-11-24 21:47 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <AANLkTik0x3tjnM85Mu=vwU6GB2pXAgWXnqe28aKS+QkN@mail.gmail.com>

Hi Andrei,

On Wed, Nov 24, 2010, Andrei Emeltchenko wrote:
> > -       if (haddr->hci_dev != HCI_DEV_NONE) {
> > -               if (!(hdev = hci_dev_get(haddr->hci_dev))) {
> > +       if (haddr.hci_dev != HCI_DEV_NONE) {
> > +               if (!(hdev = hci_dev_get(haddr.hci_dev))) {
> 
> doesn't checkpatch give errors here?

Probably, but I've understood that it's ok if it's the existing code
that contains the coding style issue.

> Would be more clean like:
> ...
> hdev = hci_dev_get(haddr.hci_dev);
> if (!hdev)
> ...
> 
> At some point shall be fixed in the old code also

Agreed. A separate code cleanup patch would be nice. I've intentionally
kept the old style to not mix coding style and functional changes into
the same patch and to make it clear that I'm not introducing any changes
to the code logic at this place.

Johan

^ permalink raw reply

* Re: [RFC 00/20] Simple SMP Just Works implementation
From: Gustavo F. Padovan @ 2010-11-24 21:08 UTC (permalink / raw)
  To: Ville Tervo; +Cc: ext Vinicius Costa Gomes, linux-bluetooth
In-Reply-To: <20101124054211.GI874@null>

Hi Ville,

* Ville Tervo <ville.tervo@nokia.com> [2010-11-24 07:42:11 +0200]:

> Hi,
> 
> 
> On Tue, Nov 23, 2010 at 12:06:16PM -0300, ext Vinicius Costa Gomes wrote:
> > Hi,
> > 
> > This is an implementation of the Just Works SMP procedure, on top of the work
> > done by Ville (included in this series to give some context). The SMP stuff
> > starts at 11/20.
> > 
> > The most important thing about this series is the discussion that (I hope)
> > it will cause.
> > 
> > Some things that I would like to point (in no order):
> > 
> > - the SMP function names follow the spec nomeclature, would it be better to
> >   use more meaningful names?
> > 
> > - the crypto transform is allocated during the adapter registration, is this
> >   the best place to do this?
> > 
> > - renaming l2cap.c to l2cap_core.c was the only way we could find to keep the
> >   SMP implementation separated from the "core" l2cap.
> 
> Gustavo do you have something against this change?

I'm ok with this change.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [PATCH v2] Bluetooth: Fix error handling for l2cap_init()
From: Gustavo F. Padovan @ 2010-11-24 21:05 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: Marcel Holtmann, linux-bluetooth
In-Reply-To: <AANLkTi=pwDA5JQCoz4QNO=mLAHuW+moNOEO4Nqz3jQrJ@mail.gmail.com>

Hi Anderson,

* Anderson Lizardo <anderson.lizardo@openbossa.org> [2010-11-24 11:13:30 -0=
400]:

> Hi Marcel,
>=20
> On Wed, Nov 24, 2010 at 8:42 AM, Marcel Holtmann <marcel@holtmann.org> wr=
ote:
> >> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> >> index 18a802c..7980e24 100644
> >> --- a/net/bluetooth/l2cap.c
> >> +++ b/net/bluetooth/l2cap.c
> >> @@ -4875,8 +4875,10 @@ static int __init l2cap_init(void)
> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return err;
> >>
> >> =A0 =A0 =A0 _busy_wq =3D create_singlethread_workqueue("l2cap");
> >> - =A0 =A0 if (!_busy_wq)
> >> - =A0 =A0 =A0 =A0 =A0 =A0 goto error;
> >> + =A0 =A0 if (!_busy_wq) {
> >> + =A0 =A0 =A0 =A0 =A0 =A0 err =3D -ENOMEM;
> >> + =A0 =A0 =A0 =A0 =A0 =A0 goto error_busy_wq;
> >> + =A0 =A0 }
> >
> > aren't these returning PTR_ERR etc.?
>=20
> No, create_singlethread_workqueue() is just a wrapper around
> __alloc_workqueue_key(), which returns eiter a kzalloc()'ed pointer,
> or NULL on error. There is no way to get the actual reason of the
> error, but by taking a look at the function we can see most (if not
> all) errors are -ENOMEM. Thus why I used it here.
>=20
> Padovan: so how to proceed here: keep the patch as is and keep
> semantics, of make your proposed changes (with a slightly risk of a
> race condition and having _busy_wq NULL) ?

I'm not sure that my idea is right, so I have another option here. On
create_singlethread_workqueue error, just call proto_unregister() and
then return -ENOMEM, and destroy your workqueue under the label error.
This way we avoid create a new label and also have a simple error
handling there.

--=20
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: [PATCH 2/3] Bluetooth: Add initial Bluetooth Management interface callbacks
From: Gustavo F. Padovan @ 2010-11-24 19:10 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: johan.hedberg, linux-bluetooth, Johan Hedberg
In-Reply-To: <AANLkTimFz=FRPiGT31xt9wMjWoxC5cuKBubEqYNT0s=5@mail.gmail.com>

Hi Anderson,

* Anderson Lizardo <anderson.lizardo@openbossa.org> [2010-11-24 11:38:22 -0400]:

> Hi Johan,
> 
> On Wed, Nov 24, 2010 at 10:39 AM,  <johan.hedberg@gmail.com> wrote:
> >  static int hci_sock_bind(struct socket *sock, struct sockaddr *addr, int addr_len)
> >  {
> > -       struct sockaddr_hci *haddr = (struct sockaddr_hci *) addr;
> > +       struct sockaddr_hci haddr;
> 
> Just out of curiosity: why is this change necessary (i.e. make a stack
> copy of addr data and use it instead of using a cast of addr)?
> 
> >        struct sock *sk = sock->sk;
> >        struct hci_dev *hdev = NULL;
> > -       int err = 0;
> > +       int len, err = 0;
> >
> >        BT_DBG("sock %p sk %p", sock, sk);
> >
> > -       if (!haddr || haddr->hci_family != AF_BLUETOOTH)
> > +       if (!addr)
> > +               return -EINVAL;
> > +
> > +       memset(&haddr, 0, sizeof(haddr));
> > +       len = min_t(unsigned int, sizeof(haddr), addr_len);
> > +       memcpy(&haddr, addr, len);
> 
> Looks like you are playing safe here, but looking at least a few
> ->bind() implementations I see most just cast the original struct
> sockaddr, which is has size (sizeof(unsigned short) + 14).

Older userspace versions can use smaller struct sockaddr, so it's a
better idea move to th stack and zero-filling the the struct before the
copy the data, this way if the size of the data copied is smaller than
the struct, the fields in the end of the struct will be filled with
zeros and not something stranger.

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* [PATCH] sdpd header cleanup
From: Claudio Takahasi @ 2010-11-24 18:16 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Claudio Takahasi

---
 src/sdpd-request.c |   12 +++++++++++-
 src/sdpd.h         |   14 --------------
 2 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/src/sdpd-request.c b/src/sdpd-request.c
index 025de60..9e4b6b8 100644
--- a/src/sdpd-request.c
+++ b/src/sdpd-request.c
@@ -45,6 +45,16 @@
 #include "sdpd.h"
 #include "log.h"
 
+typedef struct {
+	uint32_t timestamp;
+	union {
+		uint16_t maxBytesSent;
+		uint16_t lastIndexSent;
+	} cStateValue;
+} sdp_cont_state_t;
+
+#define SDP_CONT_STATE_SIZE (sizeof(uint8_t) + sizeof(sdp_cont_state_t))
+
 #define MIN(x, y) ((x) < (y)) ? (x): (y)
 
 typedef struct _sdp_cstate_list sdp_cstate_list_t;
@@ -58,7 +68,7 @@ struct _sdp_cstate_list {
 static sdp_cstate_list_t *cstates;
 
 // FIXME: should probably remove it when it's found
-sdp_buf_t *sdp_get_cached_rsp(sdp_cont_state_t *cstate)
+static sdp_buf_t *sdp_get_cached_rsp(sdp_cont_state_t *cstate)
 {
 	sdp_cstate_list_t *p;
 
diff --git a/src/sdpd.h b/src/sdpd.h
index a46ad3c..f8e6ee7 100644
--- a/src/sdpd.h
+++ b/src/sdpd.h
@@ -79,20 +79,6 @@ void register_server_service(void);
 void register_device_id(const uint16_t vendor, const uint16_t product,
 						const uint16_t version);
 
-typedef struct {
-	uint32_t timestamp;
-	union {
-		uint16_t maxBytesSent;
-		uint16_t lastIndexSent;
-	} cStateValue;
-} sdp_cont_state_t;
-
-#define SDP_CONT_STATE_SIZE (sizeof(uint8_t) + sizeof(sdp_cont_state_t))
-
-sdp_buf_t *sdp_get_cached_rsp(sdp_cont_state_t *cstate);
-void sdp_cstate_cache_init(void);
-void sdp_cstate_clean_buf(void);
-
 int record_sort(const void *r1, const void *r2);
 void sdp_svcdb_reset(void);
 void sdp_svcdb_collect_all(int sock);
-- 
1.7.3.2


^ permalink raw reply related

* Re: [PATCH 2/3] Bluetooth: Add initial Bluetooth Management interface callbacks
From: Anderson Lizardo @ 2010-11-24 15:38 UTC (permalink / raw)
  To: johan.hedberg; +Cc: linux-bluetooth, Johan Hedberg
In-Reply-To: <1290609575-28435-3-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

On Wed, Nov 24, 2010 at 10:39 AM,  <johan.hedberg@gmail.com> wrote:
>  static int hci_sock_bind(struct socket *sock, struct sockaddr *addr, int addr_len)
>  {
> -       struct sockaddr_hci *haddr = (struct sockaddr_hci *) addr;
> +       struct sockaddr_hci haddr;

Just out of curiosity: why is this change necessary (i.e. make a stack
copy of addr data and use it instead of using a cast of addr)?

>        struct sock *sk = sock->sk;
>        struct hci_dev *hdev = NULL;
> -       int err = 0;
> +       int len, err = 0;
>
>        BT_DBG("sock %p sk %p", sock, sk);
>
> -       if (!haddr || haddr->hci_family != AF_BLUETOOTH)
> +       if (!addr)
> +               return -EINVAL;
> +
> +       memset(&haddr, 0, sizeof(haddr));
> +       len = min_t(unsigned int, sizeof(haddr), addr_len);
> +       memcpy(&haddr, addr, len);

Looks like you are playing safe here, but looking at least a few
->bind() implementations I see most just cast the original struct
sockaddr, which is has size (sizeof(unsigned short) + 14).

Regards,
-- 
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil

^ permalink raw reply

* Re: [PATCH v2] Bluetooth: Fix error handling for l2cap_init()
From: Anderson Lizardo @ 2010-11-24 15:13 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth, padovan
In-Reply-To: <1290602554.13641.13.camel@aeonflux.mpl.access-company.com>

Hi Marcel,

On Wed, Nov 24, 2010 at 8:42 AM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
>> index 18a802c..7980e24 100644
>> --- a/net/bluetooth/l2cap.c
>> +++ b/net/bluetooth/l2cap.c
>> @@ -4875,8 +4875,10 @@ static int __init l2cap_init(void)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return err;
>>
>> =A0 =A0 =A0 _busy_wq =3D create_singlethread_workqueue("l2cap");
>> - =A0 =A0 if (!_busy_wq)
>> - =A0 =A0 =A0 =A0 =A0 =A0 goto error;
>> + =A0 =A0 if (!_busy_wq) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 err =3D -ENOMEM;
>> + =A0 =A0 =A0 =A0 =A0 =A0 goto error_busy_wq;
>> + =A0 =A0 }
>
> aren't these returning PTR_ERR etc.?

No, create_singlethread_workqueue() is just a wrapper around
__alloc_workqueue_key(), which returns eiter a kzalloc()'ed pointer,
or NULL on error. There is no way to get the actual reason of the
error, but by taking a look at the function we can see most (if not
all) errors are -ENOMEM. Thus why I used it here.

Padovan: so how to proceed here: keep the patch as is and keep
semantics, of make your proposed changes (with a slightly risk of a
race condition and having _busy_wq NULL) ?

Regards,
--=20
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil

^ permalink raw reply

* [PATCH 6/6] Code clean-up: lines longer 80 symbols removed
From: Dmitriy Paliy @ 2010-11-24 15:07 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Dmitriy Paliy
In-Reply-To: <1290611236-25656-1-git-send-email-dmitriy.paliy@nokia.com>

---
 plugins/pbap.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/plugins/pbap.c b/plugins/pbap.c
index e0df444..9d166d9 100644
--- a/plugins/pbap.c
+++ b/plugins/pbap.c
@@ -263,8 +263,8 @@ static void query_result(const char *buffer, size_t bufsize, int vcards,
 	if (!pbap->obj->buffer)
 		pbap->obj->buffer = g_string_new_len(buffer, bufsize);
 	else
-		pbap->obj->buffer = g_string_append_len(pbap->obj->buffer, buffer,
-								bufsize);
+		pbap->obj->buffer = g_string_append_len(pbap->obj->buffer,
+							buffer, bufsize);
 
 	obex_object_set_io_flags(pbap->obj, G_IO_IN, 0);
 }
@@ -423,11 +423,12 @@ static void cache_ready_notify(void *user_data)
 	for (; l && max; l = l->next, max--) {
 		const struct cache_entry *entry = l->data;
 
-		g_string_append_printf(pbap->obj->buffer, VCARD_LISTING_ELEMENT,
-						entry->handle, entry->name);
+		g_string_append_printf(pbap->obj->buffer,
+			VCARD_LISTING_ELEMENT, entry->handle, entry->name);
 	}
 
-	pbap->obj->buffer = g_string_append(pbap->obj->buffer, VCARD_LISTING_END);
+	pbap->obj->buffer = g_string_append(pbap->obj->buffer,
+							VCARD_LISTING_END);
 
 	g_slist_free(sorted);
 
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 5/6] Add update phonebook_create_cache
From: Dmitriy Paliy @ 2010-11-24 15:07 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Dmitriy Paliy
In-Reply-To: <1290611236-25656-1-git-send-email-dmitriy.paliy@nokia.com>

Add phonebook_create_cache updated function prototype to return void
pointer to backend specific request and error code. All backend
handlers updated accordingly (phonebook-tracker.c,
phonebook-dummy.c, and phonebook-ebook.c).

In PBAP vobject_list_open and vobject_vcard_open are updated to
store pointer to phonebook request. IRMC is not modified.

Phonebook request is stored and canceled only for tracker.
---
 plugins/pbap.c              |   14 +++++++++-----
 plugins/phonebook-dummy.c   |   16 ++++++++++------
 plugins/phonebook-ebook.c   |   20 ++++++++++++++------
 plugins/phonebook-tracker.c |   16 ++++++++--------
 plugins/phonebook.h         |    4 ++--
 5 files changed, 43 insertions(+), 27 deletions(-)

diff --git a/plugins/pbap.c b/plugins/pbap.c
index 40bda69..e0df444 100644
--- a/plugins/pbap.c
+++ b/plugins/pbap.c
@@ -759,6 +759,7 @@ static void *vobject_list_open(const char *name, int oflag, mode_t mode,
 {
 	struct pbap_session *pbap = context;
 	int ret = 0;
+	void *request = NULL;
 
 	DBG("name %s context %p valid %d", name, context, pbap->cache.valid);
 
@@ -788,14 +789,17 @@ static void *vobject_list_open(const char *name, int oflag, mode_t mode,
 		goto done;
 	}
 
-	ret = phonebook_create_cache(name,
-		cache_entry_notify, cache_ready_notify, pbap);
+	request = phonebook_create_cache(name,
+		cache_entry_notify, cache_ready_notify, pbap, &ret);
 
 	if (ret < 0)
 		goto fail;
 
 done:
-	return vobject_create(pbap, NULL);
+	if (err)
+		*err = ret;
+
+	return vobject_create(pbap, request);
 
 fail:
 	if (err)
@@ -827,8 +831,8 @@ static void *vobject_vcard_open(const char *name, int oflag, mode_t mode,
 
 	if (pbap->cache.valid == FALSE) {
 		pbap->find_handle = handle;
-		ret = phonebook_create_cache(pbap->folder, cache_entry_notify,
-						cache_entry_done, pbap);
+		request = phonebook_create_cache(pbap->folder,
+			cache_entry_notify, cache_entry_done, pbap, &ret);
 		goto done;
 	}
 
diff --git a/plugins/phonebook-dummy.c b/plugins/phonebook-dummy.c
index 7a963b6..a269ea8 100644
--- a/plugins/phonebook-dummy.c
+++ b/plugins/phonebook-dummy.c
@@ -528,8 +528,8 @@ void *phonebook_get_entry(const char *folder, const char *id,
 	return NULL;
 }
 
-int phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
-		phonebook_cache_ready_cb ready_cb, void *user_data)
+void *phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
+		phonebook_cache_ready_cb ready_cb, void *user_data, int *err)
 {
 	struct cache_query *query;
 	char *foldername;
@@ -540,9 +540,10 @@ int phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
 	g_free(foldername);
 
 	if (dp == NULL) {
-		int err = errno;
-		DBG("opendir(): %s(%d)", strerror(err), err);
-		return -ENOENT;
+		DBG("opendir(): %s(%d)", strerror(errno), errno);
+		if (err)
+			*err = -ENOENT;
+		return NULL;
 	}
 
 	query = g_new0(struct cache_query, 1);
@@ -553,5 +554,8 @@ int phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
 
 	g_idle_add_full(G_PRIORITY_DEFAULT_IDLE, create_cache, query,
 								query_free);
-	return 0;
+	if (err)
+		*err = 0;
+
+	return NULL;
 }
diff --git a/plugins/phonebook-ebook.c b/plugins/phonebook-ebook.c
index 2515bb0..5d7f624 100644
--- a/plugins/phonebook-ebook.c
+++ b/plugins/phonebook-ebook.c
@@ -459,15 +459,18 @@ void *phonebook_get_entry(const char *folder, const char *id,
 	return NULL;
 }
 
-int phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
-			phonebook_cache_ready_cb ready_cb, void *user_data)
+void *phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
+		phonebook_cache_ready_cb ready_cb, void *user_data, int *err)
 {
 	struct cache_query *data;
 	EBookQuery *query;
 	gboolean ret;
 
-	if (g_strcmp0("/telecom/pb", name) != 0)
-		return -ENOENT;
+	if (g_strcmp0("/telecom/pb", name) != 0) {
+		if (err)
+			*err = -ENOENT;
+		return NULL;
+	}
 
 	query = e_book_query_any_field_contains("");
 
@@ -480,8 +483,13 @@ int phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
 	e_book_query_unref(query);
 	if (ret != FALSE) {
 		g_free(data);
-		return -EFAULT;
+		if (err)
+			*err = -EFAULT;
+		return NULL;
 	}
 
-	return 0;
+	if (err)
+		*err = 0;
+
+	return NULL;
 }
diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index 142e799..cd2ff31 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -1880,25 +1880,25 @@ void *phonebook_get_entry(const char *folder, const char *id,
 	return call;
 }
 
-int phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
-			phonebook_cache_ready_cb ready_cb, void *user_data)
+void *phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
+		phonebook_cache_ready_cb ready_cb, void *user_data, int *err)
 {
 	struct cache_data *cache;
 	const char *query;
-	int ret;
 
 	DBG("name %s", name);
 
 	query = folder2query(name);
-	if (query == NULL)
-		return -ENOENT;
+	if (query == NULL) {
+		if (err)
+			*err = -ENOENT;
+		return NULL;
+	}
 
 	cache = g_new0(struct cache_data, 1);
 	cache->entry_cb = entry_cb;
 	cache->ready_cb = ready_cb;
 	cache->user_data = user_data;
 
-	query_tracker(query, 7, add_to_cache, cache, &ret);
-
-	return ret;
+	return query_tracker(query, 7, add_to_cache, cache, err);
 }
diff --git a/plugins/phonebook.h b/plugins/phonebook.h
index 5342841..b6ae5a8 100644
--- a/plugins/phonebook.h
+++ b/plugins/phonebook.h
@@ -105,8 +105,8 @@ void *phonebook_get_entry(const char *folder, const char *id,
  * Cache will store only the necessary information required to reply to
  * PullvCardListing request and verify if a given contact belongs to the source.
  */
-int phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
-			phonebook_cache_ready_cb ready_cb, void *user_data);
+void *phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
+		phonebook_cache_ready_cb ready_cb, void *user_data, int *err);
 
 /* 
  * Function used to cancel pending request to backend and free resources
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 4/6] Add update phonebook_get_entry
From: Dmitriy Paliy @ 2010-11-24 15:07 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Dmitriy Paliy
In-Reply-To: <1290611236-25656-1-git-send-email-dmitriy.paliy@nokia.com>

Add phonebook_get_entry function prototype updated to return void
pointer to backend specific request and error code. All backend
handlers updated accordingly (phonebook-tracker.c,
phonebook-dummy.c, and phonebook-ebook.c).

pbap.c is updated accordingly to handle modified
phonebook_get_entry. Phonebook request is stored for tracker only
and canceled when PBAP object is destroyed.

IRMC does not invoke phonebook_get_entry therefore is not changed.
---
 plugins/pbap.c              |   16 ++++++++++------
 plugins/phonebook-dummy.c   |   18 +++++++++++-------
 plugins/phonebook-ebook.c   |   15 ++++++++++-----
 plugins/phonebook-tracker.c |   13 +++++++------
 plugins/phonebook.h         |    4 ++--
 5 files changed, 40 insertions(+), 26 deletions(-)

diff --git a/plugins/pbap.c b/plugins/pbap.c
index 0ceaf95..40bda69 100644
--- a/plugins/pbap.c
+++ b/plugins/pbap.c
@@ -455,8 +455,8 @@ static void cache_entry_done(void *user_data)
 		return;
 	}
 
-	ret = phonebook_get_entry(pbap->folder, id, pbap->params,
-						query_result, pbap);
+	pbap->obj->request = phonebook_get_entry(pbap->folder, id,
+				pbap->params, query_result, pbap, &ret);
 	if (ret < 0)
 		obex_object_set_io_flags(pbap->obj, G_IO_ERR, ret);
 }
@@ -758,7 +758,7 @@ static void *vobject_list_open(const char *name, int oflag, mode_t mode,
 				void *context, size_t *size, int *err)
 {
 	struct pbap_session *pbap = context;
-	int ret;
+	int ret = 0;
 
 	DBG("name %s context %p valid %d", name, context, pbap->cache.valid);
 
@@ -811,6 +811,7 @@ static void *vobject_vcard_open(const char *name, int oflag, mode_t mode,
 	const char *id;
 	uint32_t handle;
 	int ret;
+	void *request = NULL;
 
 	DBG("name %s context %p valid %d", name, context, pbap->cache.valid);
 
@@ -837,14 +838,17 @@ static void *vobject_vcard_open(const char *name, int oflag, mode_t mode,
 		goto fail;
 	}
 
-	ret = phonebook_get_entry(pbap->folder, id, pbap->params, query_result,
-									pbap);
+	request = phonebook_get_entry(pbap->folder, id, pbap->params,
+						query_result, pbap, &ret);
 
 done:
 	if (ret < 0)
 		goto fail;
 
-	return vobject_create(pbap, NULL);
+	if (err)
+		*err = ret;
+
+	return vobject_create(pbap, request);
 
 fail:
 	if (err)
diff --git a/plugins/phonebook-dummy.c b/plugins/phonebook-dummy.c
index d9a5a29..7a963b6 100644
--- a/plugins/phonebook-dummy.c
+++ b/plugins/phonebook-dummy.c
@@ -496,9 +496,9 @@ void *phonebook_pull(const char *name, const struct apparam_field *params,
 	return NULL;
 }
 
-int phonebook_get_entry(const char *folder, const char *id,
-					const struct apparam_field *params,
-					phonebook_cb cb, void *user_data)
+void *phonebook_get_entry(const char *folder, const char *id,
+			const struct apparam_field *params, phonebook_cb cb,
+			void *user_data, int *err)
 {
 	struct dummy_data *dummy;
 	char *filename;
@@ -508,9 +508,10 @@ int phonebook_get_entry(const char *folder, const char *id,
 
 	fd = open(filename, O_RDONLY);
 	if (fd < 0) {
-		int err = errno;
-		DBG("open(): %s(%d)", strerror(err), err);
-		return -ENOENT;
+		DBG("open(): %s(%d)", strerror(errno), errno);
+		if (err)
+			*err = -ENOENT;
+		return NULL;
 	}
 
 	dummy = g_new0(struct dummy_data, 1);
@@ -521,7 +522,10 @@ int phonebook_get_entry(const char *folder, const char *id,
 
 	g_idle_add_full(G_PRIORITY_DEFAULT_IDLE, read_entry, dummy, dummy_free);
 
-	return 0;
+	if (err)
+		*err = 0;
+
+	return NULL;
 }
 
 int phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
diff --git a/plugins/phonebook-ebook.c b/plugins/phonebook-ebook.c
index 9097a0b..2515bb0 100644
--- a/plugins/phonebook-ebook.c
+++ b/plugins/phonebook-ebook.c
@@ -435,9 +435,9 @@ void *phonebook_pull(const char *name, const struct apparam_field *params,
 	return NULL;
 }
 
-int phonebook_get_entry(const char *folder, const char *id,
-					const struct apparam_field *params,
-					phonebook_cb cb, void *user_data)
+void *phonebook_get_entry(const char *folder, const char *id,
+				const struct apparam_field *params,
+				phonebook_cb cb, void *user_data, int *err)
 {
 	struct contacts_query *data;
 
@@ -448,10 +448,15 @@ int phonebook_get_entry(const char *folder, const char *id,
 
 	if (e_book_async_get_contact(ebook, id, ebook_entry_cb, data)) {
 		g_free(data);
-		return -ENOENT;
+		if (err)
+			*err = -ENOENT;
+		return NULL;
 	}
 
-	return 0;
+	if (err)
+		*err = 0;
+
+	return NULL;
 }
 
 int phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index c8c8e54..142e799 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -1848,13 +1848,13 @@ void *phonebook_pull(const char *name, const struct apparam_field *params,
 	return query_tracker(query, col_amount, pull_cb, data, err);
 }
 
-int phonebook_get_entry(const char *folder, const char *id,
-					const struct apparam_field *params,
-					phonebook_cb cb, void *user_data)
+void *phonebook_get_entry(const char *folder, const char *id,
+				const struct apparam_field *params,
+				phonebook_cb cb, void *user_data, int *err)
 {
 	struct phonebook_data *data;
 	char *query;
-	int ret;
+	DBusPendingCall *call;
 
 	DBG("folder %s id %s", folder, id);
 
@@ -1872,11 +1872,12 @@ int phonebook_get_entry(const char *folder, const char *id,
 		query = g_strdup_printf(CONTACTS_OTHER_QUERY_FROM_URI,
 								id, id, id);
 
-	query_tracker(query, PULL_QUERY_COL_AMOUNT, pull_contacts, data, &ret);
+	call = query_tracker(query, PULL_QUERY_COL_AMOUNT, pull_contacts, data,
+									err);
 
 	g_free(query);
 
-	return ret;
+	return call;
 }
 
 int phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
diff --git a/plugins/phonebook.h b/plugins/phonebook.h
index 951b370..5342841 100644
--- a/plugins/phonebook.h
+++ b/plugins/phonebook.h
@@ -95,9 +95,9 @@ void *phonebook_pull(const char *name, const struct apparam_field *params,
  * return only the content based on the application parameters requested
  * by the client.
  */
-int phonebook_get_entry(const char *folder, const char *id,
+void *phonebook_get_entry(const char *folder, const char *id,
 				const struct apparam_field *params,
-				phonebook_cb cb, void *user_data);
+				phonebook_cb cb, void *user_data, int *err);
 
 /*
  * PBAP core will keep the contacts cache per folder. SetPhoneBook or
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 3/6] Add update phonebook_pull
From: Dmitriy Paliy @ 2010-11-24 15:07 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Dmitriy Paliy
In-Reply-To: <1290611236-25656-1-git-send-email-dmitriy.paliy@nokia.com>

Add phonebook request pointer to pending call in PBAP object.

Phonebook_pull function prototype is updated to return void
pointer to backend specific request and error code. All backend
handlers updated accordingly (phonebook-tracker.c,
phonebook-dummy.c, and phonebook-ebook.c).

Phonebook request is stored only for tracker. It is canceled
when PBAP object is destroyed, or IRMC close method invoked.

query_tracker is updated to return pending call request.

IRMC is updated to handle pending request when phonebook is pulled.

phonebook-dummy.c and phonebook-ebook.c are updated also with
phonebook_req_cancel function.
---
 plugins/irmc.c              |   14 +++++++-------
 plugins/pbap.c              |   20 +++++++++++++++-----
 plugins/phonebook-dummy.c   |   21 ++++++++++++++++-----
 plugins/phonebook-ebook.c   |   13 ++++++++++---
 plugins/phonebook-tracker.c |   34 ++++++++++++++++++++++------------
 plugins/phonebook.h         |    4 ++--
 6 files changed, 72 insertions(+), 34 deletions(-)

diff --git a/plugins/irmc.c b/plugins/irmc.c
index f7ad33b..4c11933 100644
--- a/plugins/irmc.c
+++ b/plugins/irmc.c
@@ -110,6 +110,7 @@ struct irmc_session {
 	char did[DID_LEN];
 	char manu[DID_LEN];
 	char model[DID_LEN];
+	void *request;
 };
 
 #define IRMC_TARGET_SIZE 9
@@ -210,11 +211,8 @@ static void *irmc_connect(struct obex_session *os, int *err)
 	param->maxlistcount = 0; /* to count the number of vcards... */
 	param->filter = 0x200085; /* UID TEL N VERSION */
 	irmc->params = param;
-	phonebook_pull("telecom/pb.vcf", irmc->params, phonebook_size_result,
-									irmc);
-
-	if (err)
-		*err = 0;
+	irmc->request = phonebook_pull("telecom/pb.vcf", irmc->params,
+					phonebook_size_result, irmc, err);
 
 	return irmc;
 }
@@ -298,8 +296,8 @@ static void *irmc_open_pb(const char *name, struct irmc_session *irmc,
 
 	if (!g_strcmp0(name, ".vcf")) {
 		/* how can we tell if the vcard count call already finished? */
-		ret = phonebook_pull("telecom/pb.vcf", irmc->params,
-							query_result, irmc);
+		irmc->request = phonebook_pull("telecom/pb.vcf", irmc->params,
+						query_result, irmc, &ret);
 		if (ret < 0) {
 			DBG("phonebook_pull failed...");
 			goto fail;
@@ -435,6 +433,8 @@ static int irmc_close(void *object)
 		irmc->buffer = NULL;
 	}
 
+	phonebook_req_cancel(irmc->request);
+
 	return 0;
 }
 
diff --git a/plugins/pbap.c b/plugins/pbap.c
index 1a7d001..0ceaf95 100644
--- a/plugins/pbap.c
+++ b/plugins/pbap.c
@@ -147,6 +147,7 @@ struct pbap_session {
 struct pbap_object {
 	GString *buffer;
 	struct pbap_session *session;
+	void *request;
 };
 
 static const uint8_t PBAP_TARGET[TARGET_SIZE] = {
@@ -697,13 +698,15 @@ static struct obex_service_driver pbap = {
 	.chkput = pbap_chkput
 };
 
-static struct pbap_object *vobject_create(struct pbap_session *pbap)
+static struct pbap_object *vobject_create(struct pbap_session *pbap,
+								void *request)
 {
 	struct pbap_object *obj;
 
 	obj = g_new0(struct pbap_object, 1);
 	obj->session = pbap;
 	pbap->obj = obj;
+	obj->request = request;
 
 	return obj;
 }
@@ -714,6 +717,7 @@ static void *vobject_pull_open(const char *name, int oflag, mode_t mode,
 	struct pbap_session *pbap = context;
 	phonebook_cb cb;
 	int ret;
+	void *request = NULL;
 
 	DBG("name %s context %p maxlistcount %d", name, context,
 						pbap->params->maxlistcount);
@@ -733,11 +737,15 @@ static void *vobject_pull_open(const char *name, int oflag, mode_t mode,
 	else
 		cb = query_result;
 
-	ret = phonebook_pull(name, pbap->params, cb, pbap);
+	request = phonebook_pull(name, pbap->params, cb, pbap, &ret);
+
 	if (ret < 0)
 		goto fail;
 
-	return vobject_create(pbap);
+	if (err)
+		*err = ret;
+
+	return vobject_create(pbap, request);
 
 fail:
 	if (err)
@@ -787,7 +795,7 @@ static void *vobject_list_open(const char *name, int oflag, mode_t mode,
 		goto fail;
 
 done:
-	return vobject_create(pbap);
+	return vobject_create(pbap, NULL);
 
 fail:
 	if (err)
@@ -836,7 +844,7 @@ done:
 	if (ret < 0)
 		goto fail;
 
-	return vobject_create(pbap);
+	return vobject_create(pbap, NULL);
 
 fail:
 	if (err)
@@ -912,6 +920,8 @@ static int vobject_close(void *object)
 	if (obj->buffer)
 		g_string_free(obj->buffer, TRUE);
 
+	phonebook_req_cancel(obj->request);
+
 	g_free(obj);
 
 	return 0;
diff --git a/plugins/phonebook-dummy.c b/plugins/phonebook-dummy.c
index 7c549fa..d9a5a29 100644
--- a/plugins/phonebook-dummy.c
+++ b/plugins/phonebook-dummy.c
@@ -447,8 +447,12 @@ done:
 	return relative;
 }
 
-int phonebook_pull(const char *name, const struct apparam_field *params,
-					phonebook_cb cb, void *user_data)
+void phonebook_req_cancel(void *request)
+{
+}
+
+void *phonebook_pull(const char *name, const struct apparam_field *params,
+				phonebook_cb cb, void *user_data, int *err)
 {
 	struct dummy_data *dummy;
 	char *filename, *folder;
@@ -463,14 +467,18 @@ int phonebook_pull(const char *name, const struct apparam_field *params,
 
 	if (!g_str_has_suffix(filename, ".vcf")) {
 		g_free(filename);
-		return -EBADR;
+		if (err)
+			*err = -EBADR;
+		return NULL;
 	}
 
 	folder = g_strndup(filename, strlen(filename) - 4);
 	g_free(filename);
 	if (!is_dir(folder)) {
 		g_free(folder);
-		return -ENOENT;
+		if (err)
+			*err = -ENOENT;
+		return NULL;
 	}
 
 	dummy = g_new0(struct dummy_data, 1);
@@ -482,7 +490,10 @@ int phonebook_pull(const char *name, const struct apparam_field *params,
 
 	g_idle_add_full(G_PRIORITY_DEFAULT_IDLE, read_dir, dummy, dummy_free);
 
-	return 0;
+	if (err)
+		*err = 0;
+
+	return NULL;
 }
 
 int phonebook_get_entry(const char *folder, const char *id,
diff --git a/plugins/phonebook-ebook.c b/plugins/phonebook-ebook.c
index 073ff33..9097a0b 100644
--- a/plugins/phonebook-ebook.c
+++ b/plugins/phonebook-ebook.c
@@ -408,8 +408,12 @@ done:
 	return fullname;
 }
 
-int phonebook_pull(const char *name, const struct apparam_field *params,
-					phonebook_cb cb, void *user_data)
+void phonebook_req_cancel(void *request)
+{
+}
+
+void *phonebook_pull(const char *name, const struct apparam_field *params,
+				phonebook_cb cb, void *user_data, int *err)
 {
 	struct contacts_query *data;
 	EBookQuery *query;
@@ -425,7 +429,10 @@ int phonebook_pull(const char *name, const struct apparam_field *params,
 
 	e_book_query_unref(query);
 
-	return 0;
+	if (err)
+		*err = 0;
+
+	return NULL;
 }
 
 int phonebook_get_entry(const char *folder, const char *id,
diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index 5fd0074..c8c8e54 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -1198,8 +1198,8 @@ done:
 	g_free(pending);
 }
 
-static int query_tracker(const char *query, int num_fields,
-				reply_list_foreach_t callback, void *user_data)
+static DBusPendingCall *query_tracker(const char *query, int num_fields,
+		reply_list_foreach_t callback, void *user_data,	int *err)
 {
 	struct pending_reply *pending;
 	DBusPendingCall *call;
@@ -1219,7 +1219,9 @@ static int query_tracker(const char *query, int num_fields,
 							-1) == FALSE) {
 		error("Could not send dbus message");
 		dbus_message_unref(msg);
-		return -EPERM;
+		if (err)
+			*err = -EPERM;
+		return NULL;
 	}
 
 	pending = g_new0(struct pending_reply, 1);
@@ -1228,10 +1230,12 @@ static int query_tracker(const char *query, int num_fields,
 	pending->num_fields = num_fields;
 
 	dbus_pending_call_set_notify(call, query_reply, pending, NULL);
-	dbus_pending_call_unref(call);
 	dbus_message_unref(msg);
 
-	return 0;
+	if (err)
+		*err = 0;
+
+	return call;
 }
 
 static char *iso8601_utc_to_localtime(const char *datetime)
@@ -1810,8 +1814,8 @@ void phonebook_req_cancel(void *request)
 	dbus_pending_call_unref(call);
 }
 
-int phonebook_pull(const char *name, const struct apparam_field *params,
-					phonebook_cb cb, void *user_data)
+void *phonebook_pull(const char *name, const struct apparam_field *params,
+				phonebook_cb cb, void *user_data, int *err)
 {
 	struct phonebook_data *data;
 	const char *query;
@@ -1830,15 +1834,18 @@ int phonebook_pull(const char *name, const struct apparam_field *params,
 		pull_cb = pull_contacts;
 	}
 
-	if (query == NULL)
-		return -ENOENT;
+	if (query == NULL) {
+		if (err)
+			*err = -ENOENT;
+		return NULL;
+	}
 
 	data = g_new0(struct phonebook_data, 1);
 	data->params = params;
 	data->user_data = user_data;
 	data->cb = cb;
 
-	return query_tracker(query, col_amount, pull_cb, data);
+	return query_tracker(query, col_amount, pull_cb, data, err);
 }
 
 int phonebook_get_entry(const char *folder, const char *id,
@@ -1865,7 +1872,7 @@ int phonebook_get_entry(const char *folder, const char *id,
 		query = g_strdup_printf(CONTACTS_OTHER_QUERY_FROM_URI,
 								id, id, id);
 
-	ret = query_tracker(query, PULL_QUERY_COL_AMOUNT, pull_contacts, data);
+	query_tracker(query, PULL_QUERY_COL_AMOUNT, pull_contacts, data, &ret);
 
 	g_free(query);
 
@@ -1877,6 +1884,7 @@ int phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
 {
 	struct cache_data *cache;
 	const char *query;
+	int ret;
 
 	DBG("name %s", name);
 
@@ -1889,5 +1897,7 @@ int phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
 	cache->ready_cb = ready_cb;
 	cache->user_data = user_data;
 
-	return query_tracker(query, 7, add_to_cache, cache);
+	query_tracker(query, 7, add_to_cache, cache, &ret);
+
+	return ret;
 }
diff --git a/plugins/phonebook.h b/plugins/phonebook.h
index 7ae5ccc..951b370 100644
--- a/plugins/phonebook.h
+++ b/plugins/phonebook.h
@@ -86,8 +86,8 @@ char *phonebook_set_folder(const char *current_folder,
  * entries of a given folder. The back-end MUST return only the content based
  * on the application parameters requested by the client.
  */
-int phonebook_pull(const char *name, const struct apparam_field *params,
-					phonebook_cb cb, void *user_data);
+void *phonebook_pull(const char *name, const struct apparam_field *params,
+				phonebook_cb cb, void *user_data, int *err);
 
 /*
  * Function used to retrieve a contact from the backend. Only contacts
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 2/6] Add phonebook_req_cancel to tracker
From: Dmitriy Paliy @ 2010-11-24 15:07 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Dmitriy Paliy
In-Reply-To: <1290611236-25656-1-git-send-email-dmitriy.paliy@nokia.com>

Add phonebook_req_cancel function to phonebook_tracker.c that cancels
pending request.
---
 plugins/phonebook-tracker.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index 20053f6..5fd0074 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -1799,6 +1799,17 @@ done:
 	return path;
 }
 
+void phonebook_req_cancel(void *request)
+{
+	struct DBusPendingCall *call = request;
+
+	if (!call)
+		return;
+
+	dbus_pending_call_cancel(call);
+	dbus_pending_call_unref(call);
+}
+
 int phonebook_pull(const char *name, const struct apparam_field *params,
 					phonebook_cb cb, void *user_data)
 {
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 1/6] Add phonebook_req_cancel prototype
From: Dmitriy Paliy @ 2010-11-24 15:07 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Dmitriy Paliy
In-Reply-To: <1290611236-25656-1-git-send-email-dmitriy.paliy@nokia.com>

Add phonebook_req_cancel function prototype to cancel pending request
to backend.
---
 plugins/phonebook.h |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/plugins/phonebook.h b/plugins/phonebook.h
index d7cfa46..7ae5ccc 100644
--- a/plugins/phonebook.h
+++ b/plugins/phonebook.h
@@ -107,3 +107,9 @@ int phonebook_get_entry(const char *folder, const char *id,
  */
 int phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
 			phonebook_cache_ready_cb ready_cb, void *user_data);
+
+/* 
+ * Function used to cancel pending request to backend and free resources
+ * allocated for phonebook request structure.
+ */
+void phonebook_req_cancel(void *request);
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 0/6 v2] Cancel pending phonebook request
From: Dmitriy Paliy @ 2010-11-24 15:07 UTC (permalink / raw)
  To: linux-bluetooth

Hi,

This is optimized version of previous submission
[PATCH 0/7] Cancel pending phonebook request

In these patches only pointer to pending call is returned from 
phonebook_..._open functions, while no other structure is created.
For tracker this call is DBusPendingCall and for phonebook-dummy
and phonebook-ebook it is NULL.

Also, query_tracker is modified appropriately.

BR,
Dmitriy


^ permalink raw reply

* Re: [PATCH 2/3] Bluetooth: Add initial Bluetooth Management interface callbacks
From: Andrei Emeltchenko @ 2010-11-24 14:55 UTC (permalink / raw)
  To: johan.hedberg; +Cc: linux-bluetooth, Johan Hedberg
In-Reply-To: <1290609575-28435-3-git-send-email-johan.hedberg@gmail.com>

Hi,

On Wed, Nov 24, 2010 at 4:39 PM,  <johan.hedberg@gmail.com> wrote:
> From: Johan Hedberg <johan.hedberg@nokia.com>
>
> Add initial code for handling Bluetooth Management interface messages.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
> ---
>  include/net/bluetooth/hci_core.h |    3 +
>  net/bluetooth/Makefile           |    2 +-
>  net/bluetooth/hci_sock.c         |   39 +++++++++++++--
>  net/bluetooth/mgmt.c             |   99 ++++++++++++++++++++++++++++++++++++++
>  4 files changed, 136 insertions(+), 7 deletions(-)
>  create mode 100644 net/bluetooth/mgmt.c
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index b8104af..dd1573da 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -660,6 +660,9 @@ void hci_si_event(struct hci_dev *hdev, int type, int dlen, void *data);
>  /* ----- HCI Sockets ----- */
>  void hci_send_to_sock(struct hci_dev *hdev, struct sk_buff *skb);
>
> +/* Management interface */
> +int mgmt_control(struct sock *sk, struct msghdr *msg, size_t len);
> +
>  /* HCI info for socket */
>  #define hci_pi(sk) ((struct hci_pinfo *) sk)
>
> diff --git a/net/bluetooth/Makefile b/net/bluetooth/Makefile
> index d1e433f..8b411d9 100644
> --- a/net/bluetooth/Makefile
> +++ b/net/bluetooth/Makefile
> @@ -10,4 +10,4 @@ obj-$(CONFIG_BT_BNEP) += bnep/
>  obj-$(CONFIG_BT_CMTP)  += cmtp/
>  obj-$(CONFIG_BT_HIDP)  += hidp/
>
> -bluetooth-objs := af_bluetooth.o hci_core.o hci_conn.o hci_event.o hci_sock.o hci_sysfs.o lib.o
> +bluetooth-objs := af_bluetooth.o hci_core.o hci_conn.o hci_event.o mgmt.o hci_sock.o hci_sysfs.o lib.o
> diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
> index 83acd16..730a4e8 100644
> --- a/net/bluetooth/hci_sock.c
> +++ b/net/bluetooth/hci_sock.c
> @@ -49,6 +49,8 @@
>  #include <net/bluetooth/bluetooth.h>
>  #include <net/bluetooth/hci_core.h>
>
> +static int enable_mgmt = 0;
> +
>  /* ----- HCI socket interface ----- */
>
>  static inline int hci_test_bit(int nr, void *addr)
> @@ -352,25 +354,35 @@ static int hci_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long a
>
>  static int hci_sock_bind(struct socket *sock, struct sockaddr *addr, int addr_len)
>  {
> -       struct sockaddr_hci *haddr = (struct sockaddr_hci *) addr;
> +       struct sockaddr_hci haddr;
>        struct sock *sk = sock->sk;
>        struct hci_dev *hdev = NULL;
> -       int err = 0;
> +       int len, err = 0;
>
>        BT_DBG("sock %p sk %p", sock, sk);
>
> -       if (!haddr || haddr->hci_family != AF_BLUETOOTH)
> +       if (!addr)
> +               return -EINVAL;
> +
> +       memset(&haddr, 0, sizeof(haddr));
> +       len = min_t(unsigned int, sizeof(haddr), addr_len);
> +       memcpy(&haddr, addr, len);
> +
> +       if (haddr.hci_family != AF_BLUETOOTH)
> +               return -EINVAL;
> +
> +       if (haddr.hci_channel != HCI_CHANNEL_RAW && !enable_mgmt)
>                return -EINVAL;
>
>        lock_sock(sk);
>
> -       if (hci_pi(sk)->hdev) {
> +       if (sk->sk_state == BT_BOUND || hci_pi(sk)->hdev) {
>                err = -EALREADY;
>                goto done;
>        }
>
> -       if (haddr->hci_dev != HCI_DEV_NONE) {
> -               if (!(hdev = hci_dev_get(haddr->hci_dev))) {
> +       if (haddr.hci_dev != HCI_DEV_NONE) {
> +               if (!(hdev = hci_dev_get(haddr.hci_dev))) {

doesn't checkpatch give errors here? Would be more clean like:
...
hdev = hci_dev_get(haddr.hci_dev);
if (!hdev)
...

At some point shall be fixed in the old code also

otherwise looks fine

Acked-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>

>                        err = -ENODEV;
>                        goto done;
>                }
> @@ -378,6 +390,7 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr, int addr_le
>                atomic_inc(&hdev->promisc);
>        }
>
> +       hci_pi(sk)->channel = haddr.hci_channel;
>        hci_pi(sk)->hdev = hdev;
>        sk->sk_state = BT_BOUND;
>
> @@ -499,6 +512,17 @@ static int hci_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
>
>        lock_sock(sk);
>
> +       switch (hci_pi(sk)->channel) {
> +       case HCI_CHANNEL_RAW:
> +               break;
> +       case HCI_CHANNEL_CONTROL:
> +               err = mgmt_control(sk, msg, len);
> +               goto done;
> +       default:
> +               err = -EINVAL;
> +               goto done;
> +       }
> +
>        if (!(hdev = hci_pi(sk)->hdev)) {
>                err = -EBADFD;
>                goto done;
> @@ -826,3 +850,6 @@ void __exit hci_sock_cleanup(void)
>
>        proto_unregister(&hci_sk_proto);
>  }
> +
> +module_param(enable_mgmt, bool, 0644);
> +MODULE_PARM_DESC(enable_mgmt, "Enable Management interface");
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> new file mode 100644
> index 0000000..78255f1
> --- /dev/null
> +++ b/net/bluetooth/mgmt.c
> @@ -0,0 +1,99 @@
> +/*
> +   BlueZ - Bluetooth protocol stack for Linux
> +   Copyright (C) 2010  Nokia Corporation
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License version 2 as
> +   published by the Free Software Foundation;
> +
> +   THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
> +   OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +   FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT OF THIRD PARTY RIGHTS.
> +   IN NO EVENT SHALL THE COPYRIGHT HOLDER(S) AND AUTHOR(S) BE LIABLE FOR ANY
> +   CLAIM, OR ANY SPECIAL INDIRECT OR CONSEQUENTIAL DAMAGES, OR ANY DAMAGES
> +   WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> +   ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> +   OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> +
> +   ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATENTS,
> +   COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS
> +   SOFTWARE IS DISCLAIMED.
> +*/
> +
> +/* Bluetooth HCI Management interface */
> +
> +#include <asm/uaccess.h>
> +#include <asm/unaligned.h>
> +
> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +#include <net/bluetooth/mgmt.h>
> +
> +static void cmd_status(struct sock *sk, u16 cmd, u8 status)
> +{
> +       struct sk_buff *skb;
> +       struct mgmt_hdr *hdr;
> +       struct mgmt_ev_cmd_status *ev;
> +
> +       BT_DBG("sock %p", sk);
> +
> +       skb = alloc_skb(sizeof(*hdr) + sizeof(*ev), GFP_ATOMIC);
> +       if (!skb)
> +               return;
> +
> +       hdr = (void *) skb_put(skb, sizeof(struct mgmt_hdr));
> +
> +       hdr->opcode = cpu_to_le16(MGMT_EV_CMD_STATUS);
> +       hdr->len = cpu_to_le16(3);
> +
> +       ev = (void *) skb_put(skb, sizeof(*ev));
> +       ev->status = status;
> +       put_unaligned_le16(cmd, &ev->opcode);
> +
> +       if (sock_queue_rcv_skb(sk, skb) < 0)
> +               kfree_skb(skb);
> +}
> +
> +int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
> +{
> +       unsigned char *buf;
> +       struct mgmt_hdr *hdr;
> +       u16 opcode, len;
> +       int err;
> +
> +       BT_DBG("got %zu bytes", msglen);
> +
> +       if (msglen < sizeof(*hdr))
> +               return -EINVAL;
> +
> +       buf = kmalloc(msglen, GFP_ATOMIC);
> +       if (!buf)
> +               return -ENOMEM;
> +
> +       if (memcpy_fromiovec(buf, msg->msg_iov, msglen)) {
> +               err = -EFAULT;
> +               goto done;
> +       }
> +
> +       hdr = (struct mgmt_hdr *) buf;
> +       opcode = get_unaligned_le16(&hdr->opcode);
> +       len = get_unaligned_le16(&hdr->len);
> +
> +       if (len != msglen - sizeof(struct mgmt_hdr)) {
> +               err = -EINVAL;
> +               goto done;
> +       }
> +
> +       switch (opcode) {
> +       default:
> +               BT_DBG("Unknown op %u", opcode);
> +               cmd_status(sk, opcode, 0x01);
> +               break;
> +       }
> +
> +       err = msglen;
> +
> +done:
> +       kfree(buf);
> +       return err;
> +}
> --
> 1.7.2.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply

* Re: [PATCH 3/3] Bluetooth: Make hci_send_to_sock usable for management control sockets
From: Marcel Holtmann @ 2010-11-24 14:49 UTC (permalink / raw)
  To: johan.hedberg; +Cc: linux-bluetooth, Johan Hedberg
In-Reply-To: <1290609575-28435-4-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

> In order to send data to management control sockets the function should:
> 
>   - skip checks intended for raw HCI data and stack internal events
>   - make sure RAW HCI data or stack internal events don't go to
>     management control sockets
> 
> In order to accomplish this the patch adds a new member to the bluetooth
> skb private data to flag skb's that are destined for management control
> sockets.

looks fine to me.

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel



^ permalink raw reply

* Re: [PATCH 2/3] Bluetooth: Add initial Bluetooth Management interface callbacks
From: Marcel Holtmann @ 2010-11-24 14:48 UTC (permalink / raw)
  To: johan.hedberg; +Cc: linux-bluetooth, Johan Hedberg
In-Reply-To: <1290609575-28435-3-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

> Add initial code for handling Bluetooth Management interface messages.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
> ---
>  include/net/bluetooth/hci_core.h |    3 +
>  net/bluetooth/Makefile           |    2 +-
>  net/bluetooth/hci_sock.c         |   39 +++++++++++++--
>  net/bluetooth/mgmt.c             |   99 ++++++++++++++++++++++++++++++++++++++
>  4 files changed, 136 insertions(+), 7 deletions(-)
>  create mode 100644 net/bluetooth/mgmt.c

looks good.

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel



^ permalink raw reply

* Re: [PATCH 1/3] Bluetooth: Add Bluetooth Management interface definitions
From: Marcel Holtmann @ 2010-11-24 14:47 UTC (permalink / raw)
  To: johan.hedberg; +Cc: linux-bluetooth, Johan Hedberg
In-Reply-To: <1290609575-28435-2-git-send-email-johan.hedberg@gmail.com>

Hi Johan,

> Add initial definitions for the new Bluetooth Management interface to
> the bluetooth headers.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
> ---
>  include/net/bluetooth/hci.h      |    4 +++
>  include/net/bluetooth/hci_core.h |    1 +
>  include/net/bluetooth/mgmt.h     |   46 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 51 insertions(+), 0 deletions(-)
>  create mode 100644 include/net/bluetooth/mgmt.h

looks fine to me.

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel



^ permalink raw reply

* Re: [RFC 12/20] Bluetooth: fix receiving L2CAP packets over LE
From: Vinicius Costa Gomes @ 2010-11-24 14:47 UTC (permalink / raw)
  To: Ville Tervo; +Cc: ext Vinicius Costa Gomes, linux-bluetooth
In-Reply-To: <20101124053202.GH874@null>

Hi Ville,

On 07:32 Wed 24 Nov, Ville Tervo wrote:
> Hi,
> 
> 
> On Tue, Nov 23, 2010 at 12:06:28PM -0300, ext Vinicius Costa Gomes wrote:
> > As L2CAP packets coming over LE don't have any more encapsulation,
> > other than L2CAP, we are able to process them as soon as they arrive.
> > 
> 
> I thought this patch was only fixing symphoms of broken controller? LE frames
> should have same continuation bits as ACL. See Vol 2 Part E 5.4.2.

You are right. Broken controller. A better behaved one has the right flags.

Will drop this patch.

> 
> > Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
> > ---
> >  net/bluetooth/l2cap.c |   17 +++++++++++++++--
> >  1 files changed, 15 insertions(+), 2 deletions(-)
> > 
> > diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> > index e481d6b..0d168aa 100644
> > --- a/net/bluetooth/l2cap.c
> > +++ b/net/bluetooth/l2cap.c
> > @@ -4798,17 +4798,30 @@ static int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
> >  static int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
> >  {
> >  	struct l2cap_conn *conn = hcon->l2cap_data;
> > +	struct l2cap_hdr *hdr;
> > +	int len;
> >  
> >  	if (!conn && !(conn = l2cap_conn_add(hcon, 0)))
> >  		goto drop;
> >  
> >  	BT_DBG("conn %p len %d flags 0x%x", conn, skb->len, flags);
> >  
> > +	if (hcon->type == LE_LINK) {
> > +		hdr = (struct l2cap_hdr *) skb->data;
> > +		len = __le16_to_cpu(hdr->len) + L2CAP_HDR_SIZE;
> > +
> > +		if (len == skb->len) {
> > +			/* Complete frame received */
> > +			l2cap_recv_frame(conn, skb);
> > +			return 0;
> > +		}
> > +
> > +		goto drop;
> > +	}
> > +
> >  	if (flags & ACL_START) {
> > -		struct l2cap_hdr *hdr;
> >  		struct sock *sk;
> >  		u16 cid;
> > -		int len;
> >  
> >  		if (conn->rx_len) {
> >  			BT_ERR("Unexpected start frame (len %d)", skb->len);
> > -- 
> > 1.7.3.2
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html


Cheers,
-- 
Vinicius

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox