[PATCH 0/4] Bluetooth: l2cap clean ups

[PATCH 0/4] Bluetooth: l2cap clean ups

[Gustavo F. Padovan]

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

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

[Gustavo F. Padovan]

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

Signed-off-by: Gustavo F. Padovan <gustavo@las.ic.unicamp.br>
---
 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

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

[Gustavo F. Padovan]

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 <gustavo@las.ic.unicamp.br>
---
 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

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

[Gustavo F. Padovan]

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 <gustavo@las.ic.unicamp.br>
---
 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

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

[Gustavo F. Padovan]

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 <gustavo@las.ic.unicamp.br>
---
 net/bluetooth/l2cap.c |   70 ++++++++++++++++++++++++++++++-------------------
 1 files changed, 43 insertions(+), 27 deletions(-)

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

Re: [PATCH 0/4] Bluetooth: l2cap clean ups

[Marcel Holtmann]

Hi Gustavo,

there is no need to copy linux-kernel mailing list here.

> 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

I did minor modification (mostly commit message) and pushed them to
bluetooth-testing.git tree. Thanks.

Regards

Marcel

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

[Gustavo F. Padovan]

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 <gustavo@las.ic.unicamp.br>
---
 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