2009-03-28 17:47:25

by Gustavo F. Padovan

[permalink] [raw]
Subject: [PATCH 0/4] Bluetooth: clean ups to l2cap

Hi,

These patches are result of my hackings to l2cap.
First and third just create macros to do not show some number in
code. Also facilite the hacking because we don't need to go to spec
to figure out what that numbers shows. The first patch is a resend.
The last patch is the result of run l2cap files against checkpatch.pl

Regards.




2009-03-28 17:47:29

by Gustavo F. Padovan

[permalink] [raw]
Subject: [PATCH 4/4] Bluetooth: fix many errors and warnings reported by checkpatch.pl

This patch fixes the errors without change the l2cap.o binary

text data bss dec hex filename
18059 568 0 18627 48c3 l2cap.o.after
18059 568 0 18627 48c3 l2cap.o.before

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
include/net/bluetooth/l2cap.h | 12 +++---
net/bluetooth/l2cap.c | 70 +++++++++++++++++++++++++----------------
2 files changed, 49 insertions(+), 33 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 300b63f..18dcb33 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

@@ -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/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index a2cc770..76190f0 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -40,10 +40,10 @@
#include <linux/skbuff.h>
#include <linux/list.h>
#include <linux/device.h>
+#include <linux/uaccess.h>
#include <net/sock.h>

#include <asm/system.h>
-#include <asm/uaccess.h>
#include <asm/unaligned.h>

#include <net/bluetooth/bluetooth.h>
@@ -134,7 +134,8 @@ static inline struct sock *l2cap_get_chan_by_scid(struct l2cap_chan_list *l, u16
struct sock *s;
read_lock(&l->lock);
s = __l2cap_get_chan_by_scid(l, cid);
- if (s) bh_lock_sock(s);
+ if (s)
+ bh_lock_sock(s);
read_unlock(&l->lock);
return s;
}
@@ -154,7 +155,8 @@ static inline struct sock *l2cap_get_chan_by_ident(struct l2cap_chan_list *l, u8
struct sock *s;
read_lock(&l->lock);
s = __l2cap_get_chan_by_ident(l, ident);
- if (s) bh_lock_sock(s);
+ if (s)
+ bh_lock_sock(s);
read_unlock(&l->lock);
return s;
}
@@ -164,7 +166,7 @@ static u16 l2cap_alloc_cid(struct l2cap_chan_list *l)
u16 cid = L2CAP_CID_DYN_START;

for (; cid < L2CAP_CID_DYN_END; cid++) {
- if(!__l2cap_get_chan_by_scid(l, cid))
+ if (!__l2cap_get_chan_by_scid(l, cid))
return cid;
}

@@ -204,7 +206,8 @@ static void __l2cap_chan_add(struct l2cap_conn *conn, struct sock *sk, struct so
{
struct l2cap_chan_list *l = &conn->chan_list;

- BT_DBG("conn %p, psm 0x%2.2x, dcid 0x%4.4x", conn, l2cap_pi(sk)->psm, l2cap_pi(sk)->dcid);
+ BT_DBG("conn %p, psm 0x%2.2x, dcid 0x%4.4x", conn,
+ l2cap_pi(sk)->psm, l2cap_pi(sk)->dcid);

conn->disc_reason = 0x13;

@@ -272,7 +275,7 @@ static inline int l2cap_check_security(struct sock *sk)
if (l2cap_pi(sk)->sec_level == BT_SECURITY_HIGH)
auth_type = HCI_AT_NO_BONDING_MITM;
else
- auth_type = HCI_AT_NO_BONDING;
+ auth_type = HCI_AT_NO_BONDING;

if (l2cap_pi(sk)->sec_level == BT_SECURITY_LOW)
l2cap_pi(sk)->sec_level = BT_SECURITY_SDP;
@@ -588,7 +591,8 @@ static inline struct sock *l2cap_get_sock_by_psm(int state, __le16 psm, bdaddr_t
struct sock *s;
read_lock(&l2cap_sk_list.lock);
s = __l2cap_get_sock_by_psm(state, psm, src);
- if (s) bh_lock_sock(s);
+ if (s)
+ bh_lock_sock(s);
read_unlock(&l2cap_sk_list.lock);
return s;
}
@@ -849,7 +853,8 @@ static int l2cap_do_connect(struct sock *sk)
BT_DBG("%s -> %s psm 0x%2.2x", batostr(src), batostr(dst),
l2cap_pi(sk)->psm);

- if (!(hdev = hci_get_route(dst, src)))
+ hdev = hci_get_route(dst, src);
+ if (!hdev)
return -EHOSTUNREACH;

hci_dev_lock_bh(hdev);
@@ -948,7 +953,7 @@ static int l2cap_sock_connect(struct socket *sock, struct sockaddr *addr, int al
goto done;
}

- switch(sk->sk_state) {
+ switch (sk->sk_state) {
case BT_CONNECT:
case BT_CONNECT2:
case BT_CONFIG:
@@ -973,7 +978,8 @@ static int l2cap_sock_connect(struct socket *sock, struct sockaddr *addr, int al
bacpy(&bt_sk(sk)->dst, &la.l2_bdaddr);
l2cap_pi(sk)->psm = la.l2_psm;

- if ((err = l2cap_do_connect(sk)))
+ err = l2cap_do_connect(sk);
+ if (err)
goto done;

wait:
@@ -1112,7 +1118,7 @@ static inline int l2cap_do_send(struct sock *sk, struct msghdr *msg, int len)
{
struct l2cap_conn *conn = l2cap_pi(sk)->conn;
struct sk_buff *skb, **frag;
- int err, hlen, count, sent=0;
+ int err, hlen, count, sent = 0;
struct l2cap_hdr *lh;

BT_DBG("sk %p len %d", sk, len);
@@ -1165,8 +1171,8 @@ static inline int l2cap_do_send(struct sock *sk, struct msghdr *msg, int len)

frag = &(*frag)->next;
}
-
- if ((err = hci_send_acl(conn->hcon, skb, 0)) < 0)
+ err = hci_send_acl(conn->hcon, skb, 0);
+ if (err < 0)
goto fail;

return sent;
@@ -1554,7 +1560,7 @@ static void l2cap_raw_recv(struct l2cap_conn *conn, struct sk_buff *skb)
{
struct l2cap_chan_list *l = &conn->chan_list;
struct sk_buff *nskb;
- struct sock * sk;
+ struct sock *sk;

BT_DBG("conn %p", conn);

@@ -1566,8 +1572,8 @@ static void l2cap_raw_recv(struct l2cap_conn *conn, struct sk_buff *skb)
/* Don't send frame to the socket it came from */
if (skb->sk == sk)
continue;
-
- if (!(nskb = skb_clone(skb, GFP_ATOMIC)))
+ nskb = skb_clone(skb, GFP_ATOMIC);
+ if (!nskb)
continue;

if (sock_queue_rcv_skb(sk, nskb))
@@ -1585,7 +1591,8 @@ static struct sk_buff *l2cap_build_cmd(struct l2cap_conn *conn,
struct l2cap_hdr *lh;
int len, count;

- BT_DBG("conn %p, code 0x%2.2x, ident 0x%2.2x, len %d", conn, code, ident, dlen);
+ BT_DBG("conn %p, code 0x%2.2x, ident 0x%2.2x, len %d",
+ conn, code, ident, dlen);

len = L2CAP_HDR_SIZE + L2CAP_CMD_HDR_SIZE + dlen;
count = min_t(unsigned int, conn->mtu, len);
@@ -1964,10 +1971,12 @@ static inline int l2cap_connect_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hd
BT_DBG("dcid 0x%4.4x scid 0x%4.4x result 0x%2.2x status 0x%2.2x", dcid, scid, result, status);

if (scid) {
- if (!(sk = l2cap_get_chan_by_scid(&conn->chan_list, scid)))
+ sk = l2cap_get_chan_by_scid(&conn->chan_list, scid);
+ if (!sk)
return 0;
} else {
- if (!(sk = l2cap_get_chan_by_ident(&conn->chan_list, cmd->ident)))
+ sk = l2cap_get_chan_by_ident(&conn->chan_list, cmd->ident);
+ if (!sk)
return 0;
}

@@ -2010,7 +2019,8 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr

BT_DBG("dcid 0x%4.4x flags 0x%2.2x", dcid, flags);

- if (!(sk = l2cap_get_chan_by_scid(&conn->chan_list, dcid)))
+ sk = l2cap_get_chan_by_scid(&conn->chan_list, dcid);
+ if (!sk)
return -ENOENT;

if (sk->sk_state == BT_DISCONN)
@@ -2077,9 +2087,11 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
flags = __le16_to_cpu(rsp->flags);
result = __le16_to_cpu(rsp->result);

- BT_DBG("scid 0x%4.4x flags 0x%2.2x result 0x%2.2x", scid, flags, result);
+ BT_DBG("scid 0x%4.4x flags 0x%2.2x result 0x%2.2x",
+ scid, flags, result);

- if (!(sk = l2cap_get_chan_by_scid(&conn->chan_list, scid)))
+ sk = l2cap_get_chan_by_scid(&conn->chan_list, scid);
+ if (!sk)
return 0;

switch (result) {
@@ -2140,7 +2152,8 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn, struct l2cap_cmd

BT_DBG("scid 0x%4.4x dcid 0x%4.4x", scid, dcid);

- if (!(sk = l2cap_get_chan_by_scid(&conn->chan_list, dcid)))
+ sk = l2cap_get_chan_by_scid(&conn->chan_list, dcid);
+ if (!sk)
return 0;

rsp.dcid = cpu_to_le16(l2cap_pi(sk)->scid);
@@ -2167,7 +2180,8 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn, struct l2cap_cmd

BT_DBG("dcid 0x%4.4x scid 0x%4.4x", dcid, scid);

- if (!(sk = l2cap_get_chan_by_scid(&conn->chan_list, scid)))
+ sk = l2cap_get_chan_by_scid(&conn->chan_list, scid);
+ if (!sk)
return 0;

l2cap_chan_del(sk, 0);
@@ -2401,7 +2415,8 @@ drop:
kfree_skb(skb);

done:
- if (sk) bh_unlock_sock(sk);
+ if (sk)
+ bh_unlock_sock(sk);
return 0;
}

@@ -2648,7 +2663,8 @@ static int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 fl
}

/* Allocate skb for the complete frame (with header) */
- if (!(conn->rx_skb = bt_skb_alloc(len, GFP_ATOMIC)))
+ conn->rx_skb = bt_skb_alloc(len, GFP_ATOMIC);
+ if (!conn->rx_skb)
goto drop;

skb_copy_from_linear_data(skb, skb_put(conn->rx_skb, skb->len),
@@ -2708,7 +2724,7 @@ static ssize_t l2cap_sysfs_show(struct class *dev, char *buf)

read_unlock_bh(&l2cap_sk_list.lock);

- return (str - buf);
+ return str - buf;
}

static CLASS_ATTR(l2cap, S_IRUGO, l2cap_sysfs_show, NULL);
--
1.6.0.6


2009-03-28 17:47:28

by Gustavo F. Padovan

[permalink] [raw]
Subject: [PATCH 3/4] Bluetooth: create macro to hint mask on receiving config request

L2CAP_CONF_HINT is better to understand than 0x80. And we don't need to
go to spec to figure out what 0x80 is.

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
include/net/bluetooth/l2cap.h | 2 ++
net/bluetooth/l2cap.c | 2 +-
2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index ed4ba91..300b63f 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -149,6 +149,8 @@ struct l2cap_conf_opt {
} __attribute__ ((packed));
#define L2CAP_CONF_OPT_SIZE 2

+#define L2CAP_CONF_HINT 0x80
+
#define L2CAP_CONF_MTU 0x01
#define L2CAP_CONF_FLUSH_TO 0x02
#define L2CAP_CONF_QOS 0x03
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 85926f5..a2cc770 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1737,7 +1737,7 @@ static int l2cap_parse_conf_req(struct sock *sk, void *data)
while (len >= L2CAP_CONF_OPT_SIZE) {
len -= l2cap_get_conf_opt(&req, &type, &olen, &val);

- hint = type & 0x80;
+ hint = type & L2CAP_CONF_HINT;
type &= 0x7f;

switch (type) {
--
1.6.0.6


2009-03-28 17:47:27

by Gustavo F. Padovan

[permalink] [raw]
Subject: [PATCH 2/4] Bluetooth: remove unnecessary atribution to err var

The init value of err is not used until it is set to -ENOMEM. So we
initialize it directly to -ENOMEM

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/l2cap.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index ff1744e..85926f5 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -844,7 +844,7 @@ static int l2cap_do_connect(struct sock *sk)
struct hci_conn *hcon;
struct hci_dev *hdev;
__u8 auth_type;
- int err = 0;
+ int err = -ENOMEM;

BT_DBG("%s -> %s psm 0x%2.2x", batostr(src), batostr(dst),
l2cap_pi(sk)->psm);
@@ -854,8 +854,6 @@ static int l2cap_do_connect(struct sock *sk)

hci_dev_lock_bh(hdev);

- err = -ENOMEM;
-
if (sk->sk_type == SOCK_RAW) {
switch (l2cap_pi(sk)->sec_level) {
case BT_SECURITY_HIGH:
--
1.6.0.6


2009-03-28 17:47:26

by Gustavo F. Padovan

[permalink] [raw]
Subject: [PATCH 1/4] Bluetooth: create macros for channels identifiers

Use macros instead of hardcoded numbers to get a code more readable.

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
include/net/bluetooth/l2cap.h | 6 ++++++
net/bluetooth/l2cap.c | 18 +++++++++---------
2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index f566aa1..ed4ba91 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -106,6 +106,12 @@ struct l2cap_conn_rsp {
__le16 status;
} __attribute__ ((packed));

+/* channel indentifier */
+#define L2CAP_CID_SIGNALING 0x0001
+#define L2CAP_CID_CONN_LESS 0x0002
+#define L2CAP_CID_DYN_START 0x0040
+#define L2CAP_CID_DYN_END 0xffff
+
/* connect result */
#define L2CAP_CR_SUCCESS 0x0000
#define L2CAP_CR_PEND 0x0001
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index ca4d3b4..ff1744e 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -161,9 +161,9 @@ static inline struct sock *l2cap_get_chan_by_ident(struct l2cap_chan_list *l, u8

static u16 l2cap_alloc_cid(struct l2cap_chan_list *l)
{
- u16 cid = 0x0040;
+ u16 cid = L2CAP_CID_DYN_START;

- for (; cid < 0xffff; cid++) {
+ for (; cid < L2CAP_CID_DYN_END; cid++) {
if(!__l2cap_get_chan_by_scid(l, cid))
return cid;
}
@@ -215,13 +215,13 @@ static void __l2cap_chan_add(struct l2cap_conn *conn, struct sock *sk, struct so
l2cap_pi(sk)->scid = l2cap_alloc_cid(l);
} else if (sk->sk_type == SOCK_DGRAM) {
/* Connectionless socket */
- l2cap_pi(sk)->scid = 0x0002;
- l2cap_pi(sk)->dcid = 0x0002;
+ l2cap_pi(sk)->scid = L2CAP_CID_CONN_LESS;
+ l2cap_pi(sk)->dcid = L2CAP_CID_CONN_LESS;
l2cap_pi(sk)->omtu = L2CAP_DEFAULT_MTU;
} else {
/* Raw socket can send/recv signalling messages only */
- l2cap_pi(sk)->scid = 0x0001;
- l2cap_pi(sk)->dcid = 0x0001;
+ l2cap_pi(sk)->scid = L2CAP_CID_SIGNALING;
+ l2cap_pi(sk)->dcid = L2CAP_CID_SIGNALING;
l2cap_pi(sk)->omtu = L2CAP_DEFAULT_MTU;
}

@@ -1598,7 +1598,7 @@ static struct sk_buff *l2cap_build_cmd(struct l2cap_conn *conn,

lh = (struct l2cap_hdr *) skb_put(skb, L2CAP_HDR_SIZE);
lh->len = cpu_to_le16(L2CAP_CMD_HDR_SIZE + dlen);
- lh->cid = cpu_to_le16(0x0001);
+ lh->cid = cpu_to_le16(L2CAP_CID_SIGNALING);

cmd = (struct l2cap_cmd_hdr *) skb_put(skb, L2CAP_CMD_HDR_SIZE);
cmd->code = code;
@@ -2420,11 +2420,11 @@ static void l2cap_recv_frame(struct l2cap_conn *conn, struct sk_buff *skb)
BT_DBG("len %d, cid 0x%4.4x", len, cid);

switch (cid) {
- case 0x0001:
+ case L2CAP_CID_SIGNALING:
l2cap_sig_channel(conn, skb);
break;

- case 0x0002:
+ case L2CAP_CID_CONN_LESS:
psm = get_unaligned((__le16 *) skb->data);
skb_pull(skb, 2);
l2cap_conless_channel(conn, psm, skb);
--
1.6.0.6