* Re: Apple Wireless Keyboard connection issue
From: Johan Hedberg @ 2010-11-18 8:44 UTC (permalink / raw)
To: Andre Kuehne; +Cc: Gustavo F. Padovan, linux-bluetooth
In-Reply-To: <4CE48602.1000009@gmx.net>
Hi Andre,
On Thu, Nov 18, 2010, Andre Kuehne wrote:
> > git bisect good
> a352058752e541539b09e55124d411a534cc14af is the first bad commit
> commit a352058752e541539b09e55124d411a534cc14af
> Author: Johan Hedberg <johan.hedberg@nokia.com>
> Date: Thu Nov 4 04:38:35 2010 +0200
>
> Cache adapter address for quick lookup
That's actually not related to the HID issue. There's a later commit
that fixed the general initialization issue that this commit caused.
I was now able to get hold of a Bluetooth keyboard and fixed the issue.
The problem was indeed in the commit that Gustavo originally pointed
out. It's just that my original patch for it was incomplete and so
didn't properly fix the issue. Anyway, a proper fix has now been pushed
to git. Please test with that.
Johan
^ permalink raw reply
* Re: [PATCH v5] Bluetooth: btwilink driver
From: Pavan Savoy @ 2010-11-18 5:18 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: Vitaly Wool, marcel, linux-bluetooth, linux-kernel
In-Reply-To: <20101117165248.GB21729@vigoh>
On Wed, Nov 17, 2010 at 10:22 PM, Gustavo F. Padovan
<padovan@profusion.mobi> wrote:
> Hi Pavan,
>
> * Pavan Savoy <pavan_savoy@sify.com> [2010-11-17 11:13:26 +0530]:
>
>> On Wed, Nov 17, 2010 at 4:50 AM, Vitaly Wool <vitalywool@gmail.com> wrot=
e:
>> >>> + =C2=A0 =C2=A0 /* Registration with ST layer is successful,
>> >>> + =C2=A0 =C2=A0 =C2=A0* hardware is ready to accept commands from HC=
I core.
>> >>> + =C2=A0 =C2=A0 =C2=A0*/
>> >>> + =C2=A0 =C2=A0 if (test_and_set_bit(HCI_RUNNING, &hdev->flags)) {
>> >>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 clear_bit(HCI_RUNNING, &=
hdev->flags);
>> >>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 err =3D st_unregister(ST=
_BT);
>> >>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (err)
>> >>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 BT_ERR("st_unregister() failed with error %d", err);
>> >>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 hst->st_write =3D NULL;
>> >>> + =C2=A0 =C2=A0 }
>> >>
>> >>
>> >> What are you trying to do here? test_and_set_bit() result doesn't say
>> >> nothing about error and you shall put test_and_set_bit should be in t=
he
>> >> beginning, to know if your device is already opened or not and then
>> >> clear_bit if some error ocurrs during the function.
>> >>
>> >
>> > Yeap, this piece of code beats me is well. Why is it an error if this
>> > bit wasn't already set?
>>
>> Vitaly, Gustavo,
>>
>> I suppose I never understood HCI_RUNNING flag that way, as in an error
>> check mechanism to avoid multiple hci0 ups.
>>
>> What I understood was that HCI_RUNNING suggested as to when hci0 was
>> ready to be used. With this understanding, I wanted to make sure I
>> downloaded the firmware for the chip before I proclaim to the world
>> that the hci0 is ready to be used, as in HCI_RUNNING.
>>
>> For example, I didn't want my _send_frame to be called before I did
>> the firmware download - since firmware download takes time - 45kb
>> send/wait commands :(
>>
>> But I suppose I now understand - What I would rather do is test_bit in
>> the beginning of function and do a set_bit at the end of function -
>> does this make sense ?
>
> It does, but does it as test_and_set and then clear if error as we do in
> other drivers.
Ok, I understand, will do it this way.
However, still I am not too convinced - I honestly don't want to set
HCI_RUNNING before firmware download required for WiLink happens - and
this happens inside the st_register function here.
So the question again, How can I ensure _send_frame is not called when
firmware download is in progress - one of the major reasons why I
delayed the setting of HCI_RUNNING.
As mentioned before I will go ahead and create the patch - But would
still like to have an answer to this.
> Gustavo F. Padovan
> http://profusion.mobi
>
^ permalink raw reply
* Re: Apple Wireless Keyboard connection issue
From: Andre Kuehne @ 2010-11-18 1:48 UTC (permalink / raw)
To: Gustavo F. Padovan, linux-bluetooth
In-Reply-To: <20101117222751.GA20735@jh-x301>
[-- Attachment #1: Type: text/plain, Size: 1673 bytes --]
On 11/17/2010 11:27 PM, Johan Hedberg wrote:
> Hi,
>
> On Thu, Nov 18, 2010, Andre Kuehne wrote:
>> On 11/17/2010 10:05 AM, Johan Hedberg wrote:
>>> On Tue, Nov 16, 2010, Gustavo F. Padovan wrote:
>>>> * "André Kühne"<andre.kuehne@gmx.net> [2010-11-16 22:27:01 +0100]:
>>>>> I noticed the following on my system: After upgrading to bluez-4.79
>>>>> connecting my Apple Wireless Keyboard does not work anymore. With
>>>>> bluez-4.77 the connection works just fine.
>>>>
>>>> My Microsoft keyboard is not working too. That is probably due to commit
>>>> abe7cd44124a from Johan. It should be fixed soon.
>>>
>>> The only real difference that patch makes is the reuse of the HCI socket
>>> inside hciops.c. Maybe that screws up the event filters somehow or
>>> something similar. I don't have a keyboard to verify a fix, but could
>>> you try the attached patch and see if it helps?
>>>
>> I installed
>>
>> http://www.kernel.org/pub/linux/bluetooth/bluez-4.79.tar.gz + hciops_encrypt.patch
>>
>> but the patch does not work for me. For verification I downloaded
>>
>> http://www.kernel.org/pub/linux/bluetooth/bluez-4.77.tar.gz
>>
>> and compiled/installed it the same way (without the patch) and the connection works.
>>
>> I did the same using bluez-4.78 with the result that both my keyboard and mouse failed to connect.
>
> Ok, so then it seems like abe7cd44124a might not be to blame. Could you
> do a proper git bisect so we can figure out exactly at which point this
> broke. abe7cd44124a is really the only commit touching HID code since
> 4.77 so this is starting to get a bit strange.
>
> Johan
>
Please find attached my bisect results.
Best regards
Andre
[-- Attachment #2: bluez-bisect.txt --]
[-- Type: text/plain, Size: 1540 bytes --]
> git bisect good
a352058752e541539b09e55124d411a534cc14af is the first bad commit
commit a352058752e541539b09e55124d411a534cc14af
Author: Johan Hedberg <johan.hedberg@nokia.com>
Date: Thu Nov 4 04:38:35 2010 +0200
Cache adapter address for quick lookup
:040000 040000 b7b14eb9f69c0b74ebda87b2fa40a4574f08be44 b96739e2d58fcc5a5c6654431ba49a36ee623932 M plugins
> git bisect log
git bisect start
# good: [b079cca768419d11e011cc5a7d59bc802226e633] Release 4.77
git bisect good b079cca768419d11e011cc5a7d59bc802226e633
# bad: [018c5b71748c178dcd2ffb0164c1d975788a2acc] Release 4.78
git bisect bad 018c5b71748c178dcd2ffb0164c1d975788a2acc
# good: [d5d0fa3be8f7a20d128dcbaf8fc529c38bc52395] Optimize device disconnect callback processing
git bisect good d5d0fa3be8f7a20d128dcbaf8fc529c38bc52395
# good: [b67a8a42e6ea9c2d2b0877ac3832de75ec4e00e5] Fix bdaddr naming consistency
git bisect good b67a8a42e6ea9c2d2b0877ac3832de75ec4e00e5
# bad: [5295b6a0ab0ad67a27926273a3fbf61f02663219] Fix not ignoring case of uuid given to RegisterEndpoint
git bisect bad 5295b6a0ab0ad67a27926273a3fbf61f02663219
# good: [db5266f0afedc33e2fd6fc3601c16498865f746d] Remove redundant tracking of ignored adapters
git bisect good db5266f0afedc33e2fd6fc3601c16498865f746d
# bad: [a352058752e541539b09e55124d411a534cc14af] Cache adapter address for quick lookup
git bisect bad a352058752e541539b09e55124d411a534cc14af
# good: [b289e54d53c332a51be483c1660a42c267991dd3] Remove redundant hci_devinfo call
git bisect good b289e54d53c332a51be483c1660a42c267991dd3
^ permalink raw reply
* [PATCH 2/2] Bluetooth: Get ride of __rfcomm_get_sock_by_channel()
From: Gustavo F. Padovan @ 2010-11-17 23:22 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1290036176-4022-1-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 aec505f..0207bd6 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -140,11 +140,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;
@@ -159,19 +161,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)
@@ -945,6 +938,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 1/2] Bluetooth: Get ride of __l2cap_get_sock_by_psm()
From: Gustavo F. Padovan @ 2010-11-17 23:22 UTC (permalink / raw)
To: linux-bluetooth
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 18a802c..12b4aa2 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -751,11 +751,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;
@@ -770,20 +772,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)
@@ -2934,6 +2926,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)) {
@@ -4464,6 +4458,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
* Re: [PATCH 1/7] Bluetooth: Hold the lock inside l2cap_get_sock_by_addr()
From: Gustavo F. Padovan @ 2010-11-17 23:17 UTC (permalink / raw)
To: Ville Tervo; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <20101117231141.GB32261@vigoh>
* Gustavo F. Padovan <padovan@profusion.mobi> [2010-11-17 21:11:41 -0200]:
> * Gustavo F. Padovan <padovan@profusion.mobi> [2010-11-05 10:37:11 -0400]:
>
> > Hi Ville,
> >
> > * Ville Tervo <ville.tervo@nokia.com> [2010-11-05 15:49:35 +0200]:
> >
> > > Hi Gustavo,
> > >
> > > On Tue, Nov 02, 2010 at 04:03:12PM +0100, ext Gustavo F. Padovan wrote:
> > > > 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);
> > >
> > > Code is only reading so read_lock_bh would be enough?
> >
> > Sure, I didn't looked to that, I just keept the same code that we were
> > using before. I'll fix it.
>
> I figured out that we need write_lock_bh() here, because set the psm and
> sport is like a new element to the list. l2cap_get_sock_by_addr()
> searches for either psm or sport.
>
> I'm also dropping the option to use RCU on the bt_sk_list(), It does not
> fit on our case. We can't have anyone writing the list while we are
> reading it.
That said, only patch 4 and 5 are still valid (I'll resend them), and 6 is
so trivial that I put it upstream already.
--
Gustavo F. Padovan
http://profusion.mobi
^ permalink raw reply
* Re: [PATCH 1/7] Bluetooth: Hold the lock inside l2cap_get_sock_by_addr()
From: Gustavo F. Padovan @ 2010-11-17 23:11 UTC (permalink / raw)
To: Ville Tervo; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <20101105143711.GB9116@vigoh>
* Gustavo F. Padovan <padovan@profusion.mobi> [2010-11-05 10:37:11 -0400]:
> Hi Ville,
>
> * Ville Tervo <ville.tervo@nokia.com> [2010-11-05 15:49:35 +0200]:
>
> > Hi Gustavo,
> >
> > On Tue, Nov 02, 2010 at 04:03:12PM +0100, ext Gustavo F. Padovan wrote:
> > > 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);
> >
> > Code is only reading so read_lock_bh would be enough?
>
> Sure, I didn't looked to that, I just keept the same code that we were
> using before. I'll fix it.
I figured out that we need write_lock_bh() here, because set the psm and
sport is like a new element to the list. l2cap_get_sock_by_addr()
searches for either psm or sport.
I'm also dropping the option to use RCU on the bt_sk_list(), It does not
fit on our case. We can't have anyone writing the list while we are
reading it.
--
Gustavo F. Padovan
http://profusion.mobi
^ permalink raw reply
* Re: Apple Wireless Keyboard connection issue
From: Andre Kuehne @ 2010-11-17 23:04 UTC (permalink / raw)
To: Gustavo F. Padovan, linux-bluetooth
In-Reply-To: <20101117090534.GA11494@jh-x301>
Hi Johan
On 11/17/2010 10:05 AM, Johan Hedberg wrote:
> Hi Gustavo,
>
> On Tue, Nov 16, 2010, Gustavo F. Padovan wrote:
>> * "André Kühne"<andre.kuehne@gmx.net> [2010-11-16 22:27:01 +0100]:
>>> I noticed the following on my system: After upgrading to bluez-4.79
>>> connecting my Apple Wireless Keyboard does not work anymore. With
>>> bluez-4.77 the connection works just fine.
>>
>> My Microsoft keyboard is not working too. That is probably due to commit
>> abe7cd44124a from Johan. It should be fixed soon.
>
> The only real difference that patch makes is the reuse of the HCI socket
> inside hciops.c. Maybe that screws up the event filters somehow or
> something similar. I don't have a keyboard to verify a fix, but could
> you try the attached patch and see if it helps?
>
I installed
http://www.kernel.org/pub/linux/bluetooth/bluez-4.79.tar.gz + hciops_encrypt.patch
but the patch does not work for me. For verification I downloaded
http://www.kernel.org/pub/linux/bluetooth/bluez-4.77.tar.gz
and compiled/installed it the same way (without the patch) and the connection works.
I did the same using bluez-4.78 with the result that both my keyboard and mouse failed to connect.
Best regards
Andre
^ permalink raw reply
* Re: [PATCH 3/3] Bluetooth: Automate remote name requests
From: Gustavo F. Padovan @ 2010-11-17 23:01 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <20101117223057.GB20735@jh-x301>
* Johan Hedberg <johan.hedberg@gmail.com> [2010-11-18 00:30:57 +0200]:
> Hi Gustavo,
>
> On Tue, Nov 16, 2010, Gustavo F. Padovan wrote:
> > > -static int request_outgoing_auth(struct hci_dev *hdev, bdaddr_t *bdaddr)
> > > +static int outgoing_auth_needed(struct hci_dev *hdev, bdaddr_t *bdaddr)
> >
> > Can you add a hci_ in the beginning of your functions, just to keep the
> > coherency with the rest of the code.
>
> Sure. I guess I'm too used to the userspace convention where strict
> namespacing is only used for exported (non-static) functions.
>
> > I'm not happy with have to lookup the hci_conn twice when we can do that
> > once here. I've noted that always outgoing_auth_needed() returns 1 you do
> > a request_auth, and always it returns 0 you don't, so I think we can
> > embed request_auth() inside outgoing_auth_needed() as it was in patch
> > 2/3 and the give a better name to outgoing_auth_needed() which you
> > reflect the new behavior.
>
> I don't think the check and request should be in the same function since
> there are two places that need the check but not the request.
In that case such function will return after the check not doing any
request.
--
Gustavo F. Padovan
http://profusion.mobi
^ permalink raw reply
* Re: [PATCH 3/3] Bluetooth: Automate remote name requests
From: Johan Hedberg @ 2010-11-17 22:30 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: linux-bluetooth
In-Reply-To: <20101116220907.GA30115@vigoh>
Hi Gustavo,
On Tue, Nov 16, 2010, Gustavo F. Padovan wrote:
> > -static int request_outgoing_auth(struct hci_dev *hdev, bdaddr_t *bdaddr)
> > +static int outgoing_auth_needed(struct hci_dev *hdev, bdaddr_t *bdaddr)
>
> Can you add a hci_ in the beginning of your functions, just to keep the
> coherency with the rest of the code.
Sure. I guess I'm too used to the userspace convention where strict
namespacing is only used for exported (non-static) functions.
> I'm not happy with have to lookup the hci_conn twice when we can do that
> once here. I've noted that always outgoing_auth_needed() returns 1 you do
> a request_auth, and always it returns 0 you don't, so I think we can
> embed request_auth() inside outgoing_auth_needed() as it was in patch
> 2/3 and the give a better name to outgoing_auth_needed() which you
> reflect the new behavior.
I don't think the check and request should be in the same function since
there are two places that need the check but not the request. What I can
do though (or actually what I already did) is move the hci_conn lookup
outside of the functions so it's not done multiple times. Will send new
patches in a minute.
Johan
^ permalink raw reply
* Re: Apple Wireless Keyboard connection issue
From: Johan Hedberg @ 2010-11-17 22:27 UTC (permalink / raw)
To: Andre Kuehne; +Cc: Gustavo F. Padovan, linux-bluetooth
In-Reply-To: <4CE45F63.7030902@gmx.net>
Hi,
On Thu, Nov 18, 2010, Andre Kuehne wrote:
> On 11/17/2010 10:05 AM, Johan Hedberg wrote:
> >On Tue, Nov 16, 2010, Gustavo F. Padovan wrote:
> >>* "André Kühne"<andre.kuehne@gmx.net> [2010-11-16 22:27:01 +0100]:
> >>>I noticed the following on my system: After upgrading to bluez-4.79
> >>>connecting my Apple Wireless Keyboard does not work anymore. With
> >>>bluez-4.77 the connection works just fine.
> >>
> >>My Microsoft keyboard is not working too. That is probably due to commit
> >>abe7cd44124a from Johan. It should be fixed soon.
> >
> >The only real difference that patch makes is the reuse of the HCI socket
> >inside hciops.c. Maybe that screws up the event filters somehow or
> >something similar. I don't have a keyboard to verify a fix, but could
> >you try the attached patch and see if it helps?
> >
> I installed
>
> http://www.kernel.org/pub/linux/bluetooth/bluez-4.79.tar.gz + hciops_encrypt.patch
>
> but the patch does not work for me. For verification I downloaded
>
> http://www.kernel.org/pub/linux/bluetooth/bluez-4.77.tar.gz
>
> and compiled/installed it the same way (without the patch) and the connection works.
>
> I did the same using bluez-4.78 with the result that both my keyboard and mouse failed to connect.
Ok, so then it seems like abe7cd44124a might not be to blame. Could you
do a proper git bisect so we can figure out exactly at which point this
broke. abe7cd44124a is really the only commit touching HID code since
4.77 so this is starting to get a bit strange.
Johan
^ permalink raw reply
* [PATCH 6/6] Implement Discover Primary Service by Service UUID in the gatttool
From: Claudio Takahasi @ 2010-11-17 18:09 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1290017386-23969-1-git-send-email-claudio.takahasi@openbossa.org>
Implement only the first interaction of the discovery procedure. If the
response doesn't fit in the MTU, "start" and "end" options can be used
to discover the handles ranges of the remaining primary service instances.
UUID16 and UUID128 are supported in the uuid option.
Usage example:
$gatttool -i hcix -b xx:xx:xx:xx:xx:xx --uuid=1801 --primary
---
Makefile.am | 3 +-
attrib/gatttool.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++----
2 files changed, 62 insertions(+), 6 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index da308a7..1d83943 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -177,7 +177,8 @@ if ATTRIBPLUGIN
bin_PROGRAMS += attrib/gatttool
attrib_gatttool_SOURCES = attrib/gatttool.c attrib/att.c attrib/gatt.c \
- attrib/gattrib.c btio/btio.c
+ attrib/gattrib.c btio/btio.c \
+ src/glib-helper.h src/glib-helper.c
attrib_gatttool_LDADD = lib/libbluetooth.la @GLIB_LIBS@
builtin_modules += attrib
diff --git a/attrib/gatttool.c b/attrib/gatttool.c
index ac9745d..61afd52 100644
--- a/attrib/gatttool.c
+++ b/attrib/gatttool.c
@@ -41,6 +41,7 @@
#include "att.h"
#include "btio.h"
#include "gattrib.h"
+#include "glib-helper.h"
#include "gatt.h"
/* Minimum MTU for L2CAP connections over BR/EDR */
@@ -49,6 +50,7 @@
static gchar *opt_src = NULL;
static gchar *opt_dst = NULL;
static gchar *opt_value = NULL;
+static uuid_t *opt_uuid = NULL;
static int opt_start = 0x0001;
static int opt_end = 0xffff;
static int opt_handle = -1;
@@ -135,7 +137,7 @@ static GIOChannel *do_connect(gboolean le)
return chan;
}
-static void primary_cb(guint8 status, const guint8 *pdu, guint16 plen,
+static void primary_all_cb(guint8 status, const guint8 *pdu, guint16 plen,
gpointer user_data)
{
GAttrib *attrib = user_data;
@@ -191,9 +193,9 @@ static void primary_cb(guint8 status, const guint8 *pdu, guint16 plen,
* Read by Group Type Request until Error Response is received and
* the Error Code is set to Attribute Not Found.
*/
- gatt_discover_primary(attrib, end + 1, opt_end, NULL, primary_cb,
- attrib);
+ gatt_discover_primary(attrib, end + 1, opt_end, NULL, primary_all_cb,
+ attrib);
return;
done:
@@ -201,6 +203,36 @@ done:
g_main_loop_quit(event_loop);
}
+static void primary_by_uuid_cb(guint8 status, const guint8 *pdu, guint16 plen,
+ gpointer user_data)
+{
+ GSList *ranges, *l;
+
+ if (status != 0) {
+ g_printerr("Discover primary services by UUID failed: %s\n",
+ att_ecode2str(status));
+ goto done;
+ }
+
+ ranges = dec_find_by_type_resp(pdu, plen);
+ if (ranges == NULL) {
+ g_printerr("Protocol error!\n");
+ goto done;
+ }
+
+ for (l = ranges; l; l = l->next) {
+ struct range *range = l->data;
+ g_print("Starting handle: %04x Ending handle: %04x\n",
+ range->start, range->end);
+ }
+
+ g_slist_foreach(ranges, (GFunc) g_free, NULL);
+ g_slist_free(ranges);
+
+done:
+ g_main_loop_quit(event_loop);
+}
+
static void events_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
{
GAttrib *attrib = user_data;
@@ -251,8 +283,12 @@ static gboolean primary(gpointer user_data)
{
GAttrib *attrib = user_data;
- gatt_discover_primary(attrib, opt_start, opt_end, NULL, primary_cb,
- attrib);
+ if (opt_uuid)
+ gatt_discover_primary(attrib, opt_start, opt_end, opt_uuid,
+ primary_by_uuid_cb, attrib);
+ else
+ gatt_discover_primary(attrib, opt_start, opt_end, NULL,
+ primary_all_cb, attrib);
return FALSE;
}
@@ -477,11 +513,29 @@ static gboolean characteristics_desc(gpointer user_data)
return FALSE;
}
+static gboolean parse_uuid(const char *key, const char *value,
+ gpointer user_data, GError **error)
+{
+ if (!value)
+ return FALSE;
+
+ opt_uuid = g_try_malloc(sizeof(uuid_t));
+ if (opt_uuid == NULL)
+ return FALSE;
+
+ if (bt_string2uuid(opt_uuid, value) < 0)
+ return FALSE;
+
+ return TRUE;
+}
+
static GOptionEntry primary_char_options[] = {
{ "start", 's' , 0, G_OPTION_ARG_INT, &opt_start,
"Starting handle(optional)", "0x0001" },
{ "end", 'e' , 0, G_OPTION_ARG_INT, &opt_end,
"Ending handle(optional)", "0xffff" },
+ { "uuid", 'u', G_OPTION_FLAG_OPTIONAL_ARG, G_OPTION_ARG_CALLBACK,
+ parse_uuid, "UUID16 or UUID128(optional)", "0x1801"},
{ NULL },
};
@@ -610,6 +664,7 @@ done:
g_option_context_free(context);
g_free(opt_src);
g_free(opt_dst);
+ g_free(opt_uuid);
if (got_error)
exit(EXIT_FAILURE);
--
1.7.3.2
^ permalink raw reply related
* [PATCH 5/6] Add an extra parameter in the discovery primary to specify the UUID
From: Claudio Takahasi @ 2010-11-17 18:09 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1290017386-23969-1-git-send-email-claudio.takahasi@openbossa.org>
Extends discover primary function to perform discover by UUID. UUID
parameter defines which procedure will be executed: Discover All
Primary Services or Discover Primary Service by Service UUID.
---
attrib/client.c | 8 ++++----
attrib/gatt.c | 37 ++++++++++++++++++++++++++++++-------
attrib/gatt.h | 4 ++--
attrib/gatttool.c | 6 ++++--
4 files changed, 40 insertions(+), 15 deletions(-)
diff --git a/attrib/client.c b/attrib/client.c
index 955e623..a851a74 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -363,8 +363,8 @@ static void connect_cb(GIOChannel *chan, GError *gerr, gpointer user_data)
return;
}
- atid = gatt_discover_primary(gatt->attrib, 0x0001, 0xffff, primary_cb,
- gatt);
+ atid = gatt_discover_primary(gatt->attrib, 0x0001, 0xffff, NULL,
+ primary_cb, gatt);
if (atid == 0)
goto fail;
@@ -1311,8 +1311,8 @@ static void primary_cb(guint8 status, const guint8 *pdu, guint16 plen,
* the Error Code is set to Attribute Not Found.
*/
gatt->attrib = g_attrib_ref(gatt->attrib);
- gatt->atid = gatt_discover_primary(gatt->attrib,
- end + 1, 0xffff, primary_cb, gatt);
+ gatt->atid = gatt_discover_primary(gatt->attrib, end + 1, 0xffff, NULL,
+ primary_cb, gatt);
done:
g_attrib_unref(gatt->attrib);
}
diff --git a/attrib/gatt.c b/attrib/gatt.c
index 24ec990..2c87daf 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -31,21 +31,44 @@
#include "gattrib.h"
#include "gatt.h"
-guint gatt_discover_primary(GAttrib *attrib, uint16_t start,
- uint16_t end, GAttribResultFunc func, gpointer user_data)
+guint gatt_discover_primary(GAttrib *attrib, uint16_t start, uint16_t end,
+ uuid_t *uuid, GAttribResultFunc func, gpointer user_data)
{
uint8_t pdu[ATT_DEFAULT_MTU];
- uuid_t uuid;
+ uuid_t prim;
guint16 plen;
+ uint8_t op;
+
+ sdp_uuid16_create(&prim, GATT_PRIM_SVC_UUID);
+
+ if (uuid == NULL) {
+
+ /* Discover all primary services */
+ op = ATT_OP_READ_BY_GROUP_REQ;
+ plen = enc_read_by_grp_req(start, end, &prim, pdu, sizeof(pdu));
+ } else {
+ const void *value;
+ int vlen;
- sdp_uuid16_create(&uuid, GATT_PRIM_SVC_UUID);
+ /* Discover primary service by service UUID */
+ op = ATT_OP_FIND_BY_TYPE_REQ;
+
+ if (uuid->type == SDP_UUID16) {
+ value = &uuid->value.uuid16;
+ vlen = sizeof(uuid->value.uuid16);
+ } else {
+ value = &uuid->value.uuid128;
+ vlen = sizeof(uuid->value.uuid128);
+ }
+
+ plen = enc_find_by_type_req(start, end, &prim, value, vlen,
+ pdu, sizeof(pdu));
+ }
- plen = enc_read_by_grp_req(start, end, &uuid, pdu, sizeof(pdu));
if (plen == 0)
return 0;
- return g_attrib_send(attrib, ATT_OP_READ_BY_GROUP_REQ,
- pdu, plen, func, user_data, NULL);
+ return g_attrib_send(attrib, op, pdu, plen, func, user_data, NULL);
}
guint gatt_discover_char(GAttrib *attrib, uint16_t start, uint16_t end,
diff --git a/attrib/gatt.h b/attrib/gatt.h
index f1599c2..4e7d88b 100644
--- a/attrib/gatt.h
+++ b/attrib/gatt.h
@@ -24,8 +24,8 @@
#define GATT_CID 4
-guint gatt_discover_primary(GAttrib *attrib, uint16_t start,
- uint16_t end, GAttribResultFunc func, gpointer user_data);
+guint gatt_discover_primary(GAttrib *attrib, uint16_t start, uint16_t end,
+ uuid_t *uuid, GAttribResultFunc func, gpointer user_data);
guint gatt_discover_char(GAttrib *attrib, uint16_t start, uint16_t end,
GAttribResultFunc func, gpointer user_data);
diff --git a/attrib/gatttool.c b/attrib/gatttool.c
index b9f5138..ac9745d 100644
--- a/attrib/gatttool.c
+++ b/attrib/gatttool.c
@@ -191,7 +191,8 @@ static void primary_cb(guint8 status, const guint8 *pdu, guint16 plen,
* Read by Group Type Request until Error Response is received and
* the Error Code is set to Attribute Not Found.
*/
- gatt_discover_primary(attrib, end + 1, opt_end, primary_cb, attrib);
+ gatt_discover_primary(attrib, end + 1, opt_end, NULL, primary_cb,
+ attrib);
return;
@@ -250,7 +251,8 @@ static gboolean primary(gpointer user_data)
{
GAttrib *attrib = user_data;
- gatt_discover_primary(attrib, opt_start, opt_end, primary_cb, attrib);
+ gatt_discover_primary(attrib, opt_start, opt_end, NULL, primary_cb,
+ attrib);
return FALSE;
}
--
1.7.3.2
^ permalink raw reply related
* [PATCH 4/6] Extend bt_string2uuid to convert hex strings to UUID16
From: Claudio Takahasi @ 2010-11-17 18:09 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1290017386-23969-1-git-send-email-claudio.takahasi@openbossa.org>
Convert four or six(0x) digits length hexadecimal strings to UUID16.
---
src/glib-helper.c | 22 ++++++++++++++++++++--
1 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/src/glib-helper.c b/src/glib-helper.c
index 9d76626..232fc71 100644
--- a/src/glib-helper.c
+++ b/src/glib-helper.c
@@ -466,6 +466,24 @@ static inline gboolean is_uuid128(const char *string)
string[23] == '-');
}
+static int string2uuid16(uuid_t *uuid, const char *string)
+{
+ int length = strlen(string);
+ char *endptr = NULL;
+ uint16_t u16;
+
+ if (length != 4 && length != 6)
+ return -EINVAL;
+
+ u16 = strtol(string, &endptr, 16);
+ if (endptr && *endptr == '\0') {
+ sdp_uuid16_create(uuid, u16);
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
char *bt_name2string(const char *pattern)
{
uuid_t uuid;
@@ -529,9 +547,9 @@ int bt_string2uuid(uuid_t *uuid, const char *string)
sdp_uuid16_create(uuid, class);
return 0;
}
- }
- return -1;
+ return string2uuid16(uuid, string);
+ }
}
gchar *bt_list2string(GSList *list)
--
1.7.3.2
^ permalink raw reply related
* [PATCH 3/6] Implement Find by Type Value Request in the atttribute server
From: Claudio Takahasi @ 2010-11-17 18:09 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1290017386-23969-1-git-send-email-claudio.takahasi@openbossa.org>
GATT Discover Primary Service by Service UUID sub-procedure is based
on ATT Find By Type Value Request/Response.
Implement an extra verification for broken requests: "Ending Handle"
different than 0xFFFF. The Group End Handle may be greater than the
"Ending Handle" in the Find By Type Value Request. Forces the "Ending
Handle" in the response to 0xFFFF to avoid another request from the
clients. 0xFFFF means that the sub-procedure is complete.
---
src/attrib-server.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 80 insertions(+), 0 deletions(-)
diff --git a/src/attrib-server.c b/src/attrib-server.c
index 375b731..05a0fb7 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -383,6 +383,76 @@ static int find_info(uint16_t start, uint16_t end, uint8_t *pdu, int len)
return length;
}
+static int find_by_type(uint16_t start, uint16_t end, uuid_t *uuid,
+ const uint8_t *value, int vlen, uint8_t *opdu, int mtu)
+{
+ struct attribute *a;
+ struct range *range;
+ GSList *l, *matches;
+ int len;
+
+ if (start > end || start == 0x0000)
+ return enc_error_resp(ATT_OP_FIND_BY_TYPE_REQ, start,
+ ATT_ECODE_INVALID_HANDLE, opdu, mtu);
+
+ /* Searching first requested handle number */
+ for (l = database, matches = NULL, range = NULL; l; l = l->next) {
+ a = l->data;
+
+ if (a->handle < start)
+ continue;
+
+ if (a->handle > end)
+ break;
+
+ /* Primary service? Attribute value matches? */
+ if ((sdp_uuid_cmp(&a->uuid, uuid) == 0) && (a->len == vlen) &&
+ (memcmp(a->data, value, vlen) == 0)) {
+
+ range = g_new0(struct range, 1);
+ range->start = a->handle;
+
+ matches = g_slist_append(matches, range);
+ } else if (range) {
+ /*
+ * Update the last found handle or reset the pointer
+ * to track that a new group started: Primary or
+ * Secondary service.
+ */
+ if (sdp_uuid_cmp(&a->uuid, &prim_uuid) == 0 ||
+ sdp_uuid_cmp(&a->uuid, &snd_uuid) == 0)
+ range = NULL;
+ else
+ range->end = a->handle;
+ }
+ }
+
+ if (range) {
+ if (l == NULL) {
+ /* Avoids another iteration */
+ range->end = 0xFFFF;
+ } else if (range->end == 0) {
+ /*
+ * Broken requests: requested End Handle is not 0xFFFF.
+ * Given handle is in the middle of a service definition.
+ */
+ matches = g_slist_remove(matches, range);
+ g_free(range);
+ }
+ }
+
+ if (matches == NULL)
+ return enc_error_resp(ATT_OP_FIND_BY_TYPE_REQ, start,
+ ATT_ECODE_ATTR_NOT_FOUND, opdu, mtu);
+
+ len = enc_find_by_type_resp(matches, opdu, mtu);
+
+ g_slist_foreach(matches, (GFunc) g_free, NULL);
+ g_slist_free(matches);
+
+ return len;
+}
+
static int handle_cmp(gconstpointer a, gconstpointer b)
{
const struct attribute *attrib = a;
@@ -522,6 +592,16 @@ static void channel_handler(const uint8_t *ipdu, uint16_t len,
write_value(start, value, vlen);
return;
case ATT_OP_FIND_BY_TYPE_REQ:
+ length = dec_find_by_type_req(ipdu, len, &start, &end,
+ &uuid, value, &vlen);
+ if (length == 0) {
+ status = ATT_ECODE_INVALID_PDU;
+ goto done;
+ }
+
+ length = find_by_type(start, end, &uuid, value, vlen,
+ opdu, channel->mtu);
+ break;
case ATT_OP_READ_BLOB_REQ:
case ATT_OP_READ_MULTI_REQ:
case ATT_OP_PREP_WRITE_REQ:
--
1.7.3.2
^ permalink raw reply related
* [PATCH 2/6] Add Find By Type Value Response encoding/decoding functions
From: Claudio Takahasi @ 2010-11-17 18:09 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1290017386-23969-1-git-send-email-claudio.takahasi@openbossa.org>
Find by type operation is used by Discover Primary Service by Service
UUID. Find By Type Value Response shall contain one or more group handles.
---
attrib/att.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
attrib/att.h | 7 +++++++
2 files changed, 53 insertions(+), 0 deletions(-)
diff --git a/attrib/att.c b/attrib/att.c
index 6c889f2..60fc74b 100644
--- a/attrib/att.c
+++ b/attrib/att.c
@@ -30,6 +30,8 @@
#include <bluetooth/sdp.h>
#include <bluetooth/sdp_lib.h>
+#include <glib.h>
+
#include "att.h"
const char *att_ecode2str(uint8_t status)
@@ -271,6 +273,50 @@ uint16_t dec_find_by_type_req(const uint8_t *pdu, int len, uint16_t *start,
return len;
}
+uint16_t enc_find_by_type_resp(GSList *matches, uint8_t *pdu, int len)
+{
+ GSList *l;
+ uint16_t offset;
+
+ if (pdu == NULL || len < 5)
+ return 0;
+
+ pdu[0] = ATT_OP_FIND_BY_TYPE_RESP;
+
+ for (l = matches, offset = 1; l && len >= (offset + 4);
+ l = l->next, offset += 4) {
+ struct range *range = l->data;
+
+ att_put_u16(range->start, &pdu[offset]);
+ att_put_u16(range->end, &pdu[offset + 2]);
+ }
+
+ return offset;
+}
+
+GSList *dec_find_by_type_resp(const uint8_t *pdu, int len)
+{
+ struct range *range;
+ GSList *matches;
+ int offset;
+
+ if (pdu == NULL || len < 5)
+ return NULL;
+
+ if (pdu[0] != ATT_OP_FIND_BY_TYPE_RESP)
+ return NULL;
+
+ for (offset = 1, matches = NULL; len >= (offset + 4); offset += 4) {
+ range = malloc(sizeof(struct range));
+ range->start = att_get_u16(&pdu[offset]);
+ range->end = att_get_u16(&pdu[offset + 2]);
+
+ matches = g_slist_append(matches, range);
+ }
+
+ return matches;
+}
+
uint16_t enc_read_by_type_req(uint16_t start, uint16_t end, uuid_t *uuid,
uint8_t *pdu, int len)
{
diff --git a/attrib/att.h b/attrib/att.h
index 9de338d..c7260ff 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -122,6 +122,11 @@ struct att_data_list {
uint8_t **data;
};
+struct range {
+ uint16_t start;
+ uint16_t end;
+};
+
/* These functions do byte conversion */
static inline uint8_t att_get_u8(const void *ptr)
{
@@ -168,6 +173,8 @@ uint16_t enc_find_by_type_req(uint16_t start, uint16_t end, uuid_t *uuid,
const uint8_t *value, int vlen, uint8_t *pdu, int len);
uint16_t dec_find_by_type_req(const uint8_t *pdu, int len, uint16_t *start,
uint16_t *end, uuid_t *uuid, uint8_t *value, int *vlen);
+uint16_t enc_find_by_type_resp(GSList *ranges, uint8_t *pdu, int len);
+GSList *dec_find_by_type_resp(const uint8_t *pdu, int len);
struct att_data_list *dec_read_by_grp_resp(const uint8_t *pdu, int len);
uint16_t enc_read_by_type_req(uint16_t start, uint16_t end, uuid_t *uuid,
uint8_t *pdu, int len);
--
1.7.3.2
^ permalink raw reply related
* [PATCH 1/6] Implement Find by Type request encode/decoding
From: Claudio Takahasi @ 2010-11-17 18:09 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Bruna Moreira
From: Bruna Moreira <bruna.moreira@openbossa.org>
---
attrib/att.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
attrib/att.h | 4 ++-
2 files changed, 73 insertions(+), 3 deletions(-)
diff --git a/attrib/att.c b/attrib/att.c
index fe41d0e..6c889f2 100644
--- a/attrib/att.c
+++ b/attrib/att.c
@@ -198,9 +198,77 @@ struct att_data_list *dec_read_by_grp_resp(const uint8_t *pdu, int len)
}
uint16_t enc_find_by_type_req(uint16_t start, uint16_t end, uuid_t *uuid,
- uint8_t *pdu, int len)
+ const uint8_t *value, int vlen, uint8_t *pdu, int len)
+{
+ uint16_t min_len = sizeof(pdu[0]) + sizeof(start) + sizeof(end) +
+ sizeof(uint16_t);
+
+ if (pdu == NULL)
+ return 0;
+
+ if (!uuid)
+ return 0;
+
+ if (uuid->type != SDP_UUID16)
+ return 0;
+
+ if (len < min_len)
+ return 0;
+
+ if (vlen > len - min_len)
+ vlen = len - min_len;
+
+ pdu[0] = ATT_OP_FIND_BY_TYPE_REQ;
+ att_put_u16(start, &pdu[1]);
+ att_put_u16(end, &pdu[3]);
+ att_put_u16(uuid->value.uuid16, &pdu[5]);
+
+ if (vlen > 0) {
+ memcpy(&pdu[7], value, vlen);
+ return min_len + vlen;
+ }
+
+ return min_len;
+}
+
+uint16_t dec_find_by_type_req(const uint8_t *pdu, int len, uint16_t *start,
+ uint16_t *end, uuid_t *uuid, uint8_t *value, int *vlen)
{
- return 0;
+ int valuelen;
+ uint16_t min_len = sizeof(pdu[0]) + sizeof(*start) +
+ sizeof(*end) + sizeof(uint16_t);
+
+ if (pdu == NULL)
+ return 0;
+
+ if (len < min_len)
+ return 0;
+
+ if (pdu[0] != ATT_OP_FIND_BY_TYPE_REQ)
+ return 0;
+
+ /* First requested handle number */
+ if (start)
+ *start = att_get_u16(&pdu[1]);
+
+ /* Last requested handle number */
+ if (end)
+ *end = att_get_u16(&pdu[3]);
+
+ /* Always UUID16 */
+ if (uuid)
+ sdp_uuid16_create(uuid, att_get_u16(&pdu[5]));
+
+ valuelen = len - min_len;
+
+ /* Attribute value to find */
+ if (valuelen > 0 && value)
+ memcpy(value, pdu + min_len, valuelen);
+
+ if (vlen)
+ *vlen = valuelen;
+
+ return len;
}
uint16_t enc_read_by_type_req(uint16_t start, uint16_t end, uuid_t *uuid,
diff --git a/attrib/att.h b/attrib/att.h
index ea49dc2..9de338d 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -165,7 +165,9 @@ uint16_t dec_read_by_grp_req(const uint8_t *pdu, int len, uint16_t *start,
uint16_t *end, uuid_t *uuid);
uint16_t enc_read_by_grp_resp(struct att_data_list *list, uint8_t *pdu, int len);
uint16_t enc_find_by_type_req(uint16_t start, uint16_t end, uuid_t *uuid,
- uint8_t *pdu, int len);
+ const uint8_t *value, int vlen, uint8_t *pdu, int len);
+uint16_t dec_find_by_type_req(const uint8_t *pdu, int len, uint16_t *start,
+ uint16_t *end, uuid_t *uuid, uint8_t *value, int *vlen);
struct att_data_list *dec_read_by_grp_resp(const uint8_t *pdu, int len);
uint16_t enc_read_by_type_req(uint16_t start, uint16_t end, uuid_t *uuid,
uint8_t *pdu, int len);
--
1.7.3.2
^ permalink raw reply related
* Re: [PATCH 3/4] Add DBus OOB API documentation.
From: jaikumar Ganesh @ 2010-11-17 17:15 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <201011161112.05202.szymon.janc@tieto.com>
Hi Szymon:
On Tue, Nov 16, 2010 at 2:12 AM, Szymon Janc <szymon.janc@tieto.com> wrote:
>> >> Why are we enforcing this limitation ?
>> >
>> > Spec requires that each hash&randomizer values should be used only for one OOB
>> > transfer. Implementation enforces that by allowing only one active OOB plugin
>> > at time and DBUS OOB plugin only 'expose' plugin API over DBUS. So this
>> > limitation is a consequence of that.
>>
>> I don't think the spec says that. It just says every single time the
>> call to read the local adapters hash
>> and randomizer is done you get new values. You can use the value that
>> you have obtained for as many OOB pairings
>> as you wish.
>
> Please see note in Vol2. Part E. 7.3.60:
>
> "Each OOB transfer will have unique C and R values so after each OOB
> transfer this command shall be used to obtain a new set of values for the next
> OOB transfer."
>
Yes we were talking about the same thing, just at different layers. I
was talking about it from the user of the provider.
This looks fine to me.
>
> --
> Szymon Janc
>
^ permalink raw reply
* RE: [PATCH] Adding a new option to specify security level for gatttool
From: Mike Tsai @ 2010-11-17 16:57 UTC (permalink / raw)
To: tim.howes@accenture.com; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1AFE20D16950C745A2A83986B72E8748011F571E7497@EMEXM3131.dir.svc.accenture.com>
Hi Tim,
-----Original Message-----
From: tim.howes@accenture.com [mailto:tim.howes@accenture.com]
Sent: Wednesday, November 17, 2010 7:04 AM
To: Mike Tsai
Cc: linux-bluetooth@vger.kernel.org
Subject: RE: [PATCH] Adding a new option to specify security level for gatttool
Hi Mike,
-----Original Message-----
> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> owner@vger.kernel.org] On Behalf Of Johan Hedberg
> Sent: Tuesday, November 16, 2010 7:37 AM
> To: Sheldon Demario
> Cc: linux-bluetooth@vger.kernel.org
> Subject: Re: [PATCH] Adding a new option to specify security level for
> gatttool
>
> Hi Sheldon,
>
> On Tue, Nov 16, 2010, Sheldon Demario wrote:
> > ---
> > TODO | 6 ------
> > attrib/gatttool.c | 15 +++++++++++++--
> > 2 files changed, 13 insertions(+), 8 deletions(-)
>
> In general the patch seems fine, but:
>
> > + if (strncmp(opt_sec_level, "medium", 6) == 0)
> > + sec_level = BT_IO_SEC_MEDIUM;
> > + else if (strncmp(opt_sec_level, "high", 4) == 0)
> > + sec_level = BT_IO_SEC_HIGH;
> > + else
> > + sec_level = BT_IO_SEC_LOW;
>
> Why do you use strncmp instead of strcmp (or even g_str_equal)? I don't
> think there's any need to map e.g. "mediumfoobar" to BT_IO_SEC_MEDIUM
> ;)
>
> [Mtsai] I am not sure what are the definition of "low", "medium" or
> "high". By the spec of Core 4.0, LE has 2 security modes and different
> security levels based on the method of pairing (or bonding). It may be
> appeal to end user with "low", "medium" and "high" definition, but it
> can't be reference with LE spec. I would suggest, instead, following
> terms,
>
> "No security",
> "unauthenticated encryption",
> "authenticated encryption",
> "unauthenticated data signing",
> "authenticated data signing,
>
> Mike
To some extent I agree; however, the semantics of such an API would have to be careful. A particular profile should not "force" data signing because if the link is already encrypted there is little point using data signing. So from that point of view exposing a more abstract API (a bit like "high") is better. However, it is hard to map "high" onto any of the ones you listed (which I agree is a good list). So perhaps it is better to have the API semantics as "advisory" or "requests" which can be fulfilled by the underlying stack in other ways (eg encryption for data-signing).
Cheers
Tim
[Mtsai] I fully embrace the "abstract" API concept. However, all new LE profiles have clearly defined the security request per each attribute now. As a generic attribute profile (as GATT), without clear indication from Application what level of security is requested, GATT/GAP can handle the security request incorrectly and cause IOP issues. For example, what does "low" mean when GAP/SM gets this request from Application? It does not tell GAP/SM what kind of pairing shall be performed and if encryption or data signing for accessing the attributes.
Cheers,
Mike
This message is for the designated recipient only and may contain privileged, proprietary, or otherwise private information. If you have received it in error, please notify the sender immediately and delete the original. Any other use of the email by you is prohibited.
^ permalink raw reply
* Re: [PATCH v5] Bluetooth: btwilink driver
From: Gustavo F. Padovan @ 2010-11-17 16:52 UTC (permalink / raw)
To: Pavan Savoy; +Cc: Vitaly Wool, marcel, linux-bluetooth, linux-kernel
In-Reply-To: <AANLkTi=NH8gbW8iCKOQN0bmbL=nqewsZkUik+VkvWUgi@mail.gmail.com>
Hi Pavan,
* Pavan Savoy <pavan_savoy@sify.com> [2010-11-17 11:13:26 +0530]:
> On Wed, Nov 17, 2010 at 4:50 AM, Vitaly Wool <vitalywool@gmail.com> wrote:
> >>> + =A0 =A0 /* Registration with ST layer is successful,
> >>> + =A0 =A0 =A0* hardware is ready to accept commands from HCI core.
> >>> + =A0 =A0 =A0*/
> >>> + =A0 =A0 if (test_and_set_bit(HCI_RUNNING, &hdev->flags)) {
> >>> + =A0 =A0 =A0 =A0 =A0 =A0 clear_bit(HCI_RUNNING, &hdev->flags);
> >>> + =A0 =A0 =A0 =A0 =A0 =A0 err =3D st_unregister(ST_BT);
> >>> + =A0 =A0 =A0 =A0 =A0 =A0 if (err)
> >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 BT_ERR("st_unregister() fai=
led with error %d", err);
> >>> + =A0 =A0 =A0 =A0 =A0 =A0 hst->st_write =3D NULL;
> >>> + =A0 =A0 }
> >>
> >>
> >> What are you trying to do here? test_and_set_bit() result doesn't say
> >> nothing about error and you shall put test_and_set_bit should be in the
> >> beginning, to know if your device is already opened or not and then
> >> clear_bit if some error ocurrs during the function.
> >>
> >
> > Yeap, this piece of code beats me is well. Why is it an error if this
> > bit wasn't already set?
>=20
> Vitaly, Gustavo,
>=20
> I suppose I never understood HCI_RUNNING flag that way, as in an error
> check mechanism to avoid multiple hci0 ups.
>=20
> What I understood was that HCI_RUNNING suggested as to when hci0 was
> ready to be used. With this understanding, I wanted to make sure I
> downloaded the firmware for the chip before I proclaim to the world
> that the hci0 is ready to be used, as in HCI_RUNNING.
>=20
> For example, I didn't want my _send_frame to be called before I did
> the firmware download - since firmware download takes time - 45kb
> send/wait commands :(
>=20
> But I suppose I now understand - What I would rather do is test_bit in
> the beginning of function and do a set_bit at the end of function -
> does this make sense ?
It does, but does it as test_and_set and then clear if error as we do in
other drivers.
--=20
Gustavo F. Padovan
http://profusion.mobi
^ permalink raw reply
* Re: [PATCH v5] Bluetooth: btwilink driver
From: Gustavo F. Padovan @ 2010-11-17 16:50 UTC (permalink / raw)
To: Pavan Savoy; +Cc: marcel, linux-bluetooth, linux-kernel
In-Reply-To: <AANLkTikbhaZOvM43Vv6wdcbmK8nh2Nq2Hc1i_naxWjYM@mail.gmail.com>
Hi Pavan,
* Pavan Savoy <pavan_savoy@sify.com> [2010-11-17 11:04:59 +0530]:
> Gustavo,
>=20
> On Wed, Nov 17, 2010 at 4:24 AM, Gustavo F. Padovan
> <padovan@profusion.mobi> wrote:
> > Hi Pavan,
> >
> > * pavan_savoy@ti.com <pavan_savoy@ti.com> [2010-11-10 08:07:26 -0500]:
> >
> >> From: Pavan Savoy <pavan_savoy@ti.com>
> >>
> >> Marcel,
> >>
> >> Thanks for the comments...
> >> This patch contains,
> >> v5 comments :-
> >> declaration and assiging of variables and private data fixed up.
> >> proper casting.
> >> removed redundant un-necessary checks in send_frame.
> >> HCI_RUNNING fixes in terms of test_and_set/clear bit instead of set and
> >> clear.
> >> removed redundant checks for hdev, skb being NULL.
> >> removed module_param of reset, also WiLink don't need HCI_RESET anyway=
s.
> >> removed ti_st_register_dev function and functionality moved to _probe.
> >> module_init/exit function names fixed up.
> >>
> >> stat byte counter increments and tx_complete is similar to hci_ldisc.
> >> Also I have not implemented the flush routine, since the functionality
> >> which needs to be done in flush routine is done in the underlying driv=
er
> >> which is the shared transport driver and moreover the btwilink driver =
by
> >> itself doesn't maintains queue or data relevant to transport, so nothi=
ng
> >> to do.
> >>
> >> And Yes, I have verified this driver with multiple up/down reset on
> >> hci0.
> >> Also I generally test a2dp/ftp to verify large data transfers.
> >
> > Before re-submit a patch you have to fix all issues reported by the
> > reviewer or reply to patch thread why do you think you right and don't
> > want to change that. That is not happening with your patches, you are
> > not fixing all the stuff before re-submiting and it is not the first
> > time. =A0If you do it right we can review it fast and your code goes
> > earlier to mainline.
> > You should also answer the questions first =A0like "Where is the
> > ti_st_proto.write coming from?" You just ignored the
> > review and submitted a new patch.
>=20
> This is the reason, I tend to keep the patch comments in the mail.
> I have mentioned here the
> 1. comments I have taken care of,
> 2. few comments which I don't understand why it is like that and which
> are not taken care of.
You should reply inline, and not do top posting like this.
>=20
> Also the question on ti_st_proto.write, I have answered it twice in
> mail, once to you and once to marcel, for more clarity I have even
> added it in the code comments, Please have a look @ line
> >> + /* ti_st_proto.write is filled up by the underlying shared
> >> + * transport driver upon registration
> >> + */
> As to why this function is not EXPORT_SYMBOL and just an function
> pointer, Yes I had it as function pointer - But since something like
> _read is callback from the protocol driver perspective - to maintain
> uniformity I have this as function pointer.
> (Note: comments to other drivers which are based on ST driver intended
> read/write to be pointers which register/unregister to be EXPORTs).
>=20
> Ok, apart from this there was an open question as why I am checking
> for only 2 error conditions, it is because the underlying driver only
> can send those 2 errors and nothing else (st_register has few errors
> it can throw...)
I don't care if it can send only two errors, someone can change the
underlying driver and we at the Bluetooth subsystem may never know that.
So check for all errors there, instead of two.
--=20
Gustavo F. Padovan
http://profusion.mobi
^ permalink raw reply
* Re: [PATCHv3 5/7] Add approval request for incoming pairing requests with OOB mechanism
From: Szymon Janc @ 2010-11-17 16:03 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth@vger.kernel.org, marcel@holtmann.org
In-Reply-To: <20101116162412.GB3638@jh-x301>
Hi Johan,
> > + req->msg = dbus_message_new_method_call(agent->name, agent->path,
> > + "org.bluez.Agent", "RequestApproval");
>
> I think the name of this method is one of the more important things to
> get right before we push this upstream. I'm not so sure of the current
> proposal. I have a feeling that it'd be a good idea to have the term
> "pair" somewhere explicitly in the name. How about "RequestPairing"? I
> know Marcel will have something to say about this too.
Maybe make it more descriptive like RequestPairingApproval ?
Yet, RequestPairing sounds ok.
--
Szymon Janc
on behalf of ST-Ericsson
^ permalink raw reply
* [PATCH 3/3] Emit "DeviceFound" signal for LE devices
From: Bruna Moreira @ 2010-11-17 15:59 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Bruna Moreira
In-Reply-To: <1290009593-13658-1-git-send-email-bruna.moreira@openbossa.org>
The adapter_emit_device_found() function was modified to emit
DeviceFound signal for LE devices as well.
---
src/adapter.c | 30 +++++++++++++++++++++++++-----
1 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index a7e78bc..e376f59 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2974,6 +2974,27 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
if (device)
paired = device_is_paired(device);
+ /* Extract UUIDs from extended inquiry response if any */
+ dev->services = get_eir_uuids(eir_data, eir_length, dev->services);
+ uuid_count = g_slist_length(dev->services);
+
+ if (dev->services)
+ uuids = strlist2array(dev->services);
+
+ if (dev->le) {
+ emit_device_found(adapter->path, paddr,
+ "Address", DBUS_TYPE_STRING, &paddr,
+ "RSSI", DBUS_TYPE_INT16, &rssi,
+ "Name", DBUS_TYPE_STRING, &dev->name,
+ "Paired", DBUS_TYPE_BOOLEAN, &paired,
+ "UUIDs", DBUS_TYPE_ARRAY, &uuids, uuid_count,
+ NULL);
+
+ g_strfreev(uuids);
+
+ return;
+ }
+
icon = class_to_icon(dev->class);
if (!dev->alias) {
@@ -2985,12 +3006,7 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
} else
alias = g_strdup(dev->alias);
- /* Extract UUIDs from extended inquiry response if any */
- dev->services = get_eir_uuids(eir_data, eir_length, dev->services);
- uuid_count = g_slist_length(dev->services);
-
if (dev->services) {
- uuids = strlist2array(dev->services);
g_slist_foreach(dev->services, (GFunc) g_free, NULL);
g_slist_free(dev->services);
dev->services = NULL;
@@ -3071,6 +3087,10 @@ void adapter_update_device_from_info(struct btd_adapter *adapter,
dev->name = tmp_name;
}
}
+
+ /* FIXME: check if other information was changed before emitting the
+ * signal */
+ adapter_emit_device_found(adapter, dev, info->data, info->length);
}
void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
--
1.7.0.4
^ permalink raw reply related
* [PATCH 2/3] Extract service UUIDs from advertising data
From: Bruna Moreira @ 2010-11-17 15:59 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Bruna Moreira
In-Reply-To: <1290009593-13658-1-git-send-email-bruna.moreira@openbossa.org>
Make get_eir_uuids() return a GSList of strings, so it can be reused to
extract UUIDs from LE advertising data. The bt_strlist2array() helper
function was created to convert a GSList into a plain array of strings
(needed to send through D-Bus).
---
src/adapter.c | 71 ++++++++++++++++++++++++++++++++++++++++++++-------------
src/adapter.h | 1 +
2 files changed, 56 insertions(+), 16 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 0089d6d..a7e78bc 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -202,6 +202,8 @@ static void dev_info_free(struct remote_dev_info *dev)
{
g_free(dev->name);
g_free(dev->alias);
+ g_slist_foreach(dev->services, (GFunc) g_free, NULL);
+ g_slist_free(dev->services);
g_free(dev);
}
@@ -2826,11 +2828,27 @@ static void emit_device_found(const char *path, const char *address,
g_dbus_send_message(connection, signal);
}
-static char **get_eir_uuids(uint8_t *eir_data, size_t eir_length,
- size_t *uuid_count)
+static char **strlist2array(GSList *list)
+{
+ GSList *l;
+ unsigned int i, n;
+ char **array;
+
+ if (list == NULL)
+ return NULL;
+
+ n = g_slist_length(list);
+ array = g_new0(char *, n + 1);
+
+ for (l = list, i = 0; l; l = l->next, i++)
+ array[i] = g_strdup((const gchar *) l->data);
+
+ return array;
+}
+
+static GSList *get_eir_uuids(uint8_t *eir_data, size_t eir_length, GSList *list)
{
uint16_t len = 0;
- char **uuids;
size_t total;
size_t uuid16_count = 0;
size_t uuid32_count = 0;
@@ -2839,10 +2857,11 @@ static char **get_eir_uuids(uint8_t *eir_data, size_t eir_length,
uint8_t *uuid32;
uint8_t *uuid128;
uuid_t service;
+ char *uuid_str;
unsigned int i;
if (eir_data == NULL || eir_length == 0)
- return NULL;
+ return list;
while (len < eir_length - 1) {
uint8_t field_len = eir_data[0];
@@ -2875,15 +2894,12 @@ static char **get_eir_uuids(uint8_t *eir_data, size_t eir_length,
/* Bail out if got incorrect length */
if (len > eir_length)
- return NULL;
+ return list;
total = uuid16_count + uuid32_count + uuid128_count;
- *uuid_count = total;
if (!total)
- return NULL;
-
- uuids = g_new0(char *, total + 1);
+ return list;
/* Generate uuids in SDP format (EIR data is Little Endian) */
service.type = SDP_UUID16;
@@ -2892,7 +2908,12 @@ static char **get_eir_uuids(uint8_t *eir_data, size_t eir_length,
val16 = (val16 << 8) + uuid16[0];
service.value.uuid16 = val16;
- uuids[i] = bt_uuid2string(&service);
+ uuid_str = bt_uuid2string(&service);
+ if (g_slist_find_custom(list, uuid_str,
+ (GCompareFunc) strcmp) == NULL)
+ list = g_slist_append(list, uuid_str);
+ else
+ g_free(uuid_str);
uuid16 += 2;
}
@@ -2905,7 +2926,12 @@ static char **get_eir_uuids(uint8_t *eir_data, size_t eir_length,
val32 = (val32 << 8) + uuid32[k];
service.value.uuid32 = val32;
- uuids[i] = bt_uuid2string(&service);
+ uuid_str = bt_uuid2string(&service);
+ if (g_slist_find_custom(list, uuid_str,
+ (GCompareFunc) strcmp) == NULL)
+ list = g_slist_append(list, uuid_str);
+ else
+ g_free(uuid_str);
uuid32 += 4;
}
@@ -2916,11 +2942,16 @@ static char **get_eir_uuids(uint8_t *eir_data, size_t eir_length,
for (k = 0; k < 16; k++)
service.value.uuid128.data[k] = uuid128[16 - k - 1];
- uuids[i] = bt_uuid2string(&service);
+ uuid_str = bt_uuid2string(&service);
+ if (g_slist_find_custom(list, uuid_str,
+ (GCompareFunc) strcmp) == NULL)
+ list = g_slist_append(list, uuid_str);
+ else
+ g_free(uuid_str);
uuid128 += 16;
}
- return uuids;
+ return list;
}
void adapter_emit_device_found(struct btd_adapter *adapter,
@@ -2934,7 +2965,7 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
dbus_int16_t rssi = dev->rssi;
char *alias;
char **uuids = NULL;
- size_t uuid_count = 0;
+ size_t uuid_count;
ba2str(&dev->bdaddr, peer_addr);
ba2str(&adapter->bdaddr, local_addr);
@@ -2954,8 +2985,16 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
} else
alias = g_strdup(dev->alias);
- /* Extract UUIDs from extended inquiry response if any*/
- uuids = get_eir_uuids(eir_data, eir_length, &uuid_count);
+ /* Extract UUIDs from extended inquiry response if any */
+ dev->services = get_eir_uuids(eir_data, eir_length, dev->services);
+ uuid_count = g_slist_length(dev->services);
+
+ if (dev->services) {
+ uuids = strlist2array(dev->services);
+ g_slist_foreach(dev->services, (GFunc) g_free, NULL);
+ g_slist_free(dev->services);
+ dev->services = NULL;
+ }
emit_device_found(adapter->path, paddr,
"Address", DBUS_TYPE_STRING, &paddr,
diff --git a/src/adapter.h b/src/adapter.h
index a744f61..4af69b3 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -71,6 +71,7 @@ struct remote_dev_info {
name_status_t name_status;
gboolean le;
/* LE adv data */
+ GSList *services;
uint8_t evt_type;
uint8_t bdaddr_type;
};
--
1.7.0.4
^ permalink raw reply related
* [PATCH 1/3] Advertising data: extract local name
From: Bruna Moreira @ 2010-11-17 15:59 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Bruna Moreira
Move extract_eir_name() to glib-helper.c file and rename function to
bt_extract_eir_name(). The local name is extracted from the advertising
data.
---
src/adapter.c | 9 +++++++++
src/event.c | 23 +----------------------
src/glib-helper.c | 22 ++++++++++++++++++++++
src/glib-helper.h | 1 +
4 files changed, 33 insertions(+), 22 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 7fb0d3f..0089d6d 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -3007,6 +3007,7 @@ void adapter_update_device_from_info(struct btd_adapter *adapter,
bdaddr_t bdaddr;
gboolean new_dev;
int8_t rssi;
+ uint8_t type;
rssi = *(info->data + info->length);
bdaddr = info->bdaddr;
@@ -3023,6 +3024,14 @@ void adapter_update_device_from_info(struct btd_adapter *adapter,
adapter->found_devices = g_slist_sort(adapter->found_devices,
(GCompareFunc) dev_rssi_cmp);
+
+ if (info->length) {
+ char *tmp_name = bt_extract_eir_name(info->data, &type);
+ if (tmp_name) {
+ g_free(dev->name);
+ dev->name = tmp_name;
+ }
+ }
}
void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
diff --git a/src/event.c b/src/event.c
index 008bbe3..daab71a 100644
--- a/src/event.c
+++ b/src/event.c
@@ -301,27 +301,6 @@ void btd_event_simple_pairing_complete(bdaddr_t *local, bdaddr_t *peer,
device_simple_pairing_complete(device, status);
}
-static char *extract_eir_name(uint8_t *data, uint8_t *type)
-{
- if (!data || !type)
- return NULL;
-
- if (data[0] == 0)
- return NULL;
-
- *type = data[1];
-
- switch (*type) {
- case 0x08:
- case 0x09:
- if (!g_utf8_validate((char *) (data + 2), data[0] - 1, NULL))
- return strdup("");
- return strndup((char *) (data + 2), data[0] - 1);
- }
-
- return NULL;
-}
-
void btd_event_advertising_report(bdaddr_t *local, le_advertising_info *info)
{
struct btd_adapter *adapter;
@@ -410,7 +389,7 @@ void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
} else
legacy = TRUE;
- tmp_name = extract_eir_name(data, &name_type);
+ tmp_name = bt_extract_eir_name(data, &name_type);
if (tmp_name) {
if (name_type == 0x09) {
write_device_name(local, peer, tmp_name);
diff --git a/src/glib-helper.c b/src/glib-helper.c
index 9d76626..33668d7 100644
--- a/src/glib-helper.c
+++ b/src/glib-helper.c
@@ -43,6 +43,7 @@
#include <glib.h>
#include "glib-helper.h"
+#include "sdpd.h"
/* Number of seconds to keep a sdp_session_t in the cache */
#define CACHE_TIMEOUT 2
@@ -576,3 +577,24 @@ GSList *bt_string2list(const gchar *str)
return l;
}
+
+char *bt_extract_eir_name(uint8_t *data, uint8_t *type)
+{
+ if (!data || !type)
+ return NULL;
+
+ if (data[0] == 0)
+ return NULL;
+
+ *type = data[1];
+
+ switch (*type) {
+ case EIR_NAME_SHORT:
+ case EIR_NAME_COMPLETE:
+ if (!g_utf8_validate((char *) (data + 2), data[0] - 1, NULL))
+ return strdup("");
+ return strndup((char *) (data + 2), data[0] - 1);
+ }
+
+ return NULL;
+}
diff --git a/src/glib-helper.h b/src/glib-helper.h
index e89c2c6..dfe4123 100644
--- a/src/glib-helper.h
+++ b/src/glib-helper.h
@@ -38,3 +38,4 @@ char *bt_name2string(const char *string);
int bt_string2uuid(uuid_t *uuid, const char *string);
gchar *bt_list2string(GSList *list);
GSList *bt_string2list(const gchar *str);
+char *bt_extract_eir_name(uint8_t *data, uint8_t *type);
--
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