Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH v7] Bluetooth: btwilink driver
From: Pavan Savoy @ 2010-12-02  0:48 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: marcel, linux-bluetooth, linux-kernel
In-Reply-To: <AANLkTiktDkjFH=aGbu3f+mkVUgmFDwB6xLW3e1VNXKtz@mail.gmail.com>

Gustavo,

On Tue, Nov 30, 2010 at 9:30 PM, Pavan Savoy <pavan_savoy@ti.com> wrote:
>>> Marcel, Gustavo,
>>> comments attended to from v5 and v6,

There was 1 solution which was proposed for this,
See in tty_receive/st_int_recv() - we pretty much want only the
packet_id, header length and
the offset of the payload in the header structure.
this is to pack together the data coming in single frame or sets of
multiple frames...

so if bt driver can do something like,
st_register(acl_proto) -> where ACL_PKT=3D0x2, acl_hdr.len and
acl_hdr.payload_offset is sent to ST
st_register(sco_proto) -> where SCO_PKT=3D0x3, sco_hdr.len and
sco_hdr.payload_offset is sent to ST
st_register(evt_proto) -> where EVT_PKT=3D0x2, sco_hdr.len and
evt_hdr.payload_offset is sent to ST

and all 3 map to same st_recv() functions from ST, then
the ST driver as you suggested can be generic enough not to include
any net/bluetooth/ stuff,
and also can be ignorant about any bluetooth, fm or gps stuff in general...

So please suggest if this seems a valid solution, we had previously
declined it because it made bt_driver
sort of register to ST 3 times instead of 1 time like now....

Thanks,
Pavan

>>> 1. Inside ti_st_open, I previously only checked for EINPROGRESS & EPERM=
,
>>> Now I handle for EINPROGRESS - which is not really an error and
>>> return during all other error cases.
>>>
>>> 2. _write is still a function pointer and not an exported function, I
>>> need to change the underlying driver's code for this.
>>> However, previous lkml comments on the underlying driver's code
>>> suggested it to be kept as a function pointer and not EXPORT.
>>> Gustavo, Marcel - Please comment on this.
>>> Is this absolutely required? If so why?
>>>
>>> 3. test_and_set_bit of HCI_RUNNING is done at beginning of
>>> ti_st_open, and did not see issues during firmware download.
>>> However ideally I would still like to set HCI_RUNNING once the firmware
>>> download is done, because I don't want to allow a _send_frame during
>>> firmware download - Marcel, Gustavo - Please comment.
>>>
>>> 4. test_and_clear of HCI_RUNNING now done @ beginning of close.
>>>
>>> 5. EAGAIN on failure of st_write is to suggest to try and write again.
>>> I have never this happen - However only if UART goes bad this case may
>>> occur.
>>>
>>> 6. ti_st_tx_complete is very similar to hci_ldisc's tx_complete - in
>>> fact the code is pretty much borrowed from there.
>>> Marcel, Gustavo - Please suggest where should it be done? If not here.
>>>
>>> 7. comments cleaned-up + hst memory leak fixed when hci_alloc_dev fails=
.
>>>
>>> 8. platform_driver registration inside module_init now is similar to
>>> other drivers.
>>>
>>> 9. Dan Carpenter's comments on leaking hst memory on failed
>>> hci_register_dev and empty space after quotes in debug statements
>>> fixed.
>>>
>>> Thanks for the comments...
>>> Sorry, for previously not being very clear on which comments were
>>> handled and which were not.
>>>
>>> -- patch description --
>>>
>>> This is the bluetooth protocol driver for the TI WiLink7 chipsets.
>>> Texas Instrument's WiLink chipsets combine wireless technologies
>>> like BT, FM, GPS and WLAN onto a single chip.
>>>
>>> This Bluetooth driver works on top of the TI_ST shared transport
>>> line discipline driver which also allows other drivers like
>>> FM V4L2 and GPS character driver to make use of the same UART interface=
.
>>>
>>> Kconfig and Makefile modifications to enable the Bluetooth
>>> driver for Texas Instrument's WiLink 7 chipset.
>>>
>>> Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
>>> ---
>>> =C2=A0drivers/bluetooth/Kconfig =C2=A0 =C2=A0| =C2=A0 10 ++
>>> =C2=A0drivers/bluetooth/Makefile =C2=A0 | =C2=A0 =C2=A01 +
>>> =C2=A0drivers/bluetooth/btwilink.c | =C2=A0363 ++++++++++++++++++++++++=
++++++++++++++++++
>>> =C2=A03 files changed, 374 insertions(+), 0 deletions(-)
>>> =C2=A0create mode 100644 drivers/bluetooth/btwilink.c
>>
>> So as part of reviewing this I took a look at your underlying driver and
>> I didn't like what I saw there, you are handling Bluetooth stuff inside
>> the core driver and that is just wrong. You have a Bluetooth driver here
>> then you have to leave the Bluetooth data handling to the Bluetooth
>> driver and do not do that in the core.
>
> Thanks for reviewing this and the underlying driver.
> yes, we do have Bluetooth/FM/GPS handling inside the TI ST driver, on
> addition of further technologies
> we do plan to have them inside the ST driver too.
>
> The understanding of BT or FM or GPS is required for the ST driver
> because, the data coming from the chip
> can either be of these technologies, further-more the data might not
> come in a set.
> As in, an a2dp/ftp ACL frame might come in 2 frames instead of 1, and
> in other cases,
> there might be a HCI-EVENT + FM CH8 data in a single frame received by th=
e UART.
>
>> So I'm going to clear NACK this. Your TI ST driver architecture is
>> a mess, Ideally you should not have any #include <net/bluetoooth/...>
>> there. Implement a core driver that only gets the Shared Transport
>> data and pass it to the right driver without look into the data and
>> process it. Process the data is not the role of the core driver.
>
> So as I see it the tty_receive which translates to st_int_recv() in
> TI-ST is the area of concern for
> you ...
> So any suggestions as to how we can go about just abstracting the BT,
> FM and GPS data part to their individual drivers?
>
>> Please fix this and come back when you have a proper solution for your
>> driver.
>>
>> --
>> Gustavo F. Padovan
>> http://profusion.mobi
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetoot=
h" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at =C2=A0http://vger.kernel.org/majordomo-info.html
>>
>

^ permalink raw reply

* pull request: bluetooth-next-2.6 2010-11-01
From: Gustavo F. Padovan @ 2010-12-02  0:32 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, linux-bluetooth

Hi John,

Here are the first batch for 2.6.38, biggest change here is the move of
the Remote Name Request to the kernel to fix some synchronization
problems with other commands sent during the connection setup in
Bluetooth. The rest is just bug fixes and clean ups in the whole stack,
no big feature this time.

This pull request includes the patches that I sent in the pull request
to 2.6.37, but you should expect no problem when merging everything in
wireless-next-2.6.

Please pull or let me know anything wrong. Thanks.


The following changes since commit 61790c5f3c5f158821821a00797d94504531839f:

  iwlagn: fix microcode error on 4965 (2010-11-30 13:58:18 -0500)

are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/padovan/bluetooth-next-2.6.git master

Anderson Lizardo (1):
      Bluetooth: Fix error handling for l2cap_init()

Andrei Emeltchenko (8):
      Bluetooth: Check sk is not owned before freeing l2cap_conn
      Bluetooth: timer check sk is not owned before freeing
      Bluetooth: do not use assignment in if condition
      Bluetooth: clean up sco code
      Bluetooth: clean up rfcomm code
      Bluetooth: clean up l2cap code
      Bluetooth: clean up hci code
      Bluetooth: clean up legal text

Bala Shanmugam (1):
      Bluetooth: Add new PID for Atheros 3011

Gustavo F. Padovan (4):
      Bluetooth: Fix not returning proper error in SCO
      Merge git://git.kernel.org/.../padovan/bluetooth-2.6 into test
      Bluetooth: Get rid of __l2cap_get_sock_by_psm()
      Bluetooth: Get rid of __rfcomm_get_sock_by_channel()

Johan Hedberg (3):
      Bluetooth: Simplify remote features callback function logic
      Bluetooth: Create a unified authentication request function
      Bluetooth: Automate remote name requests

Stefan Seyfried (1):
      Bluetooth: Fix log spamming in btusb due to autosuspend

Vasiliy Kulikov (3):
      Bluetooth: bnep: fix information leak to userland
      Bluetooth: cmtp: fix information leak to userland
      Bluetooth: hidp: fix information leak to userland

 drivers/bluetooth/ath3k.c        |    4 +
 drivers/bluetooth/btusb.c        |   12 ++-
 include/net/bluetooth/hci.h      |   16 ++--
 include/net/bluetooth/hci_core.h |   14 ++--
 include/net/bluetooth/l2cap.h    |   22 +++---
 include/net/bluetooth/rfcomm.h   |   18 ++--
 include/net/bluetooth/sco.h      |   20 ++--
 net/bluetooth/bnep/core.c        |    1 +
 net/bluetooth/cmtp/core.c        |    1 +
 net/bluetooth/hci_conn.c         |   23 +++--
 net/bluetooth/hci_core.c         |   66 +++++++++-----
 net/bluetooth/hci_event.c        |  177 +++++++++++++++++++++++++++-----------
 net/bluetooth/hci_sock.c         |   17 +++--
 net/bluetooth/hidp/core.c        |    2 +-
 net/bluetooth/l2cap.c            |   94 ++++++++++++++-------
 net/bluetooth/rfcomm/core.c      |    8 +-
 net/bluetooth/rfcomm/sock.c      |   24 ++---
 net/bluetooth/rfcomm/tty.c       |   28 ++++---
 net/bluetooth/sco.c              |   28 ++++---
 19 files changed, 363 insertions(+), 212 deletions(-)

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

^ permalink raw reply

* Re: [PATCHv2 4/5] Bluetooth: clean up hci code
From: Gustavo F. Padovan @ 2010-12-01 21:53 UTC (permalink / raw)
  To: Emeltchenko Andrei; +Cc: linux-bluetooth
In-Reply-To: <1291215506-11398-5-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

* Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2010-12-01 16:58:25 +0200]:

> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> 
> Do not use assignment in IF condition, remove extra spaces,
> fixing typos, simplify code.
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> ---
>  include/net/bluetooth/hci.h      |    4 +-
>  include/net/bluetooth/hci_core.h |   14 ++++----
>  net/bluetooth/hci_conn.c         |   23 ++++++++----
>  net/bluetooth/hci_core.c         |   66 ++++++++++++++++++++++++--------------
>  net/bluetooth/hci_event.c        |    8 +++--
>  net/bluetooth/hci_sock.c         |   17 ++++++---
>  6 files changed, 82 insertions(+), 50 deletions(-)

Applied.

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

^ permalink raw reply

* Re: [PATCHv2 3/5] Bluetooth: clean up l2cap code
From: Gustavo F. Padovan @ 2010-12-01 21:53 UTC (permalink / raw)
  To: Emeltchenko Andrei; +Cc: linux-bluetooth
In-Reply-To: <1291215506-11398-4-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

* Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2010-12-01 16:58:24 +0200]:

> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> 
> Do not initialize static vars to zero, macros with complex values
> shall be enclosed with (), remove unneeded braces.
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> ---
>  include/net/bluetooth/l2cap.h |   10 +++++-----
>  net/bluetooth/l2cap.c         |    7 +++----
>  2 files changed, 8 insertions(+), 9 deletions(-)

Applied, thanks for clean up this.

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

^ permalink raw reply

* Re: [PATCHv2 2/5] Bluetooth: clean up rfcomm code
From: Gustavo F. Padovan @ 2010-12-01 21:52 UTC (permalink / raw)
  To: Emeltchenko Andrei; +Cc: linux-bluetooth
In-Reply-To: <1291215506-11398-3-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

* Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2010-12-01 16:58:23 +0200]:

> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> 
> Remove extra spaces, assignments in if statement, zeroing static
> variables, extra braces. Fix includes.
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> ---
>  include/net/bluetooth/rfcomm.h |    4 ++--
>  net/bluetooth/rfcomm/core.c    |    8 ++++----
>  net/bluetooth/rfcomm/sock.c    |    5 +++--
>  net/bluetooth/rfcomm/tty.c     |   28 ++++++++++++++++------------
>  4 files changed, 25 insertions(+), 20 deletions(-)

Applied, thanks for clean up this.

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

^ permalink raw reply

* Re: [PATCHv2 1/5] Bluetooth: clean up sco code
From: Gustavo F. Padovan @ 2010-12-01 21:44 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: Emeltchenko Andrei, linux-bluetooth
In-Reply-To: <AANLkTi==oXHyrtkjRD_mbmRFsDnTH7+S_JOFHRemXJLG@mail.gmail.com>

Hi Anderson,

* Anderson Lizardo <anderson.lizardo@openbossa.org> [2010-12-01 17:31:15 -0400]:

> On Wed, Dec 1, 2010 at 5:20 PM, Gustavo F. Padovan
> <padovan@profusion.mobi> wrote:
> >> -static int disable_esco = 0;
> >> +static int disable_esco;
> >
> > I don't think this change is right. Can we be sure that disable_esco
> > will be 0 by default?
> 
> I think Andrei's patch is ok. IIRC kernel zeroes BSS on init, that's
> why checkpatch.pl complains when you initialize it explicitely.

Great, I didn't know about that. Applied then. Thanks all!

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

^ permalink raw reply

* Re: [PATCHv2 5/5] Bluetooth: clean up legal text
From: Gustavo F. Padovan @ 2010-12-01 21:42 UTC (permalink / raw)
  To: Emeltchenko Andrei; +Cc: linux-bluetooth
In-Reply-To: <1291215506-11398-6-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

* Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2010-12-01 16:58:26 +0200]:

> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> 
> Remove extra spaces from legal text so that legal stuff looks
> the same for all bluetooth code.
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> ---
>  include/net/bluetooth/hci.h    |   12 ++++++------
>  include/net/bluetooth/l2cap.h  |   12 ++++++------
>  include/net/bluetooth/rfcomm.h |   14 +++++++-------
>  include/net/bluetooth/sco.h    |   12 ++++++------
>  4 files changed, 25 insertions(+), 25 deletions(-)

This one is applied. Thanks.

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

^ permalink raw reply

* Re: [PATCHv2 1/5] Bluetooth: clean up sco code
From: Anderson Lizardo @ 2010-12-01 21:31 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: Emeltchenko Andrei, linux-bluetooth
In-Reply-To: <20101201212034.GE16125@vigoh>

On Wed, Dec 1, 2010 at 5:20 PM, Gustavo F. Padovan
<padovan@profusion.mobi> wrote:
>> -static int disable_esco = 0;
>> +static int disable_esco;
>
> I don't think this change is right. Can we be sure that disable_esco
> will be 0 by default?

I think Andrei's patch is ok. IIRC kernel zeroes BSS on init, that's
why checkpatch.pl complains when you initialize it explicitely.

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

^ permalink raw reply

* Re: [PATCHv2 3/5] Bluetooth: clean up l2cap code
From: Gustavo F. Padovan @ 2010-12-01 21:21 UTC (permalink / raw)
  To: Emeltchenko Andrei; +Cc: linux-bluetooth
In-Reply-To: <1291215506-11398-4-git-send-email-Andrei.Emeltchenko.news@gmail.com>

* Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2010-12-01 16:58:24 +0200]:

> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> 
> Do not initialize static vars to zero, macros with complex values
> shall be enclosed with (), remove unneeded braces.
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> ---
>  include/net/bluetooth/l2cap.h |   10 +++++-----
>  net/bluetooth/l2cap.c         |    7 +++----
>  2 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index c819c8b..217bb91 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -417,11 +417,11 @@ static inline int l2cap_tx_window_full(struct sock *sk)
>  	return sub == pi->remote_tx_win;
>  }
>  
> -#define __get_txseq(ctrl) ((ctrl) & L2CAP_CTRL_TXSEQ) >> 1
> -#define __get_reqseq(ctrl) ((ctrl) & L2CAP_CTRL_REQSEQ) >> 8
> -#define __is_iframe(ctrl) !((ctrl) & L2CAP_CTRL_FRAME_TYPE)
> -#define __is_sframe(ctrl) (ctrl) & L2CAP_CTRL_FRAME_TYPE
> -#define __is_sar_start(ctrl) ((ctrl) & L2CAP_CTRL_SAR) == L2CAP_SDU_START
> +#define __get_txseq(ctrl)	(((ctrl) & L2CAP_CTRL_TXSEQ) >> 1)
> +#define __get_reqseq(ctrl)	(((ctrl) & L2CAP_CTRL_REQSEQ) >> 8)
> +#define __is_iframe(ctrl)	(!((ctrl) & L2CAP_CTRL_FRAME_TYPE))
> +#define __is_sframe(ctrl)	((ctrl) & L2CAP_CTRL_FRAME_TYPE)
> +#define __is_sar_start(ctrl)	(((ctrl) & L2CAP_CTRL_SAR) == L2CAP_SDU_START)
>  
>  void l2cap_load(void);
>  
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 12b4aa2..d4b4fbd 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -57,7 +57,7 @@
>  
>  #define VERSION "2.15"
>  
> -static int disable_ertm = 0;
> +static int disable_ertm;

Same issue here.

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

^ permalink raw reply

* Re: [PATCHv2 1/5] Bluetooth: clean up sco code
From: Gustavo F. Padovan @ 2010-12-01 21:20 UTC (permalink / raw)
  To: Emeltchenko Andrei; +Cc: linux-bluetooth
In-Reply-To: <1291215506-11398-2-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

* Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2010-12-01 16:58:22 +0200]:

> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> 
> Do not use assignments in IF condition, remove extra spaces
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> ---
>  include/net/bluetooth/sco.h |    8 ++++----
>  net/bluetooth/sco.c         |   22 +++++++++++++---------
>  2 files changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/include/net/bluetooth/sco.h b/include/net/bluetooth/sco.h
> index e28a2a7..ea5c664 100644
> --- a/include/net/bluetooth/sco.h
> +++ b/include/net/bluetooth/sco.h
> @@ -55,11 +55,11 @@ struct sco_conninfo {
>  struct sco_conn {
>  	struct hci_conn	*hcon;
>  
> -	bdaddr_t 	*dst;
> -	bdaddr_t 	*src;
> -	
> +	bdaddr_t	*dst;
> +	bdaddr_t	*src;
> +
>  	spinlock_t	lock;
> -	struct sock 	*sk;
> +	struct sock	*sk;
>  
>  	unsigned int    mtu;
>  };
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index 66b9e5c..960c6d1 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -44,7 +44,7 @@
>  #include <net/sock.h>
>  
>  #include <asm/system.h>
> -#include <asm/uaccess.h>
> +#include <linux/uaccess.h>
>  
>  #include <net/bluetooth/bluetooth.h>
>  #include <net/bluetooth/hci_core.h>
> @@ -52,7 +52,7 @@
>  
>  #define VERSION "0.6"
>  
> -static int disable_esco = 0;
> +static int disable_esco;

I don't think this change is right. Can we be sure that disable_esco
will be 0 by default?

>  
>  static const struct proto_ops sco_sock_ops;
>  
> @@ -138,16 +138,17 @@ static inline struct sock *sco_chan_get(struct sco_conn *conn)
>  
>  static int sco_conn_del(struct hci_conn *hcon, int err)
>  {
> -	struct sco_conn *conn;
> +	struct sco_conn *conn = hcon->sco_data;
>  	struct sock *sk;
>  
> -	if (!(conn = hcon->sco_data))
> +	if (!conn)
>  		return 0;
>  
>  	BT_DBG("hcon %p conn %p, err %d", hcon, conn, err);
>  
>  	/* Kill socket */
> -	if ((sk = sco_chan_get(conn))) {
> +	sk = sco_chan_get(conn);
> +	if (sk) {
>  		bh_lock_sock(sk);
>  		sco_sock_clear_timer(sk);
>  		sco_chan_del(sk, err);
> @@ -185,7 +186,8 @@ static int sco_connect(struct sock *sk)
>  
>  	BT_DBG("%s -> %s", batostr(src), batostr(dst));
>  
> -	if (!(hdev = hci_get_route(dst, src)))
> +	hdev = hci_get_route(dst, src);
> +	if (!hdev)
>  		return -EHOSTUNREACH;
>  
>  	hci_dev_lock_bh(hdev);
> @@ -510,7 +512,8 @@ static int sco_sock_connect(struct socket *sock, struct sockaddr *addr, int alen
>  	/* Set destination address and psm */
>  	bacpy(&bt_sk(sk)->dst, &sa->sco_bdaddr);
>  
> -	if ((err = sco_connect(sk)))
> +	err = sco_connect(sk);
> +	if (err)
>  		goto done;
>  
>  	err = bt_sock_wait_state(sk, BT_CONNECTED,
> @@ -828,13 +831,14 @@ static void sco_chan_del(struct sock *sk, int err)
>  
>  static void sco_conn_ready(struct sco_conn *conn)
>  {
> -	struct sock *parent, *sk;
> +	struct sock *parent;
> +	struct sock *sk = conn->sk;
>  
>  	BT_DBG("conn %p", conn);
>  
>  	sco_conn_lock(conn);
>  
> -	if ((sk = conn->sk)) {
> +	if (sk) {
>  		sco_sock_clear_timer(sk);
>  		bh_lock_sock(sk);
>  		sk->sk_state = BT_CONNECTED;


Otherwise looks good.

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

^ permalink raw reply

* Re: [RFC 1/4] Bluetooth: clean up sco code
From: Gustavo F. Padovan @ 2010-12-01 21:16 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: Anderson Lizardo, linux-bluetooth
In-Reply-To: <AANLkTikfeMqq-4fTRHwmL81TDnF2=sQsMhL9o7GoEcX2@mail.gmail.com>

Hi Andrei, 

* Andrei Emeltchenko <andrei.emeltchenko.news@gmail.com> [2010-12-01 15:44:50 +0200]:

> Hi,
> 
> On Fri, Nov 26, 2010 at 6:14 PM, Anderson Lizardo
> <anderson.lizardo@openbossa.org> wrote:
> > Hi Andrei,
> >
> > On Fri, Nov 26, 2010 at 11:22 AM, Emeltchenko Andrei
> > <Andrei.Emeltchenko.news@gmail.com> wrote:
> >> @@ -828,13 +831,14 @@ static void sco_chan_del(struct sock *sk, int err)
> >>
> >>  static void sco_conn_ready(struct sco_conn *conn)
> >>  {
> >> -       struct sock *parent, *sk;
> >> +       struct sock *parent;
> >> +       struct sock *sk = conn->sk;
> >
> > I wonder if there is a problem with accessing conn->sk here outside
> > the lock protection?
> 
> I believe here is a safe access. We just acquire pointer and possible
> protected operations
> are executed inside lock.

Agreed.

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

^ permalink raw reply

* pull request: bluetooth-2.6 2010-11-01
From: Gustavo F. Padovan @ 2010-12-01 21:11 UTC (permalink / raw)
  To: linville; +Cc: marcel, linux-wireless, linux-bluetooth

Hi John,

Here goes 3 simple patches intended to 2.6.37. First one is from myself
and fix some trivial return values. Then we also have a fix from Stefan
Seyfried on btusb to avoid emitting an err when we USB is autosupended.
Last, a patch from Bala Shanmugam that fix the firmware loading process
for the ath3k driver.

Please pull, or let me know if you disagree with the patches. Thanks a
lot. ;)


The following changes since commit 916448e77f6bcaaa7f13c3de0c3851783ae2bfd0:

  ath9k: Fix STA disconnect issue due to received MIC failed bcast frames (2010-11-30 13:45:02 -0500)

are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/padovan/bluetooth-2.6.git master

Bala Shanmugam (1):
      Bluetooth: Add new PID for Atheros 3011

Gustavo F. Padovan (1):
      Bluetooth: Fix not returning proper error in SCO

Stefan Seyfried (1):
      Bluetooth: Fix log spamming in btusb due to autosuspend

 drivers/bluetooth/ath3k.c |    4 ++++
 drivers/bluetooth/btusb.c |   12 +++++++++---
 net/bluetooth/sco.c       |    6 +++---
 3 files changed, 16 insertions(+), 6 deletions(-)

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

^ permalink raw reply

* Re: [PATCH v3] Bluetooth: Add new PID for Atheros 3011
From: Gustavo F. Padovan @ 2010-12-01 17:50 UTC (permalink / raw)
  To: Bala Shanmugam; +Cc: linux-bluetooth
In-Reply-To: <1290773146-4705-1-git-send-email-sbalashanmugam@atheros.com>

Hi Bala,

* Bala Shanmugam <sbalashanmugam@atheros.com> [2010-11-26 17:35:46 +0530]:

> Atheros 3011 has small sflash firmware and needs to be
> blacklisted in transport driver to load actual firmware
> in DFU driver.
> 
> Signed-off-by: Bala Shanmugam <sbalashanmugam@atheros.com>
> ---
>  drivers/bluetooth/ath3k.c |    4 ++++
>  drivers/bluetooth/btusb.c |    3 +++
>  2 files changed, 7 insertions(+), 0 deletions(-)

Applied, thanks.

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

^ permalink raw reply

* Re: [PATCH] btusb: Fix log spamming due to autosuspend
From: Gustavo F. Padovan @ 2010-12-01 17:49 UTC (permalink / raw)
  To: Stefan Seyfried
  Cc: Marcel Holtmann, linux-bluetooth, gregkh, Stefan Seyfried,
	Oliver Neukum
In-Reply-To: <20101201154714.517289f7@susi>

Hi Stefan,

* Stefan Seyfried <stefan.seyfried@googlemail.com> [2010-12-01 15:47:14 +0100]:

> Hi Marcel,
> 
> On Wed, 01 Dec 2010 11:42:20 +0100
> Marcel Holtmann <marcel@holtmann.org> wrote:
> 
> > Hi Stefan,
> > 
> > > If a device is autosuspended an inability to resubmit URBs is
> > > to be expected. Check the error code and only log real errors.
> > > (Now that autosuspend is default enabled for btusb, those log
> > > messages were happening all the time e.g. with a BT mouse)
> > > 
> > > Signed-off-by: Stefan Seyfried <seife+kernel@b1-systems.com>
> > > Signed-off-by: Oliver Neukum <oneukum@suse.de>
> > 
> > we had a similar one some time ago, but I am fine with this one as well.
> > Actually this one might be a bit better since it still keeps some
> > errors.
> > 
> > Acked-by: Marcel Holtmann <marcel@holtmann.org>
> 
> Could you (or Gustavo) send it to Linus? It's pretty trivial, but the
> messages are annoying and users will complain if they are still in 2.6.37
> final.

I'll send it, applied to bluetooth-2.6 tree. Thanks.

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

^ permalink raw reply

* not able to connect mobile with laptop using bluetooth
From: Rahul Ruikar @ 2010-12-01 16:54 UTC (permalink / raw)
  To: linux-bluetooth

Hi,

I am not able to pair my mobile with laptop.
when I try to pair it..it times out very soonwhile tying "pin"
i get following error
Dec  1 22:17:38 ganesha bluetoothd[1094]: Discovery session 0xb8fc95b8
with :1.73 activated
Dec  1 22:17:48 ganesha bluetoothd[1094]: Stopping discovery
Dec  1 22:17:50 ganesha bluetoothd[1094]: link_key_request
(sba=00:16:CF:FB:D4:99, dba=00:23:AF:D7:53:20)
Dec  1 22:17:50 ganesha bluetoothd[1094]: pin_code_request
(sba=00:16:CF:FB:D4:99, dba=00:23:AF:D7:53:20)
Dec  1 22:18:01 ganesha NetworkManager[914]: <info> BT device
00:23:AF:D7:53:20 removed
Dec  1 22:18:01 ganesha bluetoothd[1094]: Stopping discovery

sometimes i get this error if i use option "Send files to device"
Dec  1 20:13:48 ganesha bluetoothd[1267]: link_key_notify
(sba=00:16:CF:FB:D4:99, dba=00:23:AF:D7:53:20, type=0)
Dec  1 20:13:59 ganesha bluetoothd[1267]: probe failed with driver
input-headset for device /org/bluez/1252/hci0/dev_00_23_AF_D7_53_20
Dec  1 20:13:59 ganesha bluetoothd[1267]: Stopping discovery
Dec  1 20:14:06 ganesha kernel: btusb_intr_complete: hci0 urb d15cf580
failed to resubmit (1)
Dec  1 20:14:06 ganesha kernel: btusb_bulk_complete: hci0 urb d15cf080
failed to resubmit (1)
Dec  1 20:14:06 ganesha kernel: btusb_bulk_complete: hci0 urb d15cf480
failed to resubmit (1)
Dec  1 20:14:24 ganesha kernel: btusb_intr_complete: hci0 urb d1491a00
failed to resubmit (1)
Dec  1 20:14:24 ganesha kernel: btusb_bulk_complete: hci0 urb d1491300
failed to resubmit (1)
Dec  1 20:14:24 ganesha kernel: btusb_bulk_complete: hci0 urb d1491b80
failed to resubmit (1)
Dec  1 20:14:37 ganesha kernel: btusb_intr_complete: hci0 urb f6683400
failed to resubmit (1)
Dec  1 20:14:37 ganesha kernel: btusb_bulk_complete: hci0 urb f6683200
failed to resubmit (1)
Dec  1 20:14:37 ganesha kernel: btusb_bulk_complete: hci0 urb f6683180
failed to resubmit (1)


any idea what could be wrong ..also why it says "input-headset"

Thanks,
Rahul

^ permalink raw reply

* [PATCH 3/3] Check authentication permissions on attribute server
From: Anderson Lizardo @ 2010-12-01 16:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1291219986-29913-1-git-send-email-anderson.lizardo@openbossa.org>

Attributes may require encryption for certain operations. This commit
adds checks to the attribute server which verify whether the current
connection is encrypted (currently done by checking the security level,
but may be changed later) and the attribute being accessed requires
authentication. If encryption requirements are not satisfied, the
"Insufficient Encryption" error is returned by the server.
---
 src/attrib-server.c |   76 +++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 59 insertions(+), 17 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index 32357f0..fee5462 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -57,6 +57,7 @@ struct gatt_channel {
 	GAttrib *attrib;
 	guint mtu;
 	guint id;
+	uint8_t perms;
 };
 
 struct group_elem {
@@ -138,8 +139,11 @@ static sdp_record_t *server_record_new(void)
 	return record;
 }
 
-static uint8_t att_check_perms(uint8_t opcode, uint8_t perms)
+static uint8_t att_check_perms(struct gatt_channel *channel, uint8_t opcode,
+								uint8_t perms)
 {
+	uint8_t chp = channel->perms;
+
 	switch (opcode) {
 	case ATT_OP_READ_BY_GROUP_REQ:
 	case ATT_OP_READ_BY_TYPE_REQ:
@@ -148,20 +152,41 @@ static uint8_t att_check_perms(uint8_t opcode, uint8_t perms)
 	case ATT_OP_READ_MULTI_REQ:
 		if (!(perms & ATT_ACCESS(ATT_READ, ATT_NONE)))
 			return ATT_ECODE_READ_NOT_PERM;
+
+		if ((perms & ATT_ACCESS(ATT_READ, ATT_AUTHENTICATION)) !=
+				ATT_ACCESS(ATT_READ, ATT_AUTHENTICATION))
+			/* Attribute does not require authentication for read */
+			return 0;
+
+		if ((chp & ATT_ACCESS(ATT_READ, ATT_AUTHENTICATION)) !=
+				ATT_ACCESS(ATT_READ, ATT_AUTHENTICATION))
+			/* Connection is not encrypted */
+			return ATT_ECODE_INSUFF_AUTHEN;
 		break;
 	case ATT_OP_PREP_WRITE_REQ:
 	case ATT_OP_WRITE_REQ:
 	case ATT_OP_WRITE_CMD:
 		if (!(perms & ATT_ACCESS(ATT_WRITE, ATT_NONE)))
 			return ATT_ECODE_WRITE_NOT_PERM;
+
+		if ((perms & ATT_ACCESS(ATT_WRITE, ATT_AUTHENTICATION)) !=
+				ATT_ACCESS(ATT_WRITE, ATT_AUTHENTICATION))
+			/* Attribute does not require authentication for write */
+			return 0;
+
+		if ((chp & ATT_ACCESS(ATT_WRITE, ATT_AUTHENTICATION)) !=
+				ATT_ACCESS(ATT_WRITE, ATT_AUTHENTICATION))
+			/* Connection is not encrypted */
+			return ATT_ECODE_INSUFF_AUTHEN;
 		break;
 	}
 
 	return 0;
 }
 
-static uint16_t read_by_group(uint16_t start, uint16_t end, uuid_t *uuid,
-							uint8_t *pdu, int len)
+static uint16_t read_by_group(struct gatt_channel *channel, uint16_t start,
+						uint16_t end, uuid_t *uuid,
+						uint8_t *pdu, int len)
 {
 	struct att_data_list *adl;
 	struct attribute *a;
@@ -212,7 +237,8 @@ static uint16_t read_by_group(uint16_t start, uint16_t end, uuid_t *uuid,
 		if (last_size && (last_size != a->len))
 			break;
 
-		status = att_check_perms(ATT_OP_READ_BY_GROUP_REQ, a->perms);
+		status = att_check_perms(channel, ATT_OP_READ_BY_GROUP_REQ,
+								a->perms);
 		if (status) {
 			g_slist_foreach(groups, (GFunc) g_free, NULL);
 			g_slist_free(groups);
@@ -271,8 +297,9 @@ static uint16_t read_by_group(uint16_t start, uint16_t end, uuid_t *uuid,
 	return length;
 }
 
-static uint16_t read_by_type(uint16_t start, uint16_t end, uuid_t *uuid,
-							uint8_t *pdu, int len)
+static uint16_t read_by_type(struct gatt_channel *channel, uint16_t start,
+						uint16_t end, uuid_t *uuid,
+						uint8_t *pdu, int len)
 {
 	struct att_data_list *adl;
 	GSList *l, *types;
@@ -297,7 +324,8 @@ static uint16_t read_by_type(uint16_t start, uint16_t end, uuid_t *uuid,
 		if (sdp_uuid_cmp(&a->uuid, uuid)  != 0)
 			continue;
 
-		status = att_check_perms(ATT_OP_READ_BY_TYPE_REQ, a->perms);
+		status = att_check_perms(channel, ATT_OP_READ_BY_TYPE_REQ,
+								a->perms);
 		if (status) {
 			g_slist_free(types);
 			return enc_error_resp(ATT_OP_READ_BY_TYPE_REQ,
@@ -508,7 +536,8 @@ static int attribute_cmp(gconstpointer a1, gconstpointer a2)
 	return attrib1->handle - attrib2->handle;
 }
 
-static uint16_t read_value(uint16_t handle, uint8_t *pdu, int len)
+static uint16_t read_value(struct gatt_channel *channel, uint16_t handle,
+							uint8_t *pdu, int len)
 {
 	struct attribute *a;
 	uint8_t status;
@@ -522,7 +551,7 @@ static uint16_t read_value(uint16_t handle, uint8_t *pdu, int len)
 
 	a = l->data;
 
-	status = att_check_perms(ATT_OP_READ_REQ, a->perms);
+	status = att_check_perms(channel, ATT_OP_READ_REQ, a->perms);
 	if (status)
 		return enc_error_resp(ATT_OP_READ_REQ, handle, status, pdu,
 									len);
@@ -530,8 +559,9 @@ static uint16_t read_value(uint16_t handle, uint8_t *pdu, int len)
 	return enc_read_resp(a->data, a->len, pdu, len);
 }
 
-static uint16_t write_value(uint16_t handle, const uint8_t *value, int vlen,
-							uint8_t *pdu, int len)
+static uint16_t write_value(struct gatt_channel *channel, uint16_t handle,
+						const uint8_t *value, int vlen,
+						uint8_t *pdu, int len)
 {
 	struct attribute *a;
 	uint8_t status;
@@ -546,7 +576,7 @@ static uint16_t write_value(uint16_t handle, const uint8_t *value, int vlen,
 
 	a = l->data;
 
-	status = att_check_perms(ATT_OP_WRITE_REQ, a->perms);
+	status = att_check_perms(channel, ATT_OP_WRITE_REQ, a->perms);
 	if (status)
 		return enc_error_resp(ATT_OP_WRITE_REQ, handle, status, pdu,
 									len);
@@ -595,7 +625,8 @@ static void channel_handler(const uint8_t *ipdu, uint16_t len,
 			goto done;
 		}
 
-		length = read_by_group(start, end, &uuid, opdu, channel->mtu);
+		length = read_by_group(channel, start, end, &uuid, opdu,
+								channel->mtu);
 		break;
 	case ATT_OP_READ_BY_TYPE_REQ:
 		length = dec_read_by_type_req(ipdu, len, &start, &end, &uuid);
@@ -604,7 +635,8 @@ static void channel_handler(const uint8_t *ipdu, uint16_t len,
 			goto done;
 		}
 
-		length = read_by_type(start, end, &uuid, opdu, channel->mtu);
+		length = read_by_type(channel, start, end, &uuid, opdu,
+								channel->mtu);
 		break;
 	case ATT_OP_READ_REQ:
 		length = dec_read_req(ipdu, len, &start);
@@ -613,7 +645,7 @@ static void channel_handler(const uint8_t *ipdu, uint16_t len,
 			goto done;
 		}
 
-		length = read_value(start, opdu, channel->mtu);
+		length = read_value(channel, start, opdu, channel->mtu);
 		break;
 	case ATT_OP_MTU_REQ:
 		length = dec_mtu_req(ipdu, len, &mtu);
@@ -640,12 +672,14 @@ static void channel_handler(const uint8_t *ipdu, uint16_t len,
 			goto done;
 		}
 
-		length = write_value(start, value, vlen, opdu, channel->mtu);
+		length = write_value(channel, start, value, vlen, opdu,
+								channel->mtu);
 		break;
 	case ATT_OP_WRITE_CMD:
 		length = dec_write_cmd(ipdu, len, &start, value, &vlen);
 		if (length > 0)
-			write_value(start, value, vlen, opdu, channel->mtu);
+			write_value(channel, start, value, vlen, opdu,
+								channel->mtu);
 		return;
 	case ATT_OP_FIND_BY_TYPE_REQ:
 		length = dec_find_by_type_req(ipdu, len, &start, &end,
@@ -682,6 +716,7 @@ static void connect_event(GIOChannel *io, GError *err, void *user_data)
 {
 	struct gatt_channel *channel;
 	GError *gerr = NULL;
+	int sec_level;
 
 	if (err) {
 		error("%s", err->message);
@@ -693,6 +728,7 @@ static void connect_event(GIOChannel *io, GError *err, void *user_data)
 	bt_io_get(io, BT_IO_L2CAP, &gerr,
 			BT_IO_OPT_SOURCE_BDADDR, &channel->src,
 			BT_IO_OPT_DEST_BDADDR, &channel->dst,
+			BT_IO_OPT_SEC_LEVEL, &sec_level,
 			BT_IO_OPT_INVALID);
 	if (gerr) {
 		error("bt_io_get: %s", gerr->message);
@@ -705,6 +741,12 @@ static void connect_event(GIOChannel *io, GError *err, void *user_data)
 	channel->attrib = g_attrib_new(io);
 	channel->mtu = ATT_DEFAULT_MTU;
 
+	if (sec_level > BT_IO_SEC_LOW)
+		channel->perms = ATT_ACCESS(ATT_READ | ATT_WRITE,
+							ATT_AUTHENTICATION);
+	else
+		channel->perms = ATT_ACCESS(ATT_READ | ATT_WRITE, ATT_NONE);
+
 	channel->id = g_attrib_register(channel->attrib, GATTRIB_ALL_EVENTS,
 				channel_handler, channel, NULL);
 
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 2/3] Check attribute permissions in attribute server
From: Anderson Lizardo @ 2010-12-01 16:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bruna Moreira
In-Reply-To: <1291219986-29913-1-git-send-email-anderson.lizardo@openbossa.org>

From: Bruna Moreira <bruna.moreira@openbossa.org>

The attribute server must verify if the operation (read/write) is
permitted before running the request, and send "Read Not Permitted" or
"Write Not Permitted" error response to client if appropriate.
---
 TODO                |    7 -----
 src/attrib-server.c |   68 ++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 62 insertions(+), 13 deletions(-)

diff --git a/TODO b/TODO
index 49a9e76..32d3a61 100644
--- a/TODO
+++ b/TODO
@@ -136,13 +136,6 @@ ATT/GATT
   Priority: Low
   Complexity: C2
 
-- Attribute server shall implement attribute permission verification,
-  returning an error code if necessary. See Volume 3, Part F, 3.2.5
-  for more information.
-
-  Priority: Low
-  Complexity: C2
-
 - Implement Client Characteristic Configuration support in the attribute
   server to manage indications and notications. This is a per client attribute
   to control how the client wants to receive reports of changes in a given
diff --git a/src/attrib-server.c b/src/attrib-server.c
index 4199355..32357f0 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -138,6 +138,28 @@ static sdp_record_t *server_record_new(void)
 	return record;
 }
 
+static uint8_t att_check_perms(uint8_t opcode, uint8_t perms)
+{
+	switch (opcode) {
+	case ATT_OP_READ_BY_GROUP_REQ:
+	case ATT_OP_READ_BY_TYPE_REQ:
+	case ATT_OP_READ_REQ:
+	case ATT_OP_READ_BLOB_REQ:
+	case ATT_OP_READ_MULTI_REQ:
+		if (!(perms & ATT_ACCESS(ATT_READ, ATT_NONE)))
+			return ATT_ECODE_READ_NOT_PERM;
+		break;
+	case ATT_OP_PREP_WRITE_REQ:
+	case ATT_OP_WRITE_REQ:
+	case ATT_OP_WRITE_CMD:
+		if (!(perms & ATT_ACCESS(ATT_WRITE, ATT_NONE)))
+			return ATT_ECODE_WRITE_NOT_PERM;
+		break;
+	}
+
+	return 0;
+}
+
 static uint16_t read_by_group(uint16_t start, uint16_t end, uuid_t *uuid,
 							uint8_t *pdu, int len)
 {
@@ -146,6 +168,7 @@ static uint16_t read_by_group(uint16_t start, uint16_t end, uuid_t *uuid,
 	struct group_elem *cur, *old = NULL;
 	GSList *l, *groups;
 	uint16_t length, last_handle, last_size = 0;
+	uint8_t status;
 	int i;
 
 	if (start > end || start == 0x0000)
@@ -189,6 +212,14 @@ static uint16_t read_by_group(uint16_t start, uint16_t end, uuid_t *uuid,
 		if (last_size && (last_size != a->len))
 			break;
 
+		status = att_check_perms(ATT_OP_READ_BY_GROUP_REQ, a->perms);
+		if (status) {
+			g_slist_foreach(groups, (GFunc) g_free, NULL);
+			g_slist_free(groups);
+			return enc_error_resp(ATT_OP_READ_BY_GROUP_REQ,
+						a->handle, status, pdu, len);
+		}
+
 		cur = g_new0(struct group_elem, 1);
 		cur->handle = a->handle;
 		cur->data = a->data;
@@ -247,6 +278,7 @@ static uint16_t read_by_type(uint16_t start, uint16_t end, uuid_t *uuid,
 	GSList *l, *types;
 	struct attribute *a;
 	uint16_t num, length;
+	uint8_t status;
 	int i;
 
 	if (start > end || start == 0x0000)
@@ -265,6 +297,13 @@ static uint16_t read_by_type(uint16_t start, uint16_t end, uuid_t *uuid,
 		if (sdp_uuid_cmp(&a->uuid, uuid)  != 0)
 			continue;
 
+		status = att_check_perms(ATT_OP_READ_BY_TYPE_REQ, a->perms);
+		if (status) {
+			g_slist_free(types);
+			return enc_error_resp(ATT_OP_READ_BY_TYPE_REQ,
+						a->handle, status, pdu, len);
+		}
+
 		/* All elements must have the same length */
 		if (length == 0)
 			length = a->len;
@@ -472,6 +511,7 @@ static int attribute_cmp(gconstpointer a1, gconstpointer a2)
 static uint16_t read_value(uint16_t handle, uint8_t *pdu, int len)
 {
 	struct attribute *a;
+	uint8_t status;
 	GSList *l;
 	guint h = handle;
 
@@ -482,23 +522,41 @@ static uint16_t read_value(uint16_t handle, uint8_t *pdu, int len)
 
 	a = l->data;
 
+	status = att_check_perms(ATT_OP_READ_REQ, a->perms);
+	if (status)
+		return enc_error_resp(ATT_OP_READ_REQ, handle, status, pdu,
+									len);
+
 	return enc_read_resp(a->data, a->len, pdu, len);
 }
 
-static void write_value(uint16_t handle, const uint8_t *value, int vlen)
+static uint16_t write_value(uint16_t handle, const uint8_t *value, int vlen,
+							uint8_t *pdu, int len)
 {
 	struct attribute *a;
+	uint8_t status;
 	GSList *l;
 	guint h = handle;
 	uuid_t uuid;
 
 	l = g_slist_find_custom(database, GUINT_TO_POINTER(h), handle_cmp);
 	if (!l)
-		return;
+		return enc_error_resp(ATT_OP_WRITE_REQ, handle,
+				ATT_ECODE_INVALID_HANDLE, pdu, len);
 
 	a = l->data;
+
+	status = att_check_perms(ATT_OP_WRITE_REQ, a->perms);
+	if (status)
+		return enc_error_resp(ATT_OP_WRITE_REQ, handle, status, pdu,
+									len);
+
 	memcpy(&uuid, &a->uuid, sizeof(uuid_t));
 	attrib_db_update(handle, &uuid, value, vlen);
+
+	pdu[0] = ATT_OP_WRITE_RESP;
+
+	return sizeof(pdu[0]);
 }
 
 static uint16_t mtu_exchange(struct gatt_channel *channel, uint16_t mtu,
@@ -582,14 +640,12 @@ static void channel_handler(const uint8_t *ipdu, uint16_t len,
 			goto done;
 		}
 
-		write_value(start, value, vlen);
-		opdu[0] = ATT_OP_WRITE_RESP;
-		length = sizeof(opdu[0]);
+		length = write_value(start, value, vlen, opdu, channel->mtu);
 		break;
 	case ATT_OP_WRITE_CMD:
 		length = dec_write_cmd(ipdu, len, &start, value, &vlen);
 		if (length > 0)
-			write_value(start, value, vlen);
+			write_value(start, value, vlen, opdu, channel->mtu);
 		return;
 	case ATT_OP_FIND_BY_TYPE_REQ:
 		length = dec_find_by_type_req(ipdu, len, &start, &end,
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 1/3] Initial attribute permission implementation
From: Anderson Lizardo @ 2010-12-01 16:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bruna Moreira
In-Reply-To: <1291219986-29913-1-git-send-email-anderson.lizardo@openbossa.org>

From: Bruna Moreira <bruna.moreira@openbossa.org>

Create definitions for attribute permissions, add "perms" field to
struct attribute and add attribute permissions to attribute server.
---
 attrib/att.h        |    1 +
 attrib/example.c    |   89 ++++++++++++++++++++++++++++-----------------------
 src/attrib-server.c |    4 ++-
 src/attrib-server.h |   13 +++++++-
 4 files changed, 65 insertions(+), 42 deletions(-)

diff --git a/attrib/att.h b/attrib/att.h
index 7c98b4a..a29aae2 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -112,6 +112,7 @@
 struct attribute {
 	uint16_t handle;
 	uuid_t uuid;
+	uint8_t perms;
 	int len;
 	uint8_t data[0];
 };
diff --git a/attrib/example.c b/attrib/example.c
index c29e1e4..2fedfc7 100644
--- a/attrib/example.c
+++ b/attrib/example.c
@@ -101,7 +101,7 @@ static int register_attributes(void)
 	u16 = htons(GENERIC_ACCESS_PROFILE_ID);
 	atval[0] = u16 >> 8;
 	atval[1] = u16;
-	attrib_db_add(0x0001, &uuid, atval, 2);
+	attrib_db_add(0x0001, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 2);
 
 	/* GAP service: device name characteristic */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
@@ -111,20 +111,21 @@ static int register_attributes(void)
 	atval[2] = 0x00;
 	atval[3] = u16 >> 8;
 	atval[4] = u16;
-	attrib_db_add(0x0004, &uuid, atval, 5);
+	attrib_db_add(0x0004, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 5);
 
 	/* GAP service: device name attribute */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_DEVICE_NAME);
 	len = strlen(devname);
 	strncpy((char *) atval, devname, len);
-	attrib_db_add(0x0006, &uuid, atval, len);
+	attrib_db_add(0x0006, &uuid, ATT_ACCESS(ATT_READ | ATT_WRITE, ATT_NONE),
+								atval, len);
 
 	/* GATT service: primary service definition */
 	sdp_uuid16_create(&uuid, GATT_PRIM_SVC_UUID);
 	u16 = htons(GENERIC_ATTRIB_PROFILE_ID);
 	atval[0] = u16 >> 8;
 	atval[1] = u16;
-	attrib_db_add(0x0010, &uuid, atval, 2);
+	attrib_db_add(0x0010, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 2);
 
 	/* GATT service: attributes opcodes characteristic */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
@@ -134,20 +135,20 @@ static int register_attributes(void)
 	atval[2] = 0x00;
 	atval[3] = u16 >> 8;
 	atval[4] = u16;
-	attrib_db_add(0x0011, &uuid, atval, 5);
+	attrib_db_add(0x0011, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 5);
 
 	/* GATT service: attribute opcodes supported */
 	sdp_uuid16_create(&uuid, OPCODES_SUPPORTED_UUID);
 	atval[0] = 0xFF;
 	atval[1] = 0x01;
-	attrib_db_add(0x0012, &uuid, atval, 2);
+	attrib_db_add(0x0012, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 2);
 
 	/* Battery state service: primary service definition */
 	sdp_uuid16_create(&uuid, GATT_PRIM_SVC_UUID);
 	u16 = htons(BATTERY_STATE_SVC_UUID);
 	atval[0] = u16 >> 8;
 	atval[1] = u16;
-	attrib_db_add(0x0100, &uuid, atval, 2);
+	attrib_db_add(0x0100, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 2);
 
 	/* Battery: battery state characteristic */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
@@ -157,18 +158,19 @@ static int register_attributes(void)
 	atval[2] = 0x01;
 	atval[3] = u16 >> 8;
 	atval[4] = u16;
-	attrib_db_add(0x0106, &uuid, atval, 5);
+	attrib_db_add(0x0106, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 5);
 
 	/* Battery: battery state attribute */
 	sdp_uuid16_create(&uuid, BATTERY_STATE_UUID);
 	atval[0] = 0x04;
-	attrib_db_add(0x0110, &uuid, atval, 1);
+	attrib_db_add(0x0110, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 1);
 
 	/* Battery: Client Characteristic Configuration */
 	sdp_uuid16_create(&uuid, GATT_CLIENT_CHARAC_CFG_UUID);
 	atval[0] = 0x00;
 	atval[1] = 0x00;
-	attrib_db_add(0x0111, &uuid, atval, 2);
+	attrib_db_add(0x0111, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE) |
+			ATT_ACCESS(ATT_WRITE, ATT_AUTHENTICATION), atval, 2);
 
 	timeout_id = g_timeout_add_seconds(10, change_battery_state, NULL);
 
@@ -177,7 +179,7 @@ static int register_attributes(void)
 	u16 = htons(THERM_HUMIDITY_SVC_UUID);
 	atval[0] = u16 >> 8;
 	atval[1] = u16;
-	attrib_db_add(0x0200, &uuid, atval, 2);
+	attrib_db_add(0x0200, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 2);
 
 	/* Thermometer: Include */
 	sdp_uuid16_create(&uuid, GATT_INCLUDE_UUID);
@@ -188,14 +190,14 @@ static int register_attributes(void)
 	atval[3] = 0x05;
 	atval[4] = u16 >> 8;
 	atval[5] = u16;
-	attrib_db_add(0x0201, &uuid, atval, 6);
+	attrib_db_add(0x0201, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 6);
 
 	/* Thermometer: Include */
 	atval[0] = 0x50;
 	atval[1] = 0x05;
 	atval[2] = 0x68;
 	atval[3] = 0x05;
-	attrib_db_add(0x0202, &uuid, atval, 4);
+	attrib_db_add(0x0202, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 4);
 
 	/* Thermometer: temperature characteristic */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
@@ -205,13 +207,13 @@ static int register_attributes(void)
 	atval[2] = 0x02;
 	atval[3] = u16 >> 8;
 	atval[4] = u16;
-	attrib_db_add(0x0203, &uuid, atval, 5);
+	attrib_db_add(0x0203, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 5);
 
 	/* Thermometer: temperature characteristic value */
 	sdp_uuid16_create(&uuid, TEMPERATURE_UUID);
 	atval[0] = 0x8A;
 	atval[1] = 0x02;
-	attrib_db_add(0x0204, &uuid, atval, 2);
+	attrib_db_add(0x0204, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 2);
 
 	/* Thermometer: temperature characteristic format */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_FMT_UUID);
@@ -224,13 +226,14 @@ static int register_attributes(void)
 	u16 = htons(FMT_OUTSIDE_UUID);
 	atval[5] = u16 >> 8;
 	atval[6] = u16;
-	attrib_db_add(0x0205, &uuid, atval, 7);
+	attrib_db_add(0x0205, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 7);
 
 	/* Thermometer: characteristic user description */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_USER_DESC_UUID);
 	len = strlen(desc_out_temp);
 	strncpy((char *) atval, desc_out_temp, len);
-	attrib_db_add(0x0206, &uuid, atval, len);
+	attrib_db_add(0x0206, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval,
+									len);
 
 	/* Thermometer: relative humidity characteristic */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
@@ -240,12 +243,12 @@ static int register_attributes(void)
 	atval[2] = 0x02;
 	atval[3] = u16 >> 8;
 	atval[4] = u16;
-	attrib_db_add(0x0210, &uuid, atval, 5);
+	attrib_db_add(0x0210, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 5);
 
 	/* Thermometer: relative humidity value */
 	sdp_uuid16_create(&uuid, RELATIVE_HUMIDITY_UUID);
 	atval[0] = 0x27;
-	attrib_db_add(0x0212, &uuid, atval, 1);
+	attrib_db_add(0x0212, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 1);
 
 	/* Thermometer: relative humidity characteristic format */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_FMT_UUID);
@@ -260,20 +263,21 @@ static int register_attributes(void)
 	u16 = htons(FMT_OUTSIDE_UUID);
 	atval[6] = u16 >> 8;
 	atval[7] = u16;
-	attrib_db_add(0x0213, &uuid, atval, 8);
+	attrib_db_add(0x0213, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 8);
 
 	/* Thermometer: characteristic user description */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_USER_DESC_UUID);
 	len = strlen(desc_out_hum);
 	strncpy((char *) atval, desc_out_hum, len);
-	attrib_db_add(0x0214, &uuid, atval, len);
+	attrib_db_add(0x0214, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval,
+									len);
 
 	/* Secondary Service: Manufacturer Service */
 	sdp_uuid16_create(&uuid, GATT_SND_SVC_UUID);
 	u16 = htons(MANUFACTURER_SVC_UUID);
 	atval[0] = u16 >> 8;
 	atval[1] = u16;
-	attrib_db_add(0x0500, &uuid, atval, 2);
+	attrib_db_add(0x0500, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 2);
 
 	/* Manufacturer name characteristic definition */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
@@ -283,13 +287,14 @@ static int register_attributes(void)
 	atval[2] = 0x05;
 	atval[3] = u16 >> 8;
 	atval[4] = u16;
-	attrib_db_add(0x0501, &uuid, atval, 5);
+	attrib_db_add(0x0501, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 5);
 
 	/* Manufacturer name characteristic value */
 	sdp_uuid16_create(&uuid, MANUFACTURER_NAME_UUID);
 	len = strlen(manufacturer_name1);
 	strncpy((char *) atval, manufacturer_name1, len);
-	attrib_db_add(0x0502, &uuid, atval, len);
+	attrib_db_add(0x0502, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval,
+									len);
 
 	/* Manufacturer serial number characteristic */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
@@ -299,20 +304,21 @@ static int register_attributes(void)
 	atval[2] = 0x05;
 	atval[3] = u16 >> 8;
 	atval[4] = u16;
-	attrib_db_add(0x0503, &uuid, atval, 5);
+	attrib_db_add(0x0503, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 5);
 
 	/* Manufacturer serial number characteristic value */
 	sdp_uuid16_create(&uuid, MANUFACTURER_SERIAL_UUID);
 	len = strlen(serial1);
 	strncpy((char *) atval, serial1, len);
-	attrib_db_add(0x0504, &uuid, atval, len);
+	attrib_db_add(0x0504, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval,
+									len);
 
 	/* Secondary Service: Manufacturer Service */
 	sdp_uuid16_create(&uuid, GATT_SND_SVC_UUID);
 	u16 = htons(MANUFACTURER_SVC_UUID);
 	atval[0] = u16 >> 8;
 	atval[1] = u16;
-	attrib_db_add(0x0505, &uuid, atval, 2);
+	attrib_db_add(0x0505, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 2);
 
 	/* Manufacturer name characteristic definition */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
@@ -322,14 +328,14 @@ static int register_attributes(void)
 	atval[2] = 0x05;
 	atval[3] = u16 >> 8;
 	atval[4] = u16;
-	attrib_db_add(0x0506, &uuid, atval, 5);
+	attrib_db_add(0x0506, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 5);
 
 	/* Secondary Service: Vendor Specific Service */
 	sdp_uuid16_create(&uuid, GATT_SND_SVC_UUID);
 	u16 = htons(VENDOR_SPECIFIC_SVC_UUID);
 	atval[0] = u16 >> 8;
 	atval[1] = u16;
-	attrib_db_add(0x0550, &uuid, atval, 2);
+	attrib_db_add(0x0550, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 2);
 
 	/* Vendor Specific Type characteristic definition */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
@@ -339,7 +345,7 @@ static int register_attributes(void)
 	atval[2] = 0x05;
 	atval[3] = u16 >> 8;
 	atval[4] = u16;
-	attrib_db_add(0x0560, &uuid, atval, 5);
+	attrib_db_add(0x0560, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 5);
 
 	/* Vendor Specific Type characteristic value */
 	sdp_uuid16_create(&uuid, VENDOR_SPECIFIC_TYPE_UUID);
@@ -349,13 +355,14 @@ static int register_attributes(void)
 	atval[3] = 0x64;
 	atval[4] = 0x6F;
 	atval[5] = 0x72;
-	attrib_db_add(0x0568, &uuid, atval, 6);
+	attrib_db_add(0x0568, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 6);
 
 	/* Manufacturer name attribute */
 	sdp_uuid16_create(&uuid, MANUFACTURER_NAME_UUID);
 	len = strlen(manufacturer_name2);
 	strncpy((char *) atval, manufacturer_name2, len);
-	attrib_db_add(0x0507, &uuid, atval, len);
+	attrib_db_add(0x0507, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval,
+									len);
 
 	/* Characteristic: serial number */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
@@ -365,18 +372,19 @@ static int register_attributes(void)
 	atval[2] = 0x05;
 	atval[3] = u16 >> 8;
 	atval[4] = u16;
-	attrib_db_add(0x0508, &uuid, atval, 5);
+	attrib_db_add(0x0508, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 5);
 
 	/* Serial number characteristic value */
 	sdp_uuid16_create(&uuid, MANUFACTURER_SERIAL_UUID);
 	len = strlen(serial2);
 	strncpy((char *) atval, serial2, len);
-	attrib_db_add(0x0509, &uuid, atval, len);
+	attrib_db_add(0x0509, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval,
+									len);
 
 	/* Weight service: primary service definition */
 	sdp_uuid16_create(&uuid, GATT_PRIM_SVC_UUID);
 	memcpy(atval, prim_weight_uuid, 16);
-	attrib_db_add(0x0680, &uuid, atval, 16);
+	attrib_db_add(0x0680, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 16);
 
 	/* Weight: include */
 	sdp_uuid16_create(&uuid, GATT_INCLUDE_UUID);
@@ -387,7 +395,7 @@ static int register_attributes(void)
 	atval[3] = 0x05;
 	atval[4] = u16 >> 8;
 	atval[5] = u16;
-	attrib_db_add(0x0681, &uuid, atval, 6);
+	attrib_db_add(0x0681, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 6);
 
 	/* Weight: characteristic */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
@@ -395,7 +403,7 @@ static int register_attributes(void)
 	atval[1] = 0x83;
 	atval[2] = 0x06;
 	memcpy(atval + 3, char_weight_uuid, 16);
-	attrib_db_add(0x0682, &uuid, atval, 19);
+	attrib_db_add(0x0682, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 19);
 
 	/* Weight: characteristic value */
 	sdp_uuid128_create(&uuid, char_weight_uuid);
@@ -403,7 +411,7 @@ static int register_attributes(void)
 	atval[1] = 0x55;
 	atval[2] = 0x00;
 	atval[3] = 0x00;
-	attrib_db_add(0x0683, &uuid, atval, 4);
+	attrib_db_add(0x0683, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 4);
 
 	/* Weight: characteristic format */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_FMT_UUID);
@@ -418,13 +426,14 @@ static int register_attributes(void)
 	u16 = htons(FMT_HANGING_UUID);
 	atval[6] = u16 >> 8;
 	atval[7] = u16;
-	attrib_db_add(0x0684, &uuid, atval, 8);
+	attrib_db_add(0x0684, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval, 8);
 
 	/* Weight: characteristic user description */
 	sdp_uuid16_create(&uuid, GATT_CHARAC_USER_DESC_UUID);
 	len = strlen(desc_weight);
 	strncpy((char *) atval, desc_weight, len);
-	attrib_db_add(0x0685, &uuid, atval, len);
+	attrib_db_add(0x0685, &uuid, ATT_ACCESS(ATT_READ, ATT_NONE), atval,
+									len);
 
 	return 0;
 }
diff --git a/src/attrib-server.c b/src/attrib-server.c
index 41c0ffc..4199355 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -760,7 +760,8 @@ void attrib_server_exit(void)
 		remove_record_from_server(sdp_handle);
 }
 
-int attrib_db_add(uint16_t handle, uuid_t *uuid, const uint8_t *value, int len)
+int attrib_db_add(uint16_t handle, uuid_t *uuid, uint8_t perms,
+						const uint8_t *value, int len)
 {
 	struct attribute *a;
 
@@ -769,6 +770,7 @@ int attrib_db_add(uint16_t handle, uuid_t *uuid, const uint8_t *value, int len)
 	a = g_malloc0(sizeof(struct attribute) + len);
 	a->handle = handle;
 	memcpy(&a->uuid, uuid, sizeof(uuid_t));
+	a->perms = perms;
 	a->len = len;
 	memcpy(a->data, value, len);
 
diff --git a/src/attrib-server.h b/src/attrib-server.h
index 4a0afa6..e74a134 100644
--- a/src/attrib-server.h
+++ b/src/attrib-server.h
@@ -25,7 +25,18 @@
 int attrib_server_init(void);
 void attrib_server_exit(void);
 
-int attrib_db_add(uint16_t handle, uuid_t *uuid, const uint8_t *value, int len);
+/* Access permissions */
+#define ATT_READ	1
+#define ATT_WRITE	2
+/* Authentication/Authorization permissions */
+#define ATT_NONE		0
+#define ATT_AUTHENTICATION	2
+#define ATT_AUTHORIZATION	4
+/* Build bit mask for permission checks */
+#define ATT_ACCESS(x, y)	(((x) << y) | (x))
+
+int attrib_db_add(uint16_t handle, uuid_t *uuid, uint8_t perms,
+						const uint8_t *value, int len);
 int attrib_db_update(uint16_t handle, uuid_t *uuid, const uint8_t *value,
 								int len);
 int attrib_db_del(uint16_t handle);
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 0/3] Basic attribute permission support
From: Anderson Lizardo @ 2010-12-01 16:13 UTC (permalink / raw)
  To: linux-bluetooth

Hi,

This patchset adds initial support for attribute permission checks. Currently,
only access and authentication permissions are checked. Authorization
permissions require integration with the BlueZ agent, which is not implemented
yet.

There are some pending issues necessary for a minimum complete attribute
permission support (all of them are being worked on):

* The attribute client, upon receiving the "Insufficient Encryption" error,
  shall increase the security level and resend the failed request.
* The attribute server shall verify the connection permissions on each ATT
  request, and not just once on connection callback.
* On kernel side, increasing the security level (using setsockopt()) shall
  trigger SMP negotiation for a LE connection, blocking next socket I/O until
  negotiation is finished.
* On BR/EDR, link encryption needs to be done "on the fly" before resending the
  failed ATT request.

Albeit the above issues, we believe these patches are ready for commit.

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


^ permalink raw reply

* [PATCHv2 5/5] Bluetooth: clean up legal text
From: Emeltchenko Andrei @ 2010-12-01 14:58 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1291215506-11398-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>

Remove extra spaces from legal text so that legal stuff looks
the same for all bluetooth code.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
---
 include/net/bluetooth/hci.h    |   12 ++++++------
 include/net/bluetooth/l2cap.h  |   12 ++++++------
 include/net/bluetooth/rfcomm.h |   14 +++++++-------
 include/net/bluetooth/sco.h    |   12 ++++++------
 4 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index e9527c5..f3c5ed6 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1,4 +1,4 @@
-/* 
+/*
    BlueZ - Bluetooth protocol stack for Linux
    Copyright (C) 2000-2001 Qualcomm Incorporated
 
@@ -12,13 +12,13 @@
    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 
+   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 
+   ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATENTS,
+   COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS
    SOFTWARE IS DISCLAIMED.
 */
 
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 217bb91..7ad25ca 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -1,4 +1,4 @@
-/* 
+/*
    BlueZ - Bluetooth protocol stack for Linux
    Copyright (C) 2000-2001 Qualcomm Incorporated
    Copyright (C) 2009-2010 Gustavo F. Padovan <gustavo@padovan.org>
@@ -14,13 +14,13 @@
    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 
+   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 
+   ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATENTS,
+   COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS
    SOFTWARE IS DISCLAIMED.
 */
 
diff --git a/include/net/bluetooth/rfcomm.h b/include/net/bluetooth/rfcomm.h
index 2e78756..6eac4a7 100644
--- a/include/net/bluetooth/rfcomm.h
+++ b/include/net/bluetooth/rfcomm.h
@@ -1,5 +1,5 @@
-/* 
-   RFCOMM implementation for Linux Bluetooth stack (BlueZ).
+/*
+   RFCOMM implementation for Linux Bluetooth stack (BlueZ)
    Copyright (C) 2002 Maxim Krasnyansky <maxk@qualcomm.com>
    Copyright (C) 2002 Marcel Holtmann <marcel@holtmann.org>
 
@@ -11,13 +11,13 @@
    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 
+   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 
+   ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATENTS,
+   COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS
    SOFTWARE IS DISCLAIMED.
 */
 
diff --git a/include/net/bluetooth/sco.h b/include/net/bluetooth/sco.h
index ea5c664..1e35c43 100644
--- a/include/net/bluetooth/sco.h
+++ b/include/net/bluetooth/sco.h
@@ -1,4 +1,4 @@
-/* 
+/*
    BlueZ - Bluetooth protocol stack for Linux
    Copyright (C) 2000-2001 Qualcomm Incorporated
 
@@ -12,13 +12,13 @@
    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 
+   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 
+   ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATENTS,
+   COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS
    SOFTWARE IS DISCLAIMED.
 */
 
-- 
1.7.1


^ permalink raw reply related

* [PATCHv2 4/5] Bluetooth: clean up hci code
From: Emeltchenko Andrei @ 2010-12-01 14:58 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1291215506-11398-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>

Do not use assignment in IF condition, remove extra spaces,
fixing typos, simplify code.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
---
 include/net/bluetooth/hci.h      |    4 +-
 include/net/bluetooth/hci_core.h |   14 ++++----
 net/bluetooth/hci_conn.c         |   23 ++++++++----
 net/bluetooth/hci_core.c         |   66 ++++++++++++++++++++++++--------------
 net/bluetooth/hci_event.c        |    8 +++--
 net/bluetooth/hci_sock.c         |   17 ++++++---
 6 files changed, 82 insertions(+), 50 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index e30e008..e9527c5 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -489,7 +489,7 @@ struct hci_rp_read_local_name {
 
 #define HCI_OP_WRITE_PG_TIMEOUT		0x0c18
 
-#define HCI_OP_WRITE_SCAN_ENABLE 	0x0c1a
+#define HCI_OP_WRITE_SCAN_ENABLE	0x0c1a
 	#define SCAN_DISABLED		0x00
 	#define SCAN_INQUIRY		0x01
 	#define SCAN_PAGE		0x02
@@ -874,7 +874,7 @@ struct hci_ev_si_security {
 
 struct hci_command_hdr {
 	__le16	opcode;		/* OCF & OGF */
-	__u8 	plen;
+	__u8	plen;
 } __packed;
 
 struct hci_event_hdr {
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ebec8c9..9c08625 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -44,15 +44,15 @@ struct inquiry_data {
 };
 
 struct inquiry_entry {
-	struct inquiry_entry 	*next;
+	struct inquiry_entry	*next;
 	__u32			timestamp;
 	struct inquiry_data	data;
 };
 
 struct inquiry_cache {
-	spinlock_t 		lock;
+	spinlock_t		lock;
 	__u32			timestamp;
-	struct inquiry_entry 	*list;
+	struct inquiry_entry	*list;
 };
 
 struct hci_conn_hash {
@@ -141,7 +141,7 @@ struct hci_dev {
 	void			*driver_data;
 	void			*core_data;
 
-	atomic_t 		promisc;
+	atomic_t		promisc;
 
 	struct dentry		*debugfs;
 
@@ -150,7 +150,7 @@ struct hci_dev {
 
 	struct rfkill		*rfkill;
 
-	struct module 		*owner;
+	struct module		*owner;
 
 	int (*open)(struct hci_dev *hdev);
 	int (*close)(struct hci_dev *hdev);
@@ -215,8 +215,8 @@ extern rwlock_t hci_dev_list_lock;
 extern rwlock_t hci_cb_list_lock;
 
 /* ----- Inquiry cache ----- */
-#define INQUIRY_CACHE_AGE_MAX   (HZ*30)   // 30 seconds
-#define INQUIRY_ENTRY_AGE_MAX   (HZ*60)   // 60 seconds
+#define INQUIRY_CACHE_AGE_MAX   (HZ*30)   /* 30 seconds */
+#define INQUIRY_ENTRY_AGE_MAX   (HZ*60)   /* 60 seconds */
 
 #define inquiry_cache_lock(c)		spin_lock(&c->lock)
 #define inquiry_cache_unlock(c)		spin_unlock(&c->lock)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 0b1e460..6b90a41 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -39,7 +39,7 @@
 #include <net/sock.h>
 
 #include <asm/system.h>
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
 #include <asm/unaligned.h>
 
 #include <net/bluetooth/bluetooth.h>
@@ -66,7 +66,8 @@ void hci_acl_connect(struct hci_conn *conn)
 	bacpy(&cp.bdaddr, &conn->dst);
 	cp.pscan_rep_mode = 0x02;
 
-	if ((ie = hci_inquiry_cache_lookup(hdev, &conn->dst))) {
+	ie = hci_inquiry_cache_lookup(hdev, &conn->dst);
+	if (ie) {
 		if (inquiry_entry_age(ie) <= INQUIRY_ENTRY_AGE_MAX) {
 			cp.pscan_rep_mode = ie->data.pscan_rep_mode;
 			cp.pscan_mode     = ie->data.pscan_mode;
@@ -368,8 +369,10 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
 
 	BT_DBG("%s dst %s", hdev->name, batostr(dst));
 
-	if (!(acl = hci_conn_hash_lookup_ba(hdev, ACL_LINK, dst))) {
-		if (!(acl = hci_conn_add(hdev, ACL_LINK, dst)))
+	acl = hci_conn_hash_lookup_ba(hdev, ACL_LINK, dst);
+	if (!acl) {
+		acl = hci_conn_add(hdev, ACL_LINK, dst);
+		if (!acl)
 			return NULL;
 	}
 
@@ -389,8 +392,10 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
 	if (type == ACL_LINK)
 		return acl;
 
-	if (!(sco = hci_conn_hash_lookup_ba(hdev, type, dst))) {
-		if (!(sco = hci_conn_add(hdev, type, dst))) {
+	sco = hci_conn_hash_lookup_ba(hdev, type, dst);
+	if (!sco) {
+		sco = hci_conn_add(hdev, type, dst);
+		if (!sco) {
 			hci_conn_put(acl);
 			return NULL;
 		}
@@ -647,10 +652,12 @@ int hci_get_conn_list(void __user *arg)
 
 	size = sizeof(req) + req.conn_num * sizeof(*ci);
 
-	if (!(cl = kmalloc(size, GFP_KERNEL)))
+	cl = kmalloc(size, GFP_KERNEL);
+	if (!cl)
 		return -ENOMEM;
 
-	if (!(hdev = hci_dev_get(req.dev_id))) {
+	hdev = hci_dev_get(req.dev_id);
+	if (!hdev) {
 		kfree(cl);
 		return -ENODEV;
 	}
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index bc2a052..51c61f7 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -44,7 +44,7 @@
 #include <net/sock.h>
 
 #include <asm/system.h>
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
 #include <asm/unaligned.h>
 
 #include <net/bluetooth/bluetooth.h>
@@ -349,20 +349,23 @@ struct inquiry_entry *hci_inquiry_cache_lookup(struct hci_dev *hdev, bdaddr_t *b
 void hci_inquiry_cache_update(struct hci_dev *hdev, struct inquiry_data *data)
 {
 	struct inquiry_cache *cache = &hdev->inq_cache;
-	struct inquiry_entry *e;
+	struct inquiry_entry *ie;
 
 	BT_DBG("cache %p, %s", cache, batostr(&data->bdaddr));
 
-	if (!(e = hci_inquiry_cache_lookup(hdev, &data->bdaddr))) {
+	ie = hci_inquiry_cache_lookup(hdev, &data->bdaddr);
+	if (!ie) {
 		/* Entry not in the cache. Add new one. */
-		if (!(e = kzalloc(sizeof(struct inquiry_entry), GFP_ATOMIC)))
+		ie = kzalloc(sizeof(struct inquiry_entry), GFP_ATOMIC);
+		if (!ie)
 			return;
-		e->next     = cache->list;
-		cache->list = e;
+
+		ie->next = cache->list;
+		cache->list = ie;
 	}
 
-	memcpy(&e->data, data, sizeof(*data));
-	e->timestamp = jiffies;
+	memcpy(&ie->data, data, sizeof(*data));
+	ie->timestamp = jiffies;
 	cache->timestamp = jiffies;
 }
 
@@ -422,16 +425,20 @@ int hci_inquiry(void __user *arg)
 
 	hci_dev_lock_bh(hdev);
 	if (inquiry_cache_age(hdev) > INQUIRY_CACHE_AGE_MAX ||
-					inquiry_cache_empty(hdev) ||
-					ir.flags & IREQ_CACHE_FLUSH) {
+				inquiry_cache_empty(hdev) ||
+				ir.flags & IREQ_CACHE_FLUSH) {
 		inquiry_cache_flush(hdev);
 		do_inquiry = 1;
 	}
 	hci_dev_unlock_bh(hdev);
 
 	timeo = ir.length * msecs_to_jiffies(2000);
-	if (do_inquiry && (err = hci_request(hdev, hci_inq_req, (unsigned long)&ir, timeo)) < 0)
-		goto done;
+
+	if (do_inquiry) {
+		err = hci_request(hdev, hci_inq_req, (unsigned long)&ir, timeo);
+		if (err < 0)
+			goto done;
+	}
 
 	/* for unlimited number of responses we will use buffer with 255 entries */
 	max_rsp = (ir.num_rsp == 0) ? 255 : ir.num_rsp;
@@ -439,7 +446,8 @@ int hci_inquiry(void __user *arg)
 	/* cache_dump can't sleep. Therefore we allocate temp buffer and then
 	 * copy it to the user space.
 	 */
-	if (!(buf = kmalloc(sizeof(struct inquiry_info) * max_rsp, GFP_KERNEL))) {
+	buf = kmalloc(sizeof(struct inquiry_info) *max_rsp, GFP_KERNEL);
+	if (!buf) {
 		err = -ENOMEM;
 		goto done;
 	}
@@ -611,7 +619,8 @@ int hci_dev_close(__u16 dev)
 	struct hci_dev *hdev;
 	int err;
 
-	if (!(hdev = hci_dev_get(dev)))
+	hdev = hci_dev_get(dev);
+	if (!hdev)
 		return -ENODEV;
 	err = hci_dev_do_close(hdev);
 	hci_dev_put(hdev);
@@ -623,7 +632,8 @@ int hci_dev_reset(__u16 dev)
 	struct hci_dev *hdev;
 	int ret = 0;
 
-	if (!(hdev = hci_dev_get(dev)))
+	hdev = hci_dev_get(dev);
+	if (!hdev)
 		return -ENODEV;
 
 	hci_req_lock(hdev);
@@ -663,7 +673,8 @@ int hci_dev_reset_stat(__u16 dev)
 	struct hci_dev *hdev;
 	int ret = 0;
 
-	if (!(hdev = hci_dev_get(dev)))
+	hdev = hci_dev_get(dev);
+	if (!hdev)
 		return -ENODEV;
 
 	memset(&hdev->stat, 0, sizeof(struct hci_dev_stats));
@@ -682,7 +693,8 @@ int hci_dev_cmd(unsigned int cmd, void __user *arg)
 	if (copy_from_user(&dr, arg, sizeof(dr)))
 		return -EFAULT;
 
-	if (!(hdev = hci_dev_get(dr.dev_id)))
+	hdev = hci_dev_get(dr.dev_id);
+	if (!hdev)
 		return -ENODEV;
 
 	switch (cmd) {
@@ -763,7 +775,8 @@ int hci_get_dev_list(void __user *arg)
 
 	size = sizeof(*dl) + dev_num * sizeof(*dr);
 
-	if (!(dl = kzalloc(size, GFP_KERNEL)))
+	dl = kzalloc(size, GFP_KERNEL);
+	if (!dl)
 		return -ENOMEM;
 
 	dr = dl->dev_req;
@@ -797,7 +810,8 @@ int hci_get_dev_info(void __user *arg)
 	if (copy_from_user(&di, arg, sizeof(di)))
 		return -EFAULT;
 
-	if (!(hdev = hci_dev_get(di.dev_id)))
+	hdev = hci_dev_get(di.dev_id);
+	if (!hdev)
 		return -ENODEV;
 
 	strcpy(di.name, hdev->name);
@@ -905,7 +919,7 @@ int hci_register_dev(struct hci_dev *hdev)
 	hdev->sniff_max_interval = 800;
 	hdev->sniff_min_interval = 80;
 
-	tasklet_init(&hdev->cmd_task, hci_cmd_task,(unsigned long) hdev);
+	tasklet_init(&hdev->cmd_task, hci_cmd_task, (unsigned long) hdev);
 	tasklet_init(&hdev->rx_task, hci_rx_task, (unsigned long) hdev);
 	tasklet_init(&hdev->tx_task, hci_tx_task, (unsigned long) hdev);
 
@@ -1368,7 +1382,8 @@ void hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags)
 	bt_cb(skb)->pkt_type = HCI_ACLDATA_PKT;
 	hci_add_acl_hdr(skb, conn->handle, flags | ACL_START);
 
-	if (!(list = skb_shinfo(skb)->frag_list)) {
+	list = skb_shinfo(skb)->frag_list;
+	if (!list) {
 		/* Non fragmented */
 		BT_DBG("%s nonfrag skb %p len %d", hdev->name, skb, skb->len);
 
@@ -1609,7 +1624,8 @@ static inline void hci_acldata_packet(struct hci_dev *hdev, struct sk_buff *skb)
 		hci_conn_enter_active_mode(conn);
 
 		/* Send to upper protocol */
-		if ((hp = hci_proto[HCI_PROTO_L2CAP]) && hp->recv_acldata) {
+		hp = hci_proto[HCI_PROTO_L2CAP];
+		if (hp && hp->recv_acldata) {
 			hp->recv_acldata(conn, skb, flags);
 			return;
 		}
@@ -1644,7 +1660,8 @@ static inline void hci_scodata_packet(struct hci_dev *hdev, struct sk_buff *skb)
 		register struct hci_proto *hp;
 
 		/* Send to upper protocol */
-		if ((hp = hci_proto[HCI_PROTO_SCO]) && hp->recv_scodata) {
+		hp = hci_proto[HCI_PROTO_SCO];
+		if (hp && hp->recv_scodata) {
 			hp->recv_scodata(conn, skb);
 			return;
 		}
@@ -1727,7 +1744,8 @@ static void hci_cmd_task(unsigned long arg)
 	if (atomic_read(&hdev->cmd_cnt) && (skb = skb_dequeue(&hdev->cmd_q))) {
 		kfree_skb(hdev->sent_cmd);
 
-		if ((hdev->sent_cmd = skb_clone(skb, GFP_ATOMIC))) {
+		hdev->sent_cmd = skb_clone(skb, GFP_ATOMIC);
+		if (hdev->sent_cmd) {
 			atomic_dec(&hdev->cmd_cnt);
 			hci_send_frame(skb);
 			hdev->cmd_last_tx = jiffies;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 3c1957c..8923b36 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -39,7 +39,7 @@
 #include <net/sock.h>
 
 #include <asm/system.h>
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
 #include <asm/unaligned.h>
 
 #include <net/bluetooth/bluetooth.h>
@@ -1512,10 +1512,12 @@ static inline void hci_num_comp_pkts_evt(struct hci_dev *hdev, struct sk_buff *s
 			conn->sent -= count;
 
 			if (conn->type == ACL_LINK) {
-				if ((hdev->acl_cnt += count) > hdev->acl_pkts)
+				hdev->acl_cnt += count;
+				if (hdev->acl_cnt > hdev->acl_pkts)
 					hdev->acl_cnt = hdev->acl_pkts;
 			} else {
-				if ((hdev->sco_cnt += count) > hdev->sco_pkts)
+				hdev->sco_cnt += count;
+				if (hdev->sco_cnt > hdev->sco_pkts)
 					hdev->sco_cnt = hdev->sco_pkts;
 			}
 		}
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index 83acd16..b3753ba 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -43,7 +43,7 @@
 #include <net/sock.h>
 
 #include <asm/system.h>
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
 #include <asm/unaligned.h>
 
 #include <net/bluetooth/bluetooth.h>
@@ -125,7 +125,8 @@ void hci_send_to_sock(struct hci_dev *hdev, struct sk_buff *skb)
 				continue;
 		}
 
-		if (!(nskb = skb_clone(skb, GFP_ATOMIC)))
+		nskb = skb_clone(skb, GFP_ATOMIC);
+		if (!nskb)
 			continue;
 
 		/* Put type byte before the data */
@@ -370,7 +371,8 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr, int addr_le
 	}
 
 	if (haddr->hci_dev != HCI_DEV_NONE) {
-		if (!(hdev = hci_dev_get(haddr->hci_dev))) {
+		hdev = hci_dev_get(haddr->hci_dev);
+		if (!hdev) {
 			err = -ENODEV;
 			goto done;
 		}
@@ -457,7 +459,8 @@ static int hci_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
 	if (sk->sk_state == BT_CLOSED)
 		return 0;
 
-	if (!(skb = skb_recv_datagram(sk, flags, noblock, &err)))
+	skb = skb_recv_datagram(sk, flags, noblock, &err);
+	if (!skb)
 		return err;
 
 	msg->msg_namelen = 0;
@@ -499,7 +502,8 @@ static int hci_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
 
 	lock_sock(sk);
 
-	if (!(hdev = hci_pi(sk)->hdev)) {
+	hdev = hci_pi(sk)->hdev;
+	if (!hdev) {
 		err = -EBADFD;
 		goto done;
 	}
@@ -509,7 +513,8 @@ static int hci_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
 		goto done;
 	}
 
-	if (!(skb = bt_skb_send_alloc(sk, len, msg->msg_flags & MSG_DONTWAIT, &err)))
+	skb = bt_skb_send_alloc(sk, len, msg->msg_flags & MSG_DONTWAIT, &err);
+	if (!skb)
 		goto done;
 
 	if (memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len)) {
-- 
1.7.1


^ permalink raw reply related

* [PATCHv2 3/5] Bluetooth: clean up l2cap code
From: Emeltchenko Andrei @ 2010-12-01 14:58 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1291215506-11398-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>

Do not initialize static vars to zero, macros with complex values
shall be enclosed with (), remove unneeded braces.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
---
 include/net/bluetooth/l2cap.h |   10 +++++-----
 net/bluetooth/l2cap.c         |    7 +++----
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index c819c8b..217bb91 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -417,11 +417,11 @@ static inline int l2cap_tx_window_full(struct sock *sk)
 	return sub == pi->remote_tx_win;
 }
 
-#define __get_txseq(ctrl) ((ctrl) & L2CAP_CTRL_TXSEQ) >> 1
-#define __get_reqseq(ctrl) ((ctrl) & L2CAP_CTRL_REQSEQ) >> 8
-#define __is_iframe(ctrl) !((ctrl) & L2CAP_CTRL_FRAME_TYPE)
-#define __is_sframe(ctrl) (ctrl) & L2CAP_CTRL_FRAME_TYPE
-#define __is_sar_start(ctrl) ((ctrl) & L2CAP_CTRL_SAR) == L2CAP_SDU_START
+#define __get_txseq(ctrl)	(((ctrl) & L2CAP_CTRL_TXSEQ) >> 1)
+#define __get_reqseq(ctrl)	(((ctrl) & L2CAP_CTRL_REQSEQ) >> 8)
+#define __is_iframe(ctrl)	(!((ctrl) & L2CAP_CTRL_FRAME_TYPE))
+#define __is_sframe(ctrl)	((ctrl) & L2CAP_CTRL_FRAME_TYPE)
+#define __is_sar_start(ctrl)	(((ctrl) & L2CAP_CTRL_SAR) == L2CAP_SDU_START)
 
 void l2cap_load(void);
 
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 12b4aa2..d4b4fbd 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -57,7 +57,7 @@
 
 #define VERSION "2.15"
 
-static int disable_ertm = 0;
+static int disable_ertm;
 
 static u32 l2cap_feat_mask = L2CAP_FEAT_FIXED_CHAN;
 static u8 l2cap_fixed_chan[8] = { 0x02, };
@@ -4162,11 +4162,10 @@ static inline void l2cap_data_channel_rrframe(struct sock *sk, u16 rx_control)
 			__mod_retrans_timer();
 
 		pi->conn_state &= ~L2CAP_CONN_REMOTE_BUSY;
-		if (pi->conn_state & L2CAP_CONN_SREJ_SENT) {
+		if (pi->conn_state & L2CAP_CONN_SREJ_SENT)
 			l2cap_send_ack(pi);
-		} else {
+		else
 			l2cap_ertm_send(sk);
-		}
 	}
 }
 
-- 
1.7.1


^ permalink raw reply related

* [PATCHv2 2/5] Bluetooth: clean up rfcomm code
From: Emeltchenko Andrei @ 2010-12-01 14:58 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1291215506-11398-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>

Remove extra spaces, assignments in if statement, zeroing static
variables, extra braces. Fix includes.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
---
 include/net/bluetooth/rfcomm.h |    4 ++--
 net/bluetooth/rfcomm/core.c    |    8 ++++----
 net/bluetooth/rfcomm/sock.c    |    5 +++--
 net/bluetooth/rfcomm/tty.c     |   28 ++++++++++++++++------------
 4 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/include/net/bluetooth/rfcomm.h b/include/net/bluetooth/rfcomm.h
index 71047bc..2e78756 100644
--- a/include/net/bluetooth/rfcomm.h
+++ b/include/net/bluetooth/rfcomm.h
@@ -105,7 +105,7 @@
 struct rfcomm_hdr {
 	u8 addr;
 	u8 ctrl;
-	u8 len;    // Actual size can be 2 bytes
+	u8 len;    /* Actual size can be 2 bytes */
 } __packed;
 
 struct rfcomm_cmd {
@@ -228,7 +228,7 @@ struct rfcomm_dlc {
 /* ---- RFCOMM SEND RPN ---- */
 int rfcomm_send_rpn(struct rfcomm_session *s, int cr, u8 dlci,
 			u8 bit_rate, u8 data_bits, u8 stop_bits,
-			u8 parity, u8 flow_ctrl_settings, 
+			u8 parity, u8 flow_ctrl_settings,
 			u8 xon_char, u8 xoff_char, u16 param_mask);
 
 /* ---- RFCOMM DLCs (channels) ---- */
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index fa642aa..c1e2bba 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -41,7 +41,7 @@
 #include <linux/slab.h>
 
 #include <net/sock.h>
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
 #include <asm/unaligned.h>
 
 #include <net/bluetooth/bluetooth.h>
@@ -51,10 +51,10 @@
 
 #define VERSION "1.11"
 
-static int disable_cfc = 0;
+static int disable_cfc;
+static int l2cap_ertm;
 static int channel_mtu = -1;
 static unsigned int l2cap_mtu = RFCOMM_MAX_L2CAP_MTU;
-static int l2cap_ertm = 0;
 
 static struct task_struct *rfcomm_thread;
 
@@ -1901,7 +1901,7 @@ static inline void rfcomm_check_connection(struct rfcomm_session *s)
 
 	BT_DBG("%p state %ld", s, s->state);
 
-	switch(sk->sk_state) {
+	switch (sk->sk_state) {
 	case BT_CONNECTED:
 		s->state = BT_CONNECT;
 
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 0207bd6..66cc1f0 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -45,7 +45,7 @@
 #include <net/sock.h>
 
 #include <asm/system.h>
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
 
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>
@@ -888,7 +888,8 @@ static int rfcomm_sock_shutdown(struct socket *sock, int how)
 
 	BT_DBG("sock %p, sk %p", sock, sk);
 
-	if (!sk) return 0;
+	if (!sk)
+		return 0;
 
 	lock_sock(sk);
 	if (!sk->sk_shutdown) {
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index a9b81f5..2575c2d 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -58,9 +58,9 @@ struct rfcomm_dev {
 
 	bdaddr_t		src;
 	bdaddr_t		dst;
-	u8 			channel;
+	u8			channel;
 
-	uint 			modem_status;
+	uint			modem_status;
 
 	struct rfcomm_dlc	*dlc;
 	struct tty_struct	*tty;
@@ -69,7 +69,7 @@ struct rfcomm_dev {
 
 	struct device		*tty_dev;
 
-	atomic_t 		wmem_alloc;
+	atomic_t		wmem_alloc;
 
 	struct sk_buff_head	pending;
 };
@@ -431,7 +431,8 @@ static int rfcomm_release_dev(void __user *arg)
 
 	BT_DBG("dev_id %d flags 0x%x", req.dev_id, req.flags);
 
-	if (!(dev = rfcomm_dev_get(req.dev_id)))
+	dev = rfcomm_dev_get(req.dev_id);
+	if (!dev)
 		return -ENODEV;
 
 	if (dev->flags != NOCAP_FLAGS && !capable(CAP_NET_ADMIN)) {
@@ -470,7 +471,8 @@ static int rfcomm_get_dev_list(void __user *arg)
 
 	size = sizeof(*dl) + dev_num * sizeof(*di);
 
-	if (!(dl = kmalloc(size, GFP_KERNEL)))
+	dl = kmalloc(size, GFP_KERNEL);
+	if (!dl)
 		return -ENOMEM;
 
 	di = dl->dev_info;
@@ -513,7 +515,8 @@ static int rfcomm_get_dev_info(void __user *arg)
 	if (copy_from_user(&di, arg, sizeof(di)))
 		return -EFAULT;
 
-	if (!(dev = rfcomm_dev_get(di.id)))
+	dev = rfcomm_dev_get(di.id);
+	if (!dev)
 		return -ENODEV;
 
 	di.flags   = dev->flags;
@@ -561,7 +564,8 @@ static void rfcomm_dev_data_ready(struct rfcomm_dlc *dlc, struct sk_buff *skb)
 		return;
 	}
 
-	if (!(tty = dev->tty) || !skb_queue_empty(&dev->pending)) {
+	tty = dev->tty;
+	if (!tty || !skb_queue_empty(&dev->pending)) {
 		skb_queue_tail(&dev->pending, skb);
 		return;
 	}
@@ -796,7 +800,8 @@ static int rfcomm_tty_write(struct tty_struct *tty, const unsigned char *buf, in
 
 		memcpy(skb_put(skb, size), buf + sent, size);
 
-		if ((err = rfcomm_dlc_send(dlc, skb)) < 0) {
+		err = rfcomm_dlc_send(dlc, skb);
+		if (err < 0) {
 			kfree_skb(skb);
 			break;
 		}
@@ -892,7 +897,7 @@ static void rfcomm_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
 
 	/* Parity on/off and when on, odd/even */
 	if (((old->c_cflag & PARENB) != (new->c_cflag & PARENB)) ||
-			((old->c_cflag & PARODD) != (new->c_cflag & PARODD)) ) {
+			((old->c_cflag & PARODD) != (new->c_cflag & PARODD))) {
 		changes |= RFCOMM_RPN_PM_PARITY;
 		BT_DBG("Parity change detected.");
 	}
@@ -937,11 +942,10 @@ static void rfcomm_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
 	/* POSIX does not support 1.5 stop bits and RFCOMM does not
 	 * support 2 stop bits. So a request for 2 stop bits gets
 	 * translated to 1.5 stop bits */
-	if (new->c_cflag & CSTOPB) {
+	if (new->c_cflag & CSTOPB)
 		stop_bits = RFCOMM_RPN_STOP_15;
-	} else {
+	else
 		stop_bits = RFCOMM_RPN_STOP_1;
-	}
 
 	/* Handle number of data bits [5-8] */
 	if ((old->c_cflag & CSIZE) != (new->c_cflag & CSIZE))
-- 
1.7.1


^ permalink raw reply related

* [PATCHv2 1/5] Bluetooth: clean up sco code
From: Emeltchenko Andrei @ 2010-12-01 14:58 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1291215506-11398-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>

Do not use assignments in IF condition, remove extra spaces

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
---
 include/net/bluetooth/sco.h |    8 ++++----
 net/bluetooth/sco.c         |   22 +++++++++++++---------
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/include/net/bluetooth/sco.h b/include/net/bluetooth/sco.h
index e28a2a7..ea5c664 100644
--- a/include/net/bluetooth/sco.h
+++ b/include/net/bluetooth/sco.h
@@ -55,11 +55,11 @@ struct sco_conninfo {
 struct sco_conn {
 	struct hci_conn	*hcon;
 
-	bdaddr_t 	*dst;
-	bdaddr_t 	*src;
-	
+	bdaddr_t	*dst;
+	bdaddr_t	*src;
+
 	spinlock_t	lock;
-	struct sock 	*sk;
+	struct sock	*sk;
 
 	unsigned int    mtu;
 };
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 66b9e5c..960c6d1 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -44,7 +44,7 @@
 #include <net/sock.h>
 
 #include <asm/system.h>
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
 
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>
@@ -52,7 +52,7 @@
 
 #define VERSION "0.6"
 
-static int disable_esco = 0;
+static int disable_esco;
 
 static const struct proto_ops sco_sock_ops;
 
@@ -138,16 +138,17 @@ static inline struct sock *sco_chan_get(struct sco_conn *conn)
 
 static int sco_conn_del(struct hci_conn *hcon, int err)
 {
-	struct sco_conn *conn;
+	struct sco_conn *conn = hcon->sco_data;
 	struct sock *sk;
 
-	if (!(conn = hcon->sco_data))
+	if (!conn)
 		return 0;
 
 	BT_DBG("hcon %p conn %p, err %d", hcon, conn, err);
 
 	/* Kill socket */
-	if ((sk = sco_chan_get(conn))) {
+	sk = sco_chan_get(conn);
+	if (sk) {
 		bh_lock_sock(sk);
 		sco_sock_clear_timer(sk);
 		sco_chan_del(sk, err);
@@ -185,7 +186,8 @@ static int sco_connect(struct sock *sk)
 
 	BT_DBG("%s -> %s", batostr(src), batostr(dst));
 
-	if (!(hdev = hci_get_route(dst, src)))
+	hdev = hci_get_route(dst, src);
+	if (!hdev)
 		return -EHOSTUNREACH;
 
 	hci_dev_lock_bh(hdev);
@@ -510,7 +512,8 @@ static int sco_sock_connect(struct socket *sock, struct sockaddr *addr, int alen
 	/* Set destination address and psm */
 	bacpy(&bt_sk(sk)->dst, &sa->sco_bdaddr);
 
-	if ((err = sco_connect(sk)))
+	err = sco_connect(sk);
+	if (err)
 		goto done;
 
 	err = bt_sock_wait_state(sk, BT_CONNECTED,
@@ -828,13 +831,14 @@ static void sco_chan_del(struct sock *sk, int err)
 
 static void sco_conn_ready(struct sco_conn *conn)
 {
-	struct sock *parent, *sk;
+	struct sock *parent;
+	struct sock *sk = conn->sk;
 
 	BT_DBG("conn %p", conn);
 
 	sco_conn_lock(conn);
 
-	if ((sk = conn->sk)) {
+	if (sk) {
 		sco_sock_clear_timer(sk);
 		bh_lock_sock(sk);
 		sk->sk_state = BT_CONNECTED;
-- 
1.7.1


^ permalink raw reply related

* [PATCHv2 0/5] Clean up sco, rfcomm and hci code
From: Emeltchenko Andrei @ 2010-12-01 14:58 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>

Remove extra spaces, do not use assignments in "if" statements,
remove initialization to zero static vars, other errors
reported by checkpatch.
*v1 RFC
*v2 taken comments from Anderson, Marcel and Gustavo:
	- legal stuff moved to special patch
	- checking for do_inquiry before removing assignments

Andrei Emeltchenko (5):
  Bluetooth: clean up sco code
  Bluetooth: clean up rfcomm code
  Bluetooth: clean up l2cap code
  Bluetooth: clean up hci code
  Bluetooth: clean up legal text

 include/net/bluetooth/hci.h      |   16 ++++----
 include/net/bluetooth/hci_core.h |   14 ++++----
 include/net/bluetooth/l2cap.h    |   22 ++++++------
 include/net/bluetooth/rfcomm.h   |   18 +++++-----
 include/net/bluetooth/sco.h      |   20 ++++++------
 net/bluetooth/hci_conn.c         |   23 ++++++++----
 net/bluetooth/hci_core.c         |   66 ++++++++++++++++++++++++--------------
 net/bluetooth/hci_event.c        |    8 +++--
 net/bluetooth/hci_sock.c         |   17 ++++++---
 net/bluetooth/l2cap.c            |    7 ++--
 net/bluetooth/rfcomm/core.c      |    8 ++--
 net/bluetooth/rfcomm/sock.c      |    5 ++-
 net/bluetooth/rfcomm/tty.c       |   28 +++++++++-------
 net/bluetooth/sco.c              |   22 +++++++-----
 14 files changed, 157 insertions(+), 117 deletions(-)


^ 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