commit 9d4990a260ce395493848640bed94fb55f440f10
Dave,
Here is another batch of fixes intended for 3.3. Most of the fixes
this time are for Bluetooth.
Quoth Johan Hedberg <[email protected]>:
"Here's one (hopefully) last set of important Bluetooth fixes intended
for 3.3:
- One important SSP fix which fixes security level upgrade handling
(this was breaking connections with many devices).
- An update to btusb fixing invalid flag usage with usb_submit_urb() as
well as one new vendor ID for a Broadcom device.
- One compilation warning fix for a bogus inline declaration
- Fix proper timeout value for l2cap_set_timer and sk_sndtimeo and
hci_conn_put
- Fix a lockdep warning with Bluetooth sockets
- Deadlock fix in hci_conn_hold
- RFCOMM reference counting fix
- QUIRK_NO_RESET fix which was breaking suspend with some laptops
- Two fixes related to synchronization with cancel_delayed_work()"
Along with the Bluetooth fixes, we have a handful of wireless LAN
fixes. First is an mwifiex fix from Amitkumar Karwar that does
proper initializations to ensure that associations can occur in
all conditions. Next is a mac80211 fix from Felix Fietkau to add a
check to rate_control_tx_status that avoids a crash in minstrel_ht.
This pairs with a mac80211 fix from Johannes Berg to ensure that
mac80211 rate control is initialized properly before being called.
Finally, we have an ath9k fix from Pavel Roskin to ensure that ath9k's
rate control processes rate control information in Tx status reports
properly.
Please let me know if there are problems!
Thanks,
John
---
The following changes since commit 64f0a836f600e9c31ffd511713ab5d328aa96ac8:
b44: remove __exit from b44_pci_exit() (2012-02-19 18:57:51 -0500)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless.git for-davem
Amitkumar Karwar (1):
mwifiex: clear previous security setting during association
Andre Guedes (1):
Bluetooth: Fix potential deadlock
Andrzej Kaczmarek (2):
Bluetooth: Fix sk_sndtimeo initialization for L2CAP socket
Bluetooth: l2cap_set_timer needs jiffies as timeout value
Daniel Wagner (1):
Bluetooth: Don't mark non xfer isoc endpoint URBs with URB_ISO_ASAP
Felix Fietkau (1):
mac80211: do not call rate control .tx_status before .rate_init
Johan Hedberg (2):
Bluetooth: Remove bogus inline declaration from l2cap_chan_connect
Bluetooth: Add missing QUIRK_NO_RESET test to hci_dev_do_close
Johannes Berg (1):
mac80211: call rate control only after init
John W. Linville (1):
Merge branch 'master' of git://git.kernel.org/.../linville/wireless into for-davem
Manoj Iyer (1):
Bluetooth: btusb: Add vendor specific ID (0a5c 21f3) for BCM20702A0
Octavian Purdila (2):
Bluetooth: silence lockdep warning
Bluetooth: Fix RFCOMM session reference counting issue
Pavel Roskin (1):
ath9k: stop on rates with idx -1 in ath9k rate control's .tx_status
Peter Hurley (1):
Bluetooth: Fix l2cap conn failures for ssp devices
Ulisses Furquim (2):
Bluetooth: Remove usage of __cancel_delayed_work()
Bluetooth: Fix possible use after free in delete path
Vinicius Costa Gomes (1):
Bluetooth: Fix using an absolute timeout on hci_conn_put()
drivers/bluetooth/btusb.c | 4 +---
drivers/net/wireless/ath/ath9k/rc.c | 2 +-
drivers/net/wireless/mwifiex/cfg80211.c | 8 +++++++-
include/net/bluetooth/bluetooth.h | 2 ++
include/net/bluetooth/hci_core.h | 6 +++---
include/net/bluetooth/l2cap.h | 12 ++++++------
net/bluetooth/af_bluetooth.c | 12 +++++-------
net/bluetooth/hci_conn.c | 4 ++++
net/bluetooth/hci_core.c | 3 ++-
net/bluetooth/l2cap_core.c | 24 ++++++++++++++----------
net/bluetooth/l2cap_sock.c | 4 +++-
net/bluetooth/rfcomm/core.c | 18 ++++++++++++------
net/bluetooth/rfcomm/sock.c | 2 ++
net/mac80211/debugfs_sta.c | 4 ++--
net/mac80211/rate.c | 2 +-
net/mac80211/rate.h | 3 ++-
net/mac80211/sta_info.h | 2 ++
17 files changed, 69 insertions(+), 43 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index f00f596..789c9b5 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -102,6 +102,7 @@ static struct usb_device_id btusb_table[] = {
/* Broadcom BCM20702A0 */
{ USB_DEVICE(0x0a5c, 0x21e3) },
+ { USB_DEVICE(0x0a5c, 0x21f3) },
{ USB_DEVICE(0x413c, 0x8197) },
{ } /* Terminating entry */
@@ -726,9 +727,6 @@ static int btusb_send_frame(struct sk_buff *skb)
usb_fill_bulk_urb(urb, data->udev, pipe,
skb->data, skb->len, btusb_tx_complete, skb);
- if (skb->priority >= HCI_PRIO_MAX - 1)
- urb->transfer_flags = URB_ISO_ASAP;
-
hdev->stat.acl_tx++;
break;
diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index 635b592..a427a16 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -1346,7 +1346,7 @@ static void ath_tx_status(void *priv, struct ieee80211_supported_band *sband,
fc = hdr->frame_control;
for (i = 0; i < sc->hw->max_rates; i++) {
struct ieee80211_tx_rate *rate = &tx_info->status.rates[i];
- if (!rate->count)
+ if (rate->idx < 0 || !rate->count)
break;
final_ts_idx = i;
diff --git a/drivers/net/wireless/mwifiex/cfg80211.c b/drivers/net/wireless/mwifiex/cfg80211.c
index c3b6c46..5b2972b 100644
--- a/drivers/net/wireless/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/mwifiex/cfg80211.c
@@ -841,7 +841,12 @@ mwifiex_cfg80211_assoc(struct mwifiex_private *priv, size_t ssid_len, u8 *ssid,
ret = mwifiex_set_rf_channel(priv, channel,
priv->adapter->channel_type);
- ret = mwifiex_set_encode(priv, NULL, 0, 0, 1); /* Disable keys */
+ /* As this is new association, clear locally stored
+ * keys and security related flags */
+ priv->sec_info.wpa_enabled = false;
+ priv->sec_info.wpa2_enabled = false;
+ priv->wep_key_curr_index = 0;
+ ret = mwifiex_set_encode(priv, NULL, 0, 0, 1);
if (mode == NL80211_IFTYPE_ADHOC) {
/* "privacy" is set only for ad-hoc mode */
@@ -886,6 +891,7 @@ mwifiex_cfg80211_assoc(struct mwifiex_private *priv, size_t ssid_len, u8 *ssid,
dev_dbg(priv->adapter->dev,
"info: setting wep encryption"
" with key len %d\n", sme->key_len);
+ priv->wep_key_curr_index = sme->key_idx;
ret = mwifiex_set_encode(priv, sme->key, sme->key_len,
sme->key_idx, 0);
}
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index abaad6e..4a82ca0 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -256,4 +256,6 @@ void l2cap_exit(void);
int sco_init(void);
void sco_exit(void);
+void bt_sock_reclassify_lock(struct sock *sk, int proto);
+
#endif /* __BLUETOOTH_H */
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ea9231f..453893b 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -540,7 +540,7 @@ void hci_conn_put_device(struct hci_conn *conn);
static inline void hci_conn_hold(struct hci_conn *conn)
{
atomic_inc(&conn->refcnt);
- cancel_delayed_work_sync(&conn->disc_work);
+ cancel_delayed_work(&conn->disc_work);
}
static inline void hci_conn_put(struct hci_conn *conn)
@@ -559,9 +559,9 @@ static inline void hci_conn_put(struct hci_conn *conn)
} else {
timeo = msecs_to_jiffies(10);
}
- cancel_delayed_work_sync(&conn->disc_work);
+ cancel_delayed_work(&conn->disc_work);
queue_delayed_work(conn->hdev->workqueue,
- &conn->disc_work, jiffies + timeo);
+ &conn->disc_work, timeo);
}
}
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 68f5891..b1664ed 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -611,7 +611,7 @@ static inline void l2cap_set_timer(struct l2cap_chan *chan,
{
BT_DBG("chan %p state %d timeout %ld", chan, chan->state, timeout);
- if (!__cancel_delayed_work(work))
+ if (!cancel_delayed_work(work))
l2cap_chan_hold(chan);
schedule_delayed_work(work, timeout);
}
@@ -619,20 +619,20 @@ static inline void l2cap_set_timer(struct l2cap_chan *chan,
static inline void l2cap_clear_timer(struct l2cap_chan *chan,
struct delayed_work *work)
{
- if (__cancel_delayed_work(work))
+ if (cancel_delayed_work(work))
l2cap_chan_put(chan);
}
#define __set_chan_timer(c, t) l2cap_set_timer(c, &c->chan_timer, (t))
#define __clear_chan_timer(c) l2cap_clear_timer(c, &c->chan_timer)
#define __set_retrans_timer(c) l2cap_set_timer(c, &c->retrans_timer, \
- L2CAP_DEFAULT_RETRANS_TO);
+ msecs_to_jiffies(L2CAP_DEFAULT_RETRANS_TO));
#define __clear_retrans_timer(c) l2cap_clear_timer(c, &c->retrans_timer)
#define __set_monitor_timer(c) l2cap_set_timer(c, &c->monitor_timer, \
- L2CAP_DEFAULT_MONITOR_TO);
+ msecs_to_jiffies(L2CAP_DEFAULT_MONITOR_TO));
#define __clear_monitor_timer(c) l2cap_clear_timer(c, &c->monitor_timer)
#define __set_ack_timer(c) l2cap_set_timer(c, &chan->ack_timer, \
- L2CAP_DEFAULT_ACK_TO);
+ msecs_to_jiffies(L2CAP_DEFAULT_ACK_TO));
#define __clear_ack_timer(c) l2cap_clear_timer(c, &c->ack_timer)
static inline int __seq_offset(struct l2cap_chan *chan, __u16 seq1, __u16 seq2)
@@ -834,7 +834,7 @@ int l2cap_add_scid(struct l2cap_chan *chan, __u16 scid);
struct l2cap_chan *l2cap_chan_create(struct sock *sk);
void l2cap_chan_close(struct l2cap_chan *chan, int reason);
void l2cap_chan_destroy(struct l2cap_chan *chan);
-inline int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
+int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
bdaddr_t *dst);
int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
u32 priority);
diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index ef92864..72eb187 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -71,19 +71,16 @@ static const char *const bt_slock_key_strings[BT_MAX_PROTO] = {
"slock-AF_BLUETOOTH-BTPROTO_AVDTP",
};
-static inline void bt_sock_reclassify_lock(struct socket *sock, int proto)
+void bt_sock_reclassify_lock(struct sock *sk, int proto)
{
- struct sock *sk = sock->sk;
-
- if (!sk)
- return;
-
+ BUG_ON(!sk);
BUG_ON(sock_owned_by_user(sk));
sock_lock_init_class_and_name(sk,
bt_slock_key_strings[proto], &bt_slock_key[proto],
bt_key_strings[proto], &bt_lock_key[proto]);
}
+EXPORT_SYMBOL(bt_sock_reclassify_lock);
int bt_sock_register(int proto, const struct net_proto_family *ops)
{
@@ -145,7 +142,8 @@ static int bt_sock_create(struct net *net, struct socket *sock, int proto,
if (bt_proto[proto] && try_module_get(bt_proto[proto]->owner)) {
err = bt_proto[proto]->create(net, sock, proto, kern);
- bt_sock_reclassify_lock(sock, proto);
+ if (!err)
+ bt_sock_reclassify_lock(sock->sk, proto);
module_put(bt_proto[proto]->owner);
}
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 3db4324..07bc69e 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -635,6 +635,10 @@ static int hci_conn_auth(struct hci_conn *conn, __u8 sec_level, __u8 auth_type)
if (!test_and_set_bit(HCI_CONN_AUTH_PEND, &conn->pend)) {
struct hci_cp_auth_requested cp;
+
+ /* encrypt must be pending if auth is also pending */
+ set_bit(HCI_CONN_ENCRYPT_PEND, &conn->pend);
+
cp.handle = cpu_to_le16(conn->handle);
hci_send_cmd(conn->hdev, HCI_OP_AUTH_REQUESTED,
sizeof(cp), &cp);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 9de9371..5aeb624 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -640,7 +640,8 @@ static int hci_dev_do_close(struct hci_dev *hdev)
/* Reset device */
skb_queue_purge(&hdev->cmd_q);
atomic_set(&hdev->cmd_cnt, 1);
- if (!test_bit(HCI_RAW, &hdev->flags)) {
+ if (!test_bit(HCI_RAW, &hdev->flags) &&
+ test_bit(HCI_QUIRK_NO_RESET, &hdev->quirks)) {
set_bit(HCI_INIT, &hdev->flags);
__hci_request(hdev, hci_reset_req, 0,
msecs_to_jiffies(250));
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index faf0b11..32d338c 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1018,10 +1018,10 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
hci_chan_del(conn->hchan);
if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_SENT)
- __cancel_delayed_work(&conn->info_timer);
+ cancel_delayed_work_sync(&conn->info_timer);
if (test_and_clear_bit(HCI_CONN_LE_SMP_PEND, &hcon->pend)) {
- __cancel_delayed_work(&conn->security_timer);
+ cancel_delayed_work_sync(&conn->security_timer);
smp_chan_destroy(conn);
}
@@ -1120,7 +1120,7 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm, bdaddr
return c1;
}
-inline int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, bdaddr_t *dst)
+int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, bdaddr_t *dst)
{
struct sock *sk = chan->sk;
bdaddr_t *src = &bt_sk(sk)->src;
@@ -2574,7 +2574,7 @@ static inline int l2cap_command_rej(struct l2cap_conn *conn, struct l2cap_cmd_hd
if ((conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_SENT) &&
cmd->ident == conn->info_ident) {
- __cancel_delayed_work(&conn->info_timer);
+ cancel_delayed_work(&conn->info_timer);
conn->info_state |= L2CAP_INFO_FEAT_MASK_REQ_DONE;
conn->info_ident = 0;
@@ -2970,7 +2970,8 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
default:
sk->sk_err = ECONNRESET;
- __set_chan_timer(chan, L2CAP_DISC_REJ_TIMEOUT);
+ __set_chan_timer(chan,
+ msecs_to_jiffies(L2CAP_DISC_REJ_TIMEOUT));
l2cap_send_disconn_req(conn, chan, ECONNRESET);
goto done;
}
@@ -3120,7 +3121,7 @@ static inline int l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm
conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_DONE)
return 0;
- __cancel_delayed_work(&conn->info_timer);
+ cancel_delayed_work(&conn->info_timer);
if (result != L2CAP_IR_SUCCESS) {
conn->info_state |= L2CAP_INFO_FEAT_MASK_REQ_DONE;
@@ -4478,7 +4479,8 @@ static inline void l2cap_check_encryption(struct l2cap_chan *chan, u8 encrypt)
if (encrypt == 0x00) {
if (chan->sec_level == BT_SECURITY_MEDIUM) {
__clear_chan_timer(chan);
- __set_chan_timer(chan, L2CAP_ENC_TIMEOUT);
+ __set_chan_timer(chan,
+ msecs_to_jiffies(L2CAP_ENC_TIMEOUT));
} else if (chan->sec_level == BT_SECURITY_HIGH)
l2cap_chan_close(chan, ECONNREFUSED);
} else {
@@ -4499,7 +4501,7 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
if (hcon->type == LE_LINK) {
smp_distribute_keys(conn, 0);
- __cancel_delayed_work(&conn->security_timer);
+ cancel_delayed_work(&conn->security_timer);
}
rcu_read_lock();
@@ -4546,7 +4548,8 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
L2CAP_CONN_REQ, sizeof(req), &req);
} else {
__clear_chan_timer(chan);
- __set_chan_timer(chan, L2CAP_DISC_TIMEOUT);
+ __set_chan_timer(chan,
+ msecs_to_jiffies(L2CAP_DISC_TIMEOUT));
}
} else if (chan->state == BT_CONNECT2) {
struct l2cap_conn_rsp rsp;
@@ -4566,7 +4569,8 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
}
} else {
l2cap_state_change(chan, BT_DISCONN);
- __set_chan_timer(chan, L2CAP_DISC_TIMEOUT);
+ __set_chan_timer(chan,
+ msecs_to_jiffies(L2CAP_DISC_TIMEOUT));
res = L2CAP_CR_SEC_BLOCK;
stat = L2CAP_CS_NO_INFO;
}
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index c61d967..401d942 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -849,6 +849,8 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(void *data)
if (!sk)
return NULL;
+ bt_sock_reclassify_lock(sk, BTPROTO_L2CAP);
+
l2cap_sock_init(sk, parent);
return l2cap_pi(sk)->chan;
@@ -1002,7 +1004,7 @@ static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, int p
INIT_LIST_HEAD(&bt_sk(sk)->accept_q);
sk->sk_destruct = l2cap_sock_destruct;
- sk->sk_sndtimeo = L2CAP_CONN_TIMEOUT;
+ sk->sk_sndtimeo = msecs_to_jiffies(L2CAP_CONN_TIMEOUT);
sock_reset_flag(sk, SOCK_ZAPPED);
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 501649b..8a60238 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -1164,12 +1164,18 @@ static int rfcomm_recv_ua(struct rfcomm_session *s, u8 dlci)
break;
case BT_DISCONN:
- /* When socket is closed and we are not RFCOMM
- * initiator rfcomm_process_rx already calls
- * rfcomm_session_put() */
- if (s->sock->sk->sk_state != BT_CLOSED)
- if (list_empty(&s->dlcs))
- rfcomm_session_put(s);
+ /* rfcomm_session_put is called later so don't do
+ * anything here otherwise we will mess up the session
+ * reference counter:
+ *
+ * (a) when we are the initiator dlc_unlink will drive
+ * the reference counter to 0 (there is no initial put
+ * after session_add)
+ *
+ * (b) when we are not the initiator rfcomm_rx_process
+ * will explicitly call put to balance the initial hold
+ * done after session add.
+ */
break;
}
}
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index f066678..22169c3 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -956,6 +956,8 @@ int rfcomm_connect_ind(struct rfcomm_session *s, u8 channel, struct rfcomm_dlc *
if (!sk)
goto done;
+ bt_sock_reclassify_lock(sk, BTPROTO_RFCOMM);
+
rfcomm_sock_init(sk, parent);
bacpy(&bt_sk(sk)->src, &src);
bacpy(&bt_sk(sk)->dst, &dst);
diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
index 2406b3e..d86217d 100644
--- a/net/mac80211/debugfs_sta.c
+++ b/net/mac80211/debugfs_sta.c
@@ -63,14 +63,14 @@ static ssize_t sta_flags_read(struct file *file, char __user *userbuf,
test_sta_flag(sta, WLAN_STA_##flg) ? #flg "\n" : ""
int res = scnprintf(buf, sizeof(buf),
- "%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s",
+ "%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s",
TEST(AUTH), TEST(ASSOC), TEST(PS_STA),
TEST(PS_DRIVER), TEST(AUTHORIZED),
TEST(SHORT_PREAMBLE),
TEST(WME), TEST(WDS), TEST(CLEAR_PS_FILT),
TEST(MFP), TEST(BLOCK_BA), TEST(PSPOLL),
TEST(UAPSD), TEST(SP), TEST(TDLS_PEER),
- TEST(TDLS_PEER_AUTH));
+ TEST(TDLS_PEER_AUTH), TEST(RATE_CONTROL));
#undef TEST
return simple_read_from_buffer(userbuf, count, ppos, buf, res);
}
diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c
index 5a5a776..ad64f4d 100644
--- a/net/mac80211/rate.c
+++ b/net/mac80211/rate.c
@@ -336,7 +336,7 @@ void rate_control_get_rate(struct ieee80211_sub_if_data *sdata,
int i;
u32 mask;
- if (sta) {
+ if (sta && test_sta_flag(sta, WLAN_STA_RATE_CONTROL)) {
ista = &sta->sta;
priv_sta = sta->rate_ctrl_priv;
}
diff --git a/net/mac80211/rate.h b/net/mac80211/rate.h
index 168427b..80cfc00 100644
--- a/net/mac80211/rate.h
+++ b/net/mac80211/rate.h
@@ -41,7 +41,7 @@ static inline void rate_control_tx_status(struct ieee80211_local *local,
struct ieee80211_sta *ista = &sta->sta;
void *priv_sta = sta->rate_ctrl_priv;
- if (!ref)
+ if (!ref || !test_sta_flag(sta, WLAN_STA_RATE_CONTROL))
return;
ref->ops->tx_status(ref->priv, sband, ista, priv_sta, skb);
@@ -62,6 +62,7 @@ static inline void rate_control_rate_init(struct sta_info *sta)
sband = local->hw.wiphy->bands[local->hw.conf.channel->band];
ref->ops->rate_init(ref->priv, sband, ista, priv_sta);
+ set_sta_flag(sta, WLAN_STA_RATE_CONTROL);
}
static inline void rate_control_rate_update(struct ieee80211_local *local,
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 6f77f12..bfed851 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -52,6 +52,7 @@
* @WLAN_STA_SP: Station is in a service period, so don't try to
* reply to other uAPSD trigger frames or PS-Poll.
* @WLAN_STA_4ADDR_EVENT: 4-addr event was already sent for this frame.
+ * @WLAN_STA_RATE_CONTROL: rate control was initialized for this station.
*/
enum ieee80211_sta_info_flags {
WLAN_STA_AUTH,
@@ -71,6 +72,7 @@ enum ieee80211_sta_info_flags {
WLAN_STA_UAPSD,
WLAN_STA_SP,
WLAN_STA_4ADDR_EVENT,
+ WLAN_STA_RATE_CONTROL,
};
enum ieee80211_sta_state {
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.
PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBsaW51eC1rZXJuZWwtb3duZXJA
dmdlci5rZXJuZWwub3JnIFttYWlsdG86bGludXgta2VybmVsLQ0KPiBvd25lckB2Z2VyLmtlcm5l
bC5vcmddIE9uIEJlaGFsZiBPZiBKb2UgUGVyY2hlcw0KPiBTZW50OiBUdWVzZGF5LCBGZWJydWFy
eSAyMSwgMjAxMiAxMjo1OSBQTQ0KPiBUbzogRGF2aWQgTWlsbGVyOyBBbmR5IFdoaXRjcm9mdDsg
QW5kcmV3IE1vcnRvbg0KPiBDYzogYW5kcmVpLmVtZWx0Y2hlbmtvLm5ld3NAZ21haWwuY29tOyBs
aW52aWxsZUB0dXhkcml2ZXIuY29tOyBsaW51eC0NCj4gd2lyZWxlc3NAdmdlci5rZXJuZWwub3Jn
OyBuZXRkZXZAdmdlci5rZXJuZWwub3JnOyBsaW51eC0NCj4ga2VybmVsQHZnZXIua2VybmVsLm9y
Zw0KPiBTdWJqZWN0OiBbUEFUQ0hdIGNoZWNrcGF0Y2g6IEFkZCBzb21lIC0tc3RyaWN0IGNvZGlu
ZyBzdHlsZSBjaGVja3MNCj4gDQo+IEFyZ3VtZW50IGFsaWdubWVudCBhY3Jvc3MgbXVsdGlwbGUg
bGluZXMgc2hvdWxkDQo+IG1hdGNoIHRoZSBvcGVuIHBhcmVudGhlc2lzLg0KPiANCj4gTG9naWNh
bCBjb250aW51YXRpb25zIHNob3VsZCBiZSBhdCB0aGUgZW5kIG9mDQo+IHRoZSBwcmV2aW91cyBs
aW5lLCBub3QgdGhlIHN0YXJ0IG9mIGEgbmV3IGxpbmUuDQo+IA0KPiBUaGVzZSBhcmUgbm90IHJl
cXVpcmVkIGJ5IENvZGluZ1N0eWxlIHNvIG1ha2UgdGhlDQo+IHRlc3RzIGFjdGl2ZSBvbmx5IHdo
ZW4gdXNpbmcgLS1zdHJpY3QuDQo+IA0KPiBUZXN0ZWQgd2l0aDoNCj4gaW50IGZvbyh2b2lkKQ0K
PiB7DQo+IAlpZiAoZm9vICYmDQo+IAkgICAgYmFyKCkpDQo+IAkJYmF6KCk7DQo+IA0KPiAJaWYg
KGZvbyAmJg0KPiAJICAgICBiYXIoKSkNCj4gCQliYXooKTsNCj4gDQo+IAlmb29fc29tZV9sb25n
X2Z1bmN0aW9uKGJhciwNCj4gCQkJICAgICAgIGJheik7DQo+IA0KPiAJZm9vX3NvbWVfbG9uZ19m
dW5jdGlvbihiYXIsDQo+IAkJYmF6KTsNCj4gDQo+IAlyZXR1cm4gMDsNCj4gfQ0KPiANCj4gU2ln
bmVkLW9mZi1ieTogSm9lIFBlcmNoZXMgPGpvZUBwZXJjaGVzLmNvbT4NCj4gLS0tDQo+ICBzY3Jp
cHRzL2NoZWNrcGF0Y2gucGwgfCAgIDIyICsrKysrKysrKysrKysrKysrKysrKysNCj4gIDEgZmls
ZXMgY2hhbmdlZCwgMjIgaW5zZXJ0aW9ucygrKSwgMCBkZWxldGlvbnMoLSkNCj4gDQo+IGRpZmYg
LS1naXQgYS9zY3JpcHRzL2NoZWNrcGF0Y2gucGwgYi9zY3JpcHRzL2NoZWNrcGF0Y2gucGwNCj4g
aW5kZXggYTNiOTc4Mi4uNjI5OTQ0ZSAxMDA3NTUNCj4gLS0tIGEvc2NyaXB0cy9jaGVja3BhdGNo
LnBsDQo+ICsrKyBiL3NjcmlwdHMvY2hlY2twYXRjaC5wbA0KPiBAQCAtMTc4Myw2ICsxNzgzLDI4
IEBAIHN1YiBwcm9jZXNzIHsNCj4gIAkJCSAgICAgInBsZWFzZSwgbm8gc3BhY2UgYmVmb3JlIHRh
YnNcbiIgLiAkaGVyZXZldCk7DQo+ICAJCX0NCj4gDQo+ICsjIGNoZWNrIGZvciAmJiBvciB8fCBh
dCB0aGUgc3RhcnQgb2YgYSBsaW5lDQo+ICsJCWlmICgkcmF3bGluZSA9fiAvXlwrXHMqKCYmfFx8
XHwpLykgew0KPiArCQkJQ0hLKCJMT0dJQ0FMX0NPTlRJTlVBVElPTlMiLA0KPiArCQkJICAgICJM
b2dpY2FsIGNvbnRpbnVhdGlvbnMgc2hvdWxkIGJlIG9uIHRoZSBwcmV2aW91cw0KPiBsaW5lXG4i
IC4gJGhlcmVwcmV2KTsNCj4gKwkJfQ0KPiArDQo+ICsjIGNoZWNrIG11bHRpLWxpbmUgc3RhdGVt
ZW50IGluZGVudGF0aW9uIG1hdGNoZXMgcHJldmlvdXMgbGluZQ0KPiArCQlpZiAoJHByZXZsaW5l
ID1+IC9eXCsoXHQqKShpZg0KPiBcKHwkSWRlbnRcKCkuKihcJlwmfFx8XHx8LClccyokLyAmJiAk
cmF3bGluZSA9fiAvXlwrKFsgXHRdKikvKSB7DQo+ICsJCQkkcHJldmxpbmUgPX4gL15cKyhcdCop
KGlmDQo+IFwofCRJZGVudFwoKS4qKFwmXCZ8XHxcfHwsKVxzKiQvOw0KPiArCQkJbXkgJG9sZGlu
ZGVudCA9ICQxOw0KPiArCQkJbXkgJGlmX29yX2Z1bmMgPSAkMjsNCj4gKwkJCSRyYXdsaW5lID1+
IC9eXCsoWyBcdF0qKS87DQo+ICsJCQlteSAkbmV3aW5kZW50ID0gJDE7DQo+ICsJCQlteSAkZ29v
ZGluZGVudCA9ICRvbGRpbmRlbnQgLg0KPiArCQkJCQkgIlx0IiB4IChsZW5ndGgoJGlmX29yX2Z1
bmMpIC8gOCkgLg0KPiArCQkJCQkgIiAiICB4IChsZW5ndGgoJGlmX29yX2Z1bmMpICUgOCk7DQo+
ICsJCQlpZiAoJG5ld2luZGVudCBuZSAiJGdvb2RpbmRlbnQiKSB7DQo+ICsJCQkJQ0hLKCJQQVJF
TlRIRVNJU19BTElHTk1FTlQiLA0KPiArCQkJCSAgICAiQWxpZ25tZW50IHNob3VsZCBtYXRjaCBv
cGVuDQo+IHBhcmVudGhlc2lzXG4iIC4gJGhlcmVwcmV2KTsNCj4gKwkJCX0NCj4gKwkJfQ0KPiAr
DQo+ICAjIGNoZWNrIGZvciBzcGFjZXMgYXQgdGhlIGJlZ2lubmluZyBvZiBhIGxpbmUuDQo+ICAj
IEV4Y2VwdGlvbnM6DQo+ICAjICAxKSB3aXRoaW4gY29tbWVudHMNCg0KVGhpcyBhcHBlYXJzIHRv
IGZhbHNlbHkgY29tcGxhaW4gYWJvdXQgcGFyZW50aGVzaXMgYWxpZ25tZW50IGluIGNvbmRpdGlv
bmFsIHN0YXRlbWVudHMgd2l0aCBtdWx0aXBsZSBvcGVuaW5nIHBhcmVudGhlc2VzLiAgRm9yIGV4
YW1wbGUsIHRoZXNlIHdpbGwgcmVwb3J0IGEgY2hlY2sgY29uZGl0aW9uOg0KDQoJaWYgKHRlc3Rf
YW5kX3NldF9iaXQobnIsDQoJCQkJICAgYWRkcikpDQoJCWJheigpOw0KDQoJaWYgKCEoZnVuY19h
KHgpICYmDQoJCWZ1bmNfYih5KSkpDQoJCWJheigpOw0KDQpBc3N1bWluZyBteSBzdHVwaWQgbWFp
bGVyIHdpbGwgc2NyZXcgdXAgdGhlIGluZGVudGF0aW9uIGFib3ZlLCB0aGUgJ2EnIGluIGFkZHIg
aW4gdGhlIGZpcnN0IGV4YW1wbGUgaXMgbWVhbnQgdG8gYmUgaW1tZWRpYXRlbHkgYmVsb3cgdGhl
ICduJyBpbiBuciwgYW5kIHRoZSB0d28gJ2YncyBpbiBmdW5jXyogYXJlIG1lYW50IHRvIGJlIHZl
cnRpY2FsbHkgbGluZWQgdXAgaW4gdGhlIHNlY29uZCBleGFtcGxlLg0KDQoNCg==
Argument alignment across multiple lines should
match the open parenthesis.
Logical continuations should be at the end of
the previous line, not the start of a new line.
These are not required by CodingStyle so make the
tests active only when using --strict.
Tested with:
int foo(void)
{
if (foo &&
bar())
baz();
if (foo &&
bar())
baz();
foo_some_long_function(bar,
baz);
foo_some_long_function(bar,
baz);
return 0;
}
Signed-off-by: Joe Perches <[email protected]>
---
scripts/checkpatch.pl | 22 ++++++++++++++++++++++
1 files changed, 22 insertions(+), 0 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index a3b9782..629944e 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1783,6 +1783,28 @@ sub process {
"please, no space before tabs\n" . $herevet);
}
+# check for && or || at the start of a line
+ if ($rawline =~ /^\+\s*(&&|\|\|)/) {
+ CHK("LOGICAL_CONTINUATIONS",
+ "Logical continuations should be on the previous line\n" . $hereprev);
+ }
+
+# check multi-line statement indentation matches previous line
+ if ($prevline =~ /^\+(\t*)(if \(|$Ident\().*(\&\&|\|\||,)\s*$/ && $rawline =~ /^\+([ \t]*)/) {
+ $prevline =~ /^\+(\t*)(if \(|$Ident\().*(\&\&|\|\||,)\s*$/;
+ my $oldindent = $1;
+ my $if_or_func = $2;
+ $rawline =~ /^\+([ \t]*)/;
+ my $newindent = $1;
+ my $goodindent = $oldindent .
+ "\t" x (length($if_or_func) / 8) .
+ " " x (length($if_or_func) % 8);
+ if ($newindent ne "$goodindent") {
+ CHK("PARENTHESIS_ALIGNMENT",
+ "Alignment should match open parenthesis\n" . $hereprev);
+ }
+ }
+
# check for spaces at the beginning of a line.
# Exceptions:
# 1) within comments
On Tue, 2012-02-21 at 22:09 +0000, Allan, Bruce W wrote:
> This appears to falsely complain about parenthesis alignment in
> conditional statements with multiple opening parentheses. For
> example, these will report a check condition:
>
> if (test_and_set_bit(nr,
> addr))
> baz();
>
> if (!(func_a(x) &&
> func_b(y)))
> baz();
>
> Assuming my stupid mailer will screw up the indentation above, the 'a'
> in addr in the first example is meant to be immediately below the 'n'
> in nr, and the two 'f's in func_* are meant to be vertically lined up
> in the second example.
You're right, thanks for testing.
The logic I used is too trivial.
Andrew, please ditch this one for awhile.
PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBKb2UgUGVyY2hlcyBbbWFpbHRv
OmpvZUBwZXJjaGVzLmNvbV0NCj4gU2VudDogVHVlc2RheSwgRmVicnVhcnkgMjEsIDIwMTIgNToz
NiBQTQ0KPiBUbzogQWxsYW4sIEJydWNlIFcNCj4gQ2M6IERhdmlkIE1pbGxlcjsgQW5keSBXaGl0
Y3JvZnQ7IEFuZHJldyBNb3J0b247DQo+IGFuZHJlaS5lbWVsdGNoZW5rby5uZXdzQGdtYWlsLmNv
bTsgbGludmlsbGVAdHV4ZHJpdmVyLmNvbTsgbGludXgtDQo+IHdpcmVsZXNzQHZnZXIua2VybmVs
Lm9yZzsgbmV0ZGV2QHZnZXIua2VybmVsLm9yZzsgbGludXgtDQo+IGtlcm5lbEB2Z2VyLmtlcm5l
bC5vcmcNCj4gU3ViamVjdDogUkU6IFtQQVRDSF0gY2hlY2twYXRjaDogQWRkIHNvbWUgLS1zdHJp
Y3QgY29kaW5nIHN0eWxlIGNoZWNrcw0KPiANCj4gT24gVHVlLCAyMDEyLTAyLTIxIGF0IDIyOjA5
ICswMDAwLCBBbGxhbiwgQnJ1Y2UgVyB3cm90ZToNCj4gPiBUaGlzIGFwcGVhcnMgdG8gZmFsc2Vs
eSBjb21wbGFpbiBhYm91dCBwYXJlbnRoZXNpcyBhbGlnbm1lbnQgaW4NCj4gPiBjb25kaXRpb25h
bCBzdGF0ZW1lbnRzIHdpdGggbXVsdGlwbGUgb3BlbmluZyBwYXJlbnRoZXNlcy4NCj4gDQo+IEhl
eSBBbGxhbi4NCj4gDQo+IENhbiB5b3UgdHJ5IHRoaXMgb25lIHBsZWFzZT8NCj4gLS0tDQo+ICBz
Y3JpcHRzL2NoZWNrcGF0Y2gucGwgfCAgIDU4DQo+ICsrKysrKysrKysrKysrKysrKysrKysrKysr
KysrKysrKysrKysrKysrKysrKysrKysNCj4gIDEgZmlsZXMgY2hhbmdlZCwgNTggaW5zZXJ0aW9u
cygrKSwgMCBkZWxldGlvbnMoLSkNCj4gDQo+IGRpZmYgLS1naXQgYS9zY3JpcHRzL2NoZWNrcGF0
Y2gucGwgYi9zY3JpcHRzL2NoZWNrcGF0Y2gucGwNCj4gaW5kZXggYTNiOTc4Mi4uZTk1NDE5ZSAx
MDA3NTUNCj4gLS0tIGEvc2NyaXB0cy9jaGVja3BhdGNoLnBsDQo+ICsrKyBiL3NjcmlwdHMvY2hl
Y2twYXRjaC5wbA0KPiBAQCAtMTMzMCw2ICsxMzMwLDM1IEBAIHN1YiBjaGVja19hYnNvbHV0ZV9m
aWxlIHsNCj4gIAl9DQo+ICB9DQo+IA0KPiArc3ViIHBvc19sYXN0X29wZW5wYXJlbiB7DQo+ICsN
Cj4gKwlteSAoJGFyZ3MpID0gQF87DQo+ICsNCj4gKwlteSAkcG9zID0gMDsNCj4gKwlteSAkbGVu
ID0gbGVuZ3RoKCRhcmdzKTsNCj4gKw0KPiArCW15ICRvcGVucyA9ICRhcmdzID1+IHRyL1woL1wo
LzsNCj4gKwlteSAkY2xvc2VzID0gJGFyZ3MgPX4gdHIvXCkvXCkvOw0KPiArDQo+ICsJbXkgJGxh
c3Rfb3BlbnBhcmVuID0gMDsNCj4gKw0KPiArCWlmICgoJG9wZW5zID09IDApIHx8ICgkY2xvc2Vz
ID49ICRvcGVucykpIHsNCj4gKwkJcmV0dXJuIDA7DQo+ICsJfQ0KPiArDQo+ICsJZm9yICgkcG9z
ID0gMDsgJHBvcyA8ICRsZW47ICRwb3MrKykgew0KPiArCQlpZiAoc3Vic3RyKCRhcmdzLCAkcG9z
KSA9fiAvXigkRnVuY0FyZykvKSB7DQo+ICsJCQkkcG9zICs9IGxlbmd0aCgkMSk7DQo+ICsJCX0N
Cj4gKw0KPiArCQlpZiAoc3Vic3RyKCRhcmdzLCAkcG9zLCAxKSBlcSAnKCcpIHsNCj4gKwkJCSRs
YXN0X29wZW5wYXJlbiA9ICRwb3M7DQo+ICsJCX0NCj4gKwl9DQo+ICsNCj4gKwlyZXR1cm4gJGxh
c3Rfb3BlbnBhcmVuICsgMTsNCj4gK30NCj4gKw0KPiAgc3ViIHByb2Nlc3Mgew0KPiAgCW15ICRm
aWxlbmFtZSA9IHNoaWZ0Ow0KPiANCj4gQEAgLTE3ODMsNiArMTgxMiwzNSBAQCBzdWIgcHJvY2Vz
cyB7DQo+ICAJCQkgICAgICJwbGVhc2UsIG5vIHNwYWNlIGJlZm9yZSB0YWJzXG4iIC4gJGhlcmV2
ZXQpOw0KPiAgCQl9DQo+IA0KPiArIyBjaGVjayBmb3IgJiYgb3IgfHwgYXQgdGhlIHN0YXJ0IG9m
IGEgbGluZQ0KPiArCQlpZiAoJHJhd2xpbmUgPX4gL15cK1xzKigmJnxcfFx8KS8pIHsNCj4gKwkJ
CUNISygiTE9HSUNBTF9DT05USU5VQVRJT05TIiwNCj4gKwkJCSAgICAiTG9naWNhbCBjb250aW51
YXRpb25zIHNob3VsZCBiZSBvbiB0aGUgcHJldmlvdXMNCj4gbGluZVxuIiAuICRoZXJlcHJldik7
DQo+ICsJCX0NCj4gKw0KPiArIyBjaGVjayBtdWx0aS1saW5lIHN0YXRlbWVudCBpbmRlbnRhdGlv
biBtYXRjaGVzIHByZXZpb3VzIGxpbmUNCj4gKwkJaWYgKCRwcmV2bGluZSA9fiAvXlwrKFx0Kiko
aWYNCj4gXCh8JElkZW50XCgpLiooXCZcJnxcfFx8fCwpXHMqJC8gJiYgJHJhd2xpbmUgPX4gL15c
KyhbIFx0XSopLykgew0KPiArCQkJJHByZXZsaW5lID1+IC9eXCsoXHQqKShpZg0KPiBcKHwkSWRl
bnRcKCkoLiopKFwmXCZ8XHxcfHwsKVxzKiQvOw0KPiArCQkJbXkgJG9sZGluZGVudCA9ICQxOw0K
PiArCQkJbXkgJGlmX29yX2Z1bmMgPSAkMjsNCj4gKwkJCW15ICRhcmdzID0gJDM7DQo+ICsNCj4g
KwkJCW15ICRwb3MgPSBwb3NfbGFzdF9vcGVucGFyZW4oJGFyZ3MpOw0KPiArDQo+ICsJCQlteSAk
bGVuID0gbGVuZ3RoKCRpZl9vcl9mdW5jKSArICRwb3M7DQo+ICsJCQkkcmF3bGluZSA9fiAvXlwr
KFsgXHRdKikvOw0KPiArCQkJbXkgJG5ld2luZGVudCA9ICQxOw0KPiArDQo+ICsJCQlteSAkZ29v
ZGluZGVudCA9ICRvbGRpbmRlbnQgLg0KPiArCQkJCQkgIlx0IiB4ICgkbGVuIC8gOCkgLg0KPiAr
CQkJCQkgIiAiICB4ICgkbGVuICUgOCk7DQo+ICsNCj4gKwkJCWlmICgkbmV3aW5kZW50IG5lICRn
b29kaW5kZW50KSB7DQo+ICsJCQkJQ0hLKCJQQVJFTlRIRVNJU19BTElHTk1FTlQiLA0KPiArCQkJ
CSAgICAiQWxpZ25tZW50IHNob3VsZCBtYXRjaCBvcGVuDQo+IHBhcmVudGhlc2lzXG4iIC4gJGhl
cmVwcmV2KTsNCj4gKwkJCX0NCj4gKwkJfQ0KPiArDQo+ICAjIGNoZWNrIGZvciBzcGFjZXMgYXQg
dGhlIGJlZ2lubmluZyBvZiBhIGxpbmUuDQo+ICAjIEV4Y2VwdGlvbnM6DQo+ICAjICAxKSB3aXRo
aW4gY29tbWVudHMNCj4gDQoNCkl0J3MgYmV0dGVyLCBidXQgdGhlcmUgYXJlIHN0aWxsIGluc3Rh
bmNlcyBvZiBmYWxzZSBoaXRzIEFGQUlDVC4NCg0KQnJ1Y2UuDQoNCg==
On Tue, 2012-02-21 at 22:09 +0000, Allan, Bruce W wrote:
> This appears to falsely complain about parenthesis alignment in
> conditional statements with multiple opening parentheses.
Hey Allan.
Can you try this one please?
---
scripts/checkpatch.pl | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 58 insertions(+), 0 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index a3b9782..e95419e 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1330,6 +1330,35 @@ sub check_absolute_file {
}
}
+sub pos_last_openparen {
+
+ my ($args) = @_;
+
+ my $pos = 0;
+ my $len = length($args);
+
+ my $opens = $args =~ tr/\(/\(/;
+ my $closes = $args =~ tr/\)/\)/;
+
+ my $last_openparen = 0;
+
+ if (($opens == 0) || ($closes >= $opens)) {
+ return 0;
+ }
+
+ for ($pos = 0; $pos < $len; $pos++) {
+ if (substr($args, $pos) =~ /^($FuncArg)/) {
+ $pos += length($1);
+ }
+
+ if (substr($args, $pos, 1) eq '(') {
+ $last_openparen = $pos;
+ }
+ }
+
+ return $last_openparen + 1;
+}
+
sub process {
my $filename = shift;
@@ -1783,6 +1812,35 @@ sub process {
"please, no space before tabs\n" . $herevet);
}
+# check for && or || at the start of a line
+ if ($rawline =~ /^\+\s*(&&|\|\|)/) {
+ CHK("LOGICAL_CONTINUATIONS",
+ "Logical continuations should be on the previous line\n" . $hereprev);
+ }
+
+# check multi-line statement indentation matches previous line
+ if ($prevline =~ /^\+(\t*)(if \(|$Ident\().*(\&\&|\|\||,)\s*$/ && $rawline =~ /^\+([ \t]*)/) {
+ $prevline =~ /^\+(\t*)(if \(|$Ident\()(.*)(\&\&|\|\||,)\s*$/;
+ my $oldindent = $1;
+ my $if_or_func = $2;
+ my $args = $3;
+
+ my $pos = pos_last_openparen($args);
+
+ my $len = length($if_or_func) + $pos;
+ $rawline =~ /^\+([ \t]*)/;
+ my $newindent = $1;
+
+ my $goodindent = $oldindent .
+ "\t" x ($len / 8) .
+ " " x ($len % 8);
+
+ if ($newindent ne $goodindent) {
+ CHK("PARENTHESIS_ALIGNMENT",
+ "Alignment should match open parenthesis\n" . $hereprev);
+ }
+ }
+
# check for spaces at the beginning of a line.
# Exceptions:
# 1) within comments
Hi David,
On Tue, Feb 21, 2012 at 9:44 PM, David Miller <[email protected]> wrote:
> From: "John W. Linville" <[email protected]>
> Date: Tue, 21 Feb 2012 10:14:36 -0500
>
>> On Mon, Feb 20, 2012 at 07:23:24PM -0500, David Miller wrote:
>>> From: "John W. Linville" <[email protected]>
>>> Date: Mon, 20 Feb 2012 15:37:40 -0500
>>>
>>> > Here is another batch of fixes intended for 3.3. ?Most of the fixes
>>> > this time are for Bluetooth.
>>>
>>> Pulled, but please read the bluetooth changes more carefully in the
>>> future, there were a lot of coding style errors introduced this
>>> time around.
>>
>> I pinged Johan about this, and he tells me that he spoke to Marcel
>> as well. ?They were a bit unsure about the issue. ?Is your concern
>> primarily about some excessive tabbing for indentation of parameters
>> and such?
>
> So you actually looked at the changes you pushed to me and you are
> telling me you personally can't find anything that looks like garbage?
>
> Really? ?Do I really have to point out such obvious stuff like this?
> Are you serious?
>
> Look at ca0d6c7ece0e78268cd7c5c378d6b1b610625085 ("Bluetooth: Add
> missing QUIRK_NO_RESET test to hci_dev_do_close")
>
> You tell me what the heck you think of this thing.
>
> - ? ? ? if (!test_bit(HCI_RAW, &hdev->flags)) {
> + ? ? ? if (!test_bit(HCI_RAW, &hdev->flags) &&
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? test_bit(HCI_QUIRK_NO_RESET, &hdev->quirks)) {
>
> That's disgusting, it jumps off the screen and says "Hello, I am ugly
> as sin". ?How in the world can you miss something like this? ?Four
> TABs on the second line? ?Why? ?I can't believe we even have to discuss
> something like this, seriously.
Sorry did we understand wrong text of Linux coding style? Or do we need
right interpretation of it?
http://www.kernel.org/doc/Documentation/CodingStyle
Statements longer than 80 columns will be broken into sensible chunks.
Descendants are always substantially shorter than the parent and are placed
substantially to the right. The same applies to function headers with a long
argument list. Long strings are as well broken into shorter strings. The
only exception to this is where exceeding 80 columns significantly increases
readability and does not hide information.
void fun(int a, int b, int c)
{
if (condition)
printk(KERN_WARNING "Warning this is a long printk with "
"3 parameters a: %u b: %u "
"c: %u \n", a, b, c);
else
next_statement;
}
>
> Commit 6e1da683f79a22fafaada62d547138daaa9e3456 ("Bluetooth: l2cap_set_timer
> needs jiffies as timeout value"):
>
> - ? ? ? ? ? ? ? __set_chan_timer(chan, L2CAP_DISC_REJ_TIMEOUT);
> + ? ? ? ? ? ? ? __set_chan_timer(chan,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? msecs_to_jiffies(L2CAP_DISC_REJ_TIMEOUT));
>
> You can't see that mis-tabbed thing on the second new line? ?Really?
Are those lines longer then 80 characters?
Otherwise same applies here.
Regards,
Andrei
On 02/24/2012 08:37 PM, Joe Perches wrote:
> Argument alignment across multiple lines should
> match the open parenthesis.
>
> Logical continuations should be at the end of
> the previous line, not the start of a new line.
>
> These are not required by CodingStyle so make the
> tests active only when using --strict.
>
> Improved_by_examples_from: "Bruce W. Allen" <[email protected]>
> Signed-off-by: Joe Perches <[email protected]>
Thanks Joe, this is very useful for me. I seem to have one false alarm
though:
drivers/net/wireless/ath/ath6kl/txrx.c:464: CHECK: Alignment should
match open parenthesis
This is with patches I haven't sent yet so line numbers most likely
don't match, but the code in question is this:
if (!IS_ALIGNED((unsigned long) skb->data - HTC_HDR_LENGTH, 4) &&
skb_cloned(skb)) {
Apparently the cast "(unsigned long)" causes the false alarm as when I
remove it I don't see the warning anymore.
Kalle
> if (!(func_a(x) &&
> func_b(y)))
> baz();
Gah - that is horrid for the 'preferred style'.
A quick glance at the code puts both the func_b()
and baz() calls as inside the if.
David
PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBKb2UgUGVyY2hlcyBbbWFpbHRv
OmpvZUBwZXJjaGVzLmNvbV0NCj4gU2VudDogVHVlc2RheSwgRmVicnVhcnkgMjEsIDIwMTIgNTo1
OSBQTQ0KPiBUbzogQWxsYW4sIEJydWNlIFcNCj4gQ2M6IERhdmlkIE1pbGxlcjsgQW5keSBXaGl0
Y3JvZnQ7IEFuZHJldyBNb3J0b247DQo+IGFuZHJlaS5lbWVsdGNoZW5rby5uZXdzQGdtYWlsLmNv
bTsgbGludmlsbGVAdHV4ZHJpdmVyLmNvbTsgbGludXgtDQo+IHdpcmVsZXNzQHZnZXIua2VybmVs
Lm9yZzsgbmV0ZGV2QHZnZXIua2VybmVsLm9yZzsgbGludXgtDQo+IGtlcm5lbEB2Z2VyLmtlcm5l
bC5vcmcNCj4gU3ViamVjdDogUkU6IFtQQVRDSF0gY2hlY2twYXRjaDogQWRkIHNvbWUgLS1zdHJp
Y3QgY29kaW5nIHN0eWxlIGNoZWNrcw0KPiANCj4gT24gV2VkLCAyMDEyLTAyLTIyIGF0IDAxOjU2
ICswMDAwLCBBbGxhbiwgQnJ1Y2UgVyB3cm90ZToNCj4gPiA+IE9uIFR1ZSwgMjAxMi0wMi0yMSBh
dCAyMjowOSArMDAwMCwgQWxsYW4sIEJydWNlIFcgd3JvdGU6DQo+ID4gPiA+IFRoaXMgYXBwZWFy
cyB0byBmYWxzZWx5IGNvbXBsYWluIGFib3V0IHBhcmVudGhlc2lzIGFsaWdubWVudCBpbg0KPiA+
ID4gPiBjb25kaXRpb25hbCBzdGF0ZW1lbnRzIHdpdGggbXVsdGlwbGUgb3BlbmluZyBwYXJlbnRo
ZXNlcy4NCj4gPiA+IENhbiB5b3UgdHJ5IHRoaXMgb25lIHBsZWFzZT8NCj4gW10NCj4gPiBJdCdz
IGJldHRlciwgYnV0IHRoZXJlIGFyZSBzdGlsbCBpbnN0YW5jZXMgb2YgZmFsc2UgaGl0cyBBRkFJ
Q1QuDQo+IA0KPiBEbyB5b3UgaGF2ZSBhbnkgZXhhbXBsZXM/DQo+IFRoZSBvbmx5IG9uZSBJIG5v
dGljZWQgd2FzIHNwYWNlcyBmb3IgYWxpZ25tZW50IGluc3RlYWQgb2YgdGFicy4NCj4gSSB0aGlu
ayB0aGF0J3Mgbm90IGEgZmFsc2UgaGl0IG15c2VsZi4NCj4gDQoNCkV4YW1wbGUgMToNCglpZiAo
KChhID09IGIpIHx8DQoJICAgICAoYyA9PSBkKSB8fA0KCSAgICAgKGUgPT0gZikpICYmDQoJICAg
IChib29sX3ZhcikpDQoJCWJheigpOw0KDQpFeGFtcGxlIDI6DQoJaWYgKCghKHZhciAmIEZPT19N
QVNLKSAmJg0KCSAgICAgKGEgPT0gYikpIHx8DQoJICAgIChjID09IGQpKQ0KCQliYXooKTsNCg0K
RXhhbXBsZSAzOg0KCWlmICghKChmb28gJiBGT09fTUFTSykgJiYNCgkgICAgICAoYmFyICYgQkFS
X01BU0spKSkNCgkJYmF6KCk7DQoNCkhUSCwNCkJydWNlLg0K
From: Andrei Emeltchenko <[email protected]>
Date: Tue, 21 Feb 2012 22:38:16 +0200
> Hi David,
>
> On Tue, Feb 21, 2012 at 9:44 PM, David Miller <[email protected]> wrote:
>> From: "John W. Linville" <[email protected]>
>> Date: Tue, 21 Feb 2012 10:14:36 -0500
>>
>>> On Mon, Feb 20, 2012 at 07:23:24PM -0500, David Miller wrote:
>>>> From: "John W. Linville" <[email protected]>
>>>> Date: Mon, 20 Feb 2012 15:37:40 -0500
>>>>
>>>> > Here is another batch of fixes intended for 3.3. ?Most of the fixes
>>>> > this time are for Bluetooth.
>>>>
>>>> Pulled, but please read the bluetooth changes more carefully in the
>>>> future, there were a lot of coding style errors introduced this
>>>> time around.
>>>
>>> I pinged Johan about this, and he tells me that he spoke to Marcel
>>> as well. ?They were a bit unsure about the issue. ?Is your concern
>>> primarily about some excessive tabbing for indentation of parameters
>>> and such?
>>
>> So you actually looked at the changes you pushed to me and you are
>> telling me you personally can't find anything that looks like garbage?
>>
>> Really? ?Do I really have to point out such obvious stuff like this?
>> Are you serious?
>>
>> Look at ca0d6c7ece0e78268cd7c5c378d6b1b610625085 ("Bluetooth: Add
>> missing QUIRK_NO_RESET test to hci_dev_do_close")
>>
>> You tell me what the heck you think of this thing.
>>
>> - ? ? ? if (!test_bit(HCI_RAW, &hdev->flags)) {
>> + ? ? ? if (!test_bit(HCI_RAW, &hdev->flags) &&
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? test_bit(HCI_QUIRK_NO_RESET, &hdev->quirks)) {
>>
>> That's disgusting, it jumps off the screen and says "Hello, I am ugly
>> as sin". ?How in the world can you miss something like this? ?Four
>> TABs on the second line? ?Why? ?I can't believe we even have to discuss
>> something like this, seriously.
>
> Sorry did we understand wrong text of Linux coding style? Or do we need
> right interpretation of it?
I'm not engaging in this conversation any more, I don't care what CodingStyle
says, the quoted code is garbage.
Speaking of which, I just got this:
WARNING: %Ld/%Lu are not-standard C, use %lld/%llu
#142: FILE: fs/locks.c:2335:
+ seq_printf(f, "Start-end:\t %Ld-EOF\n\n", fl->fl_start);
But %L saves a byte! Why did we do this? Don't you like puppies?
On Tue, 2012-02-21 at 13:09 -0800, Andrew Morton wrote:
> Speaking of which, I just got this:
>
> WARNING: %Ld/%Lu are not-standard C, use %lld/%llu
> #142: FILE: fs/locks.c:2335:
> + seq_printf(f, "Start-end:\t %Ld-EOF\n\n", fl->fl_start);
>
> But %L saves a byte! Why did we do this? Don't you like puppies?
Beats me and I prefer kittens.
PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBKb2UgUGVyY2hlcyBbbWFpbHRv
OmpvZUBwZXJjaGVzLmNvbV0NCj4gU2VudDogV2VkbmVzZGF5LCBGZWJydWFyeSAyMiwgMjAxMiA3
OjQ0IFBNDQo+IFRvOiBBbGxhbiwgQnJ1Y2UgVw0KPiBDYzogRGF2aWQgTWlsbGVyOyBBbmR5IFdo
aXRjcm9mdDsgQW5kcmV3IE1vcnRvbjsNCj4gYW5kcmVpLmVtZWx0Y2hlbmtvLm5ld3NAZ21haWwu
Y29tOyBsaW52aWxsZUB0dXhkcml2ZXIuY29tOyBsaW51eC0NCj4gd2lyZWxlc3NAdmdlci5rZXJu
ZWwub3JnOyBuZXRkZXZAdmdlci5rZXJuZWwub3JnOyBsaW51eC0NCj4ga2VybmVsQHZnZXIua2Vy
bmVsLm9yZw0KPiBTdWJqZWN0OiBSRTogW1BBVENIXSBjaGVja3BhdGNoOiBBZGQgc29tZSAtLXN0
cmljdCBjb2Rpbmcgc3R5bGUgY2hlY2tzDQo+IA0KPiBIaSBCcnVjZS4gKHlheSwgSSBnb3QgeW91
ciBuYW1lIHJpZ2h0KQ0KPiANCj4gVGhhbmtzLg0KPiANCj4gSG93IGFib3V0IHRlc3RpbmcgdGhp
cz8NCj4gDQo+IEl0IGFsbG93cyBhbGwgc3BhY2VzIG9yIGFwcHJvcHJpYXRlIHRhYnMgZm9yIGlu
ZGVudGF0aW9uLA0KPiBhbmQgSSBiZWxpZXZlIGl0IHdvcmtzIE9LLg0KPiANCj4gLS0tDQo+ICBz
Y3JpcHRzL2NoZWNrcGF0Y2gucGwgfCAgIDY1DQo+ICsrKysrKysrKysrKysrKysrKysrKysrKysr
KysrKysrKysrKysrKysrKysrKysrLQ0KPiAgMSBmaWxlcyBjaGFuZ2VkLCA2MyBpbnNlcnRpb25z
KCspLCAyIGRlbGV0aW9ucygtKQ0KPiANCj4gZGlmZiAtLWdpdCBhL3NjcmlwdHMvY2hlY2twYXRj
aC5wbCBiL3NjcmlwdHMvY2hlY2twYXRjaC5wbA0KPiBpbmRleCA4OWQyNGIzLi43YTA1MTRiIDEw
MDc1NQ0KPiAtLS0gYS9zY3JpcHRzL2NoZWNrcGF0Y2gucGwNCj4gKysrIGIvc2NyaXB0cy9jaGVj
a3BhdGNoLnBsDQo+IEBAIC0zMzAsMTAgKzMzMCwxMSBAQCBzdWIgYnVpbGRfdHlwZXMgew0KPiAg
fQ0KPiAgYnVpbGRfdHlwZXMoKTsNCj4gDQo+IC1vdXIgJG1hdGNoX2JhbGFuY2VkX3BhcmVudGhl
c2VzID0gcXIvKFwoKD86W15cKFwpXSt8KC0xKSkqXCkpLzsNCj4gK291ciAkYmFsYW5jZWRfcGFy
ZW5zID0gcXIvKFwoKD86W15cKFwpXSsrfCg/LTEpKSpcKSkvOw0KPiArb3VyICRsdmFsX3BhcmVu
cyA9IHFyLyhcKCg/OlteXChcKV0rfCgtMSkpKlwpKS87DQo+IA0KPiAgb3VyICRUeXBlY2FzdAk9
IHFye1xzKihcKFxzKiROb25wdHJUeXBlXHMqXCkpezAsMX1ccyp9Ow0KPiAtb3VyICRMdmFsT3JG
dW5jCT0NCj4gcXJ7KCRMdmFsKVxzKigkbWF0Y2hfYmFsYW5jZWRfcGFyZW50aGVzZXN7MCwxfSlc
cyp9Ow0KPiArb3VyICRMdmFsT3JGdW5jCT0gcXJ7KCRMdmFsKVxzKigkbHZhbF9wYXJlbnN7MCwx
fSlccyp9Ow0KPiAgb3VyICRGdW5jQXJnID0gcXJ7JFR5cGVjYXN0ezAsMX0oJEx2YWxPckZ1bmN8
JENvbnN0YW50KX07DQo+IA0KPiAgc3ViIGRlcGFyZW50aGVzaXplIHsNCj4gQEAgLTEzMzAsNiAr
MTMzMSwzNiBAQCBzdWIgY2hlY2tfYWJzb2x1dGVfZmlsZSB7DQo+ICAJfQ0KPiAgfQ0KPiANCj4g
K3N1YiBwb3NfbGFzdF9vcGVucGFyZW4gew0KPiArCW15ICgkbGluZSkgPSBAXzsNCj4gKw0KPiAr
CW15ICRwb3MgPSAwOw0KPiArDQo+ICsJbXkgJG9wZW5zID0gJGxpbmUgPX4gdHIvXCgvXCgvOw0K
PiArCW15ICRjbG9zZXMgPSAkbGluZSA9fiB0ci9cKS9cKS87DQo+ICsNCj4gKwlteSAkbGFzdF9v
cGVucGFyZW4gPSAwOw0KPiArDQo+ICsJaWYgKCgkb3BlbnMgPT0gMCkgfHwgKCRjbG9zZXMgPj0g
JG9wZW5zKSkgew0KPiArCQlyZXR1cm4gLTE7DQo+ICsJfQ0KPiArDQo+ICsJbXkgJGxlbiA9IGxl
bmd0aCgkbGluZSk7DQo+ICsNCj4gKwlmb3IgKCRwb3MgPSAwOyAkcG9zIDwgJGxlbjsgJHBvcysr
KSB7DQo+ICsJICAgIG15ICRzdHJpbmcgPSBzdWJzdHIoJGxpbmUsICRwb3MpOw0KPiArCSAgICBp
ZiAoJHN0cmluZyA9fiAvXigkRnVuY0FyZ3wkYmFsYW5jZWRfcGFyZW5zKS8pIHsNCj4gKwkJICAg
ICRwb3MgKz0gbGVuZ3RoKCQxKTsNCj4gKwkgICAgfQ0KPiArDQo+ICsJICAgIGlmIChzdWJzdHIo
JGxpbmUsICRwb3MsIDEpIGVxICcoJykgew0KPiArCQkkbGFzdF9vcGVucGFyZW4gPSAkcG9zOw0K
PiArCSAgICB9DQo+ICsJfQ0KPiArDQo+ICsJcmV0dXJuICRsYXN0X29wZW5wYXJlbiArIDE7DQo+
ICt9DQo+ICsNCj4gIHN1YiBwcm9jZXNzIHsNCj4gIAlteSAkZmlsZW5hbWUgPSBzaGlmdDsNCj4g
DQo+IEBAIC0xNzgzLDYgKzE4MTQsMzYgQEAgc3ViIHByb2Nlc3Mgew0KPiAgCQkJICAgICAicGxl
YXNlLCBubyBzcGFjZSBiZWZvcmUgdGFic1xuIiAuICRoZXJldmV0KTsNCj4gIAkJfQ0KPiANCj4g
KyMgY2hlY2sgZm9yICYmIG9yIHx8IGF0IHRoZSBzdGFydCBvZiBhIGxpbmUNCj4gKwkJaWYgKCRy
YXdsaW5lID1+IC9eXCtccyooJiZ8XHxcfCkvKSB7DQo+ICsJCQlDSEsoIkxPR0lDQUxfQ09OVElO
VUFUSU9OUyIsDQo+ICsJCQkgICAgIkxvZ2ljYWwgY29udGludWF0aW9ucyBzaG91bGQgYmUgb24g
dGhlIHByZXZpb3VzDQo+IGxpbmVcbiIgLiAkaGVyZXByZXYpOw0KPiArCQl9DQo+ICsNCj4gKyMg
Y2hlY2sgbXVsdGktbGluZSBzdGF0ZW1lbnQgaW5kZW50YXRpb24gbWF0Y2hlcyBwcmV2aW91cyBs
aW5lDQo+ICsJCWlmICgkcHJldmxpbmUgPX4gL15cKyhcdCopKGlmDQo+IFwofCRJZGVudFwoKS4q
KFwmXCZ8XHxcfHwsKVxzKiQvKSB7DQo+ICsJCQkkcHJldmxpbmUgPX4gL15cKyhcdCopKC4qKSQv
Ow0KPiArCQkJbXkgJG9sZGluZGVudCA9ICQxOw0KPiArCQkJbXkgJHJlc3QgPSAkMjsNCj4gKw0K
PiArCQkJbXkgJHBvcyA9IHBvc19sYXN0X29wZW5wYXJlbigkcmVzdCk7DQo+ICsJCQlpZiAoJHBv
cyA+PSAwKSB7DQo+ICsJCQkJJGxpbmUgPX4gL15cKyhbIFx0XSopLzsNCj4gKwkJCQlteSAkbmV3
aW5kZW50ID0gJDE7DQo+ICsNCj4gKwkJCQlteSAkZ29vZHRhYmluZGVudCA9ICRvbGRpbmRlbnQg
Lg0KPiArCQkJCQkiXHQiIHggKCRwb3MgLyA4KSAuDQo+ICsJCQkJCSIgIiAgeCAoJHBvcyAlIDgp
Ow0KPiArCQkJCW15ICRnb29kc3BhY2VpbmRlbnQgPSAkb2xkaW5kZW50IC4gIiAiICB4DQo+ICRw
b3M7DQo+ICsNCj4gKwkJCQlpZiAoJG5ld2luZGVudCBuZSAkZ29vZHRhYmluZGVudCAmJg0KPiAr
CQkJCSAgICAkbmV3aW5kZW50IG5lICRnb29kc3BhY2VpbmRlbnQpIHsNCj4gKwkJCQkJQ0hLKCJQ
QVJFTlRIRVNJU19BTElHTk1FTlQiLA0KPiArCQkJCQkgICAgIkFsaWdubWVudCBzaG91bGQgbWF0
Y2ggb3Blbg0KPiBwYXJlbnRoZXNpc1xuIiAuICRoZXJlcHJldik7DQo+ICsJCQkJfQ0KPiArCQkJ
fQ0KPiArCQl9DQo+ICsNCj4gICMgY2hlY2sgZm9yIHNwYWNlcyBhdCB0aGUgYmVnaW5uaW5nIG9m
IGEgbGluZS4NCj4gICMgRXhjZXB0aW9uczoNCj4gICMgIDEpIHdpdGhpbiBjb21tZW50cw0KPiAN
Cg0KVGhpcyBsYXRlc3QgdmVyc2lvbiBpcyBtdWNoIGJldHRlci4gIEl0IGRvZXMgbm90IHJlcG9y
dCBmYWxzZSBwb3NpdGl2ZXMgaW4gdGhlDQpsaW1pdGVkIGNvZGUgSSBjaGVja2VkIGl0IGFnYWlu
c3QuDQoNCkJydWNlLg0K
On Mon, Feb 20, 2012 at 07:23:24PM -0500, David Miller wrote:
> From: "John W. Linville" <[email protected]>
> Date: Mon, 20 Feb 2012 15:37:40 -0500
>
> > Here is another batch of fixes intended for 3.3. Most of the fixes
> > this time are for Bluetooth.
>
> Pulled, but please read the bluetooth changes more carefully in the
> future, there were a lot of coding style errors introduced this
> time around.
I pinged Johan about this, and he tells me that he spoke to Marcel
as well. They were a bit unsure about the issue. Is your concern
primarily about some excessive tabbing for indentation of parameters
and such?
John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.
From: "John W. Linville" <[email protected]>
Date: Tue, 21 Feb 2012 10:14:36 -0500
> On Mon, Feb 20, 2012 at 07:23:24PM -0500, David Miller wrote:
>> From: "John W. Linville" <[email protected]>
>> Date: Mon, 20 Feb 2012 15:37:40 -0500
>>
>> > Here is another batch of fixes intended for 3.3. Most of the fixes
>> > this time are for Bluetooth.
>>
>> Pulled, but please read the bluetooth changes more carefully in the
>> future, there were a lot of coding style errors introduced this
>> time around.
>
> I pinged Johan about this, and he tells me that he spoke to Marcel
> as well. They were a bit unsure about the issue. Is your concern
> primarily about some excessive tabbing for indentation of parameters
> and such?
So you actually looked at the changes you pushed to me and you are
telling me you personally can't find anything that looks like garbage?
Really? Do I really have to point out such obvious stuff like this?
Are you serious?
Look at ca0d6c7ece0e78268cd7c5c378d6b1b610625085 ("Bluetooth: Add
missing QUIRK_NO_RESET test to hci_dev_do_close")
You tell me what the heck you think of this thing.
- if (!test_bit(HCI_RAW, &hdev->flags)) {
+ if (!test_bit(HCI_RAW, &hdev->flags) &&
+ test_bit(HCI_QUIRK_NO_RESET, &hdev->quirks)) {
That's disgusting, it jumps off the screen and says "Hello, I am ugly
as sin". How in the world can you miss something like this? Four
TABs on the second line? Why? I can't believe we even have to discuss
something like this, seriously.
Commit 6e1da683f79a22fafaada62d547138daaa9e3456 ("Bluetooth: l2cap_set_timer
needs jiffies as timeout value"):
- __set_chan_timer(chan, L2CAP_DISC_REJ_TIMEOUT);
+ __set_chan_timer(chan,
+ msecs_to_jiffies(L2CAP_DISC_REJ_TIMEOUT));
You can't see that mis-tabbed thing on the second new line? Really?
The list goes on and on, just walk through the bluetooth commits from
the other day, stuff like this is all over the place.
On 02/21/2012 09:40 PM, David Miller wrote:
> From: Andrei Emeltchenko <[email protected]>
> Date: Tue, 21 Feb 2012 22:38:16 +0200
>
>> Hi David,
>>
>> On Tue, Feb 21, 2012 at 9:44 PM, David Miller <[email protected]> wrote:
>>> From: "John W. Linville" <[email protected]>
>>> Date: Tue, 21 Feb 2012 10:14:36 -0500
>>> [...]
>>
>> Sorry did we understand wrong text of Linux coding style? Or do we need
>> right interpretation of it?
>
> I'm not engaging in this conversation any more, I don't care what CodingStyle
> says, the quoted code is garbage.
This is not really helpful, David.
Andrei's question was fully reasonable here, i.e. the Documentation/CodingStyle is the only binding reference given to potential contributors -- the common 'how-to-contribute-kernel-patches' docs non-24/7-kernel-folks usually find and follow on the way preparing patches end exactly there.
I recently learned the hard way when my patches were ignored just because of some missing spaces. That would be perfectly fair if some formal rules were broken, but not telling people about additional informal ones is not motivating.
I might be fully blind and just missed the related coding style document that covers the issues discussed here. But even then your answer should be 'RTFM' instead of just ending the conversation.
Cheers, Zefir
On Wed, 2012-02-22 at 01:56 +0000, Allan, Bruce W wrote:
> > On Tue, 2012-02-21 at 22:09 +0000, Allan, Bruce W wrote:
> > > This appears to falsely complain about parenthesis alignment in
> > > conditional statements with multiple opening parentheses.
> > Can you try this one please?
[]
> It's better, but there are still instances of false hits AFAICT.
Do you have any examples?
The only one I noticed was spaces for alignment instead of tabs.
I think that's not a false hit myself.
On Wed, 2012-02-22 at 09:35 +0000, David Laight wrote:
> > if (!(func_a(x) &&
> > func_b(y)))
> > baz();
>
> Gah - that is horrid for the 'preferred style'.
> A quick glance at the code puts both the func_b()
> and baz() calls as inside the if.
You forgot to read/quote the rest of Allan's email:
> Assuming my stupid mailer will screw up the indentation above, the 'a'
> in addr in the first example is meant to be immediately below the 'n'
> in nr, and the two 'f's in func_* are meant to be vertically lined up
> in the second example.
johannes
Argument alignment across multiple lines should
match the open parenthesis.
Logical continuations should be at the end of
the previous line, not the start of a new line.
These are not required by CodingStyle so make the
tests active only when using --strict.
Improved_by_examples_from: "Bruce W. Allen" <[email protected]>
Signed-off-by: Joe Perches <[email protected]>
---
V2: Scan the entire line for balanced parentheses,
use the position of the last non-balanced open parenthesis.
Allow all space indentation too, checkpatch will complain
in a different message about that if it's too long.
scripts/checkpatch.pl | 65 +++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 63 insertions(+), 2 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 89d24b3..7a0514b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -330,10 +330,11 @@ sub build_types {
}
build_types();
-our $match_balanced_parentheses = qr/(\((?:[^\(\)]+|(-1))*\))/;
+our $balanced_parens = qr/(\((?:[^\(\)]++|(?-1))*\))/;
+our $lval_parens = qr/(\((?:[^\(\)]+|(-1))*\))/;
our $Typecast = qr{\s*(\(\s*$NonptrType\s*\)){0,1}\s*};
-our $LvalOrFunc = qr{($Lval)\s*($match_balanced_parentheses{0,1})\s*};
+our $LvalOrFunc = qr{($Lval)\s*($lval_parens{0,1})\s*};
our $FuncArg = qr{$Typecast{0,1}($LvalOrFunc|$Constant)};
sub deparenthesize {
@@ -1330,6 +1331,36 @@ sub check_absolute_file {
}
}
+sub pos_last_openparen {
+ my ($line) = @_;
+
+ my $pos = 0;
+
+ my $opens = $line =~ tr/\(/\(/;
+ my $closes = $line =~ tr/\)/\)/;
+
+ my $last_openparen = 0;
+
+ if (($opens == 0) || ($closes >= $opens)) {
+ return -1;
+ }
+
+ my $len = length($line);
+
+ for ($pos = 0; $pos < $len; $pos++) {
+ my $string = substr($line, $pos);
+ if ($string =~ /^($FuncArg|$balanced_parens)/) {
+ $pos += length($1);
+ }
+
+ if (substr($line, $pos, 1) eq '(') {
+ $last_openparen = $pos;
+ }
+ }
+
+ return $last_openparen + 1;
+}
+
sub process {
my $filename = shift;
@@ -1783,6 +1814,36 @@ sub process {
"please, no space before tabs\n" . $herevet);
}
+# check for && or || at the start of a line
+ if ($rawline =~ /^\+\s*(&&|\|\|)/) {
+ CHK("LOGICAL_CONTINUATIONS",
+ "Logical continuations should be on the previous line\n" . $hereprev);
+ }
+
+# check multi-line statement indentation matches previous line
+ if ($prevline =~ /^\+(\t*)(if \(|$Ident\().*(\&\&|\|\||,)\s*$/) {
+ $prevline =~ /^\+(\t*)(.*)$/;
+ my $oldindent = $1;
+ my $rest = $2;
+
+ my $pos = pos_last_openparen($rest);
+ if ($pos >= 0) {
+ $line =~ /^\+([ \t]*)/;
+ my $newindent = $1;
+
+ my $goodtabindent = $oldindent .
+ "\t" x ($pos / 8) .
+ " " x ($pos % 8);
+ my $goodspaceindent = $oldindent . " " x $pos;
+
+ if ($newindent ne $goodtabindent &&
+ $newindent ne $goodspaceindent) {
+ CHK("PARENTHESIS_ALIGNMENT",
+ "Alignment should match open parenthesis\n" . $hereprev);
+ }
+ }
+ }
+
# check for spaces at the beginning of a line.
# Exceptions:
# 1) within comments
On Wed, 2012-02-22 at 02:17 +0000, Allan, Bruce W wrote:
> Example 1:
> if (((a == b) ||
> (c == d) ||
> (e == f)) &&
> (bool_var))
> baz();
>
> Example 2:
> if ((!(var & FOO_MASK) &&
> (a == b)) ||
> (c == d))
> baz();
>
> Example 3:
> if (!((foo & FOO_MASK) &&
> (bar & BAR_MASK)))
> baz();
>
Hi Bruce. (yay, I got your name right)
Thanks.
How about testing this?
It allows all spaces or appropriate tabs for indentation,
and I believe it works OK.
---
scripts/checkpatch.pl | 65 +++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 63 insertions(+), 2 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 89d24b3..7a0514b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -330,10 +330,11 @@ sub build_types {
}
build_types();
-our $match_balanced_parentheses = qr/(\((?:[^\(\)]+|(-1))*\))/;
+our $balanced_parens = qr/(\((?:[^\(\)]++|(?-1))*\))/;
+our $lval_parens = qr/(\((?:[^\(\)]+|(-1))*\))/;
our $Typecast = qr{\s*(\(\s*$NonptrType\s*\)){0,1}\s*};
-our $LvalOrFunc = qr{($Lval)\s*($match_balanced_parentheses{0,1})\s*};
+our $LvalOrFunc = qr{($Lval)\s*($lval_parens{0,1})\s*};
our $FuncArg = qr{$Typecast{0,1}($LvalOrFunc|$Constant)};
sub deparenthesize {
@@ -1330,6 +1331,36 @@ sub check_absolute_file {
}
}
+sub pos_last_openparen {
+ my ($line) = @_;
+
+ my $pos = 0;
+
+ my $opens = $line =~ tr/\(/\(/;
+ my $closes = $line =~ tr/\)/\)/;
+
+ my $last_openparen = 0;
+
+ if (($opens == 0) || ($closes >= $opens)) {
+ return -1;
+ }
+
+ my $len = length($line);
+
+ for ($pos = 0; $pos < $len; $pos++) {
+ my $string = substr($line, $pos);
+ if ($string =~ /^($FuncArg|$balanced_parens)/) {
+ $pos += length($1);
+ }
+
+ if (substr($line, $pos, 1) eq '(') {
+ $last_openparen = $pos;
+ }
+ }
+
+ return $last_openparen + 1;
+}
+
sub process {
my $filename = shift;
@@ -1783,6 +1814,36 @@ sub process {
"please, no space before tabs\n" . $herevet);
}
+# check for && or || at the start of a line
+ if ($rawline =~ /^\+\s*(&&|\|\|)/) {
+ CHK("LOGICAL_CONTINUATIONS",
+ "Logical continuations should be on the previous line\n" . $hereprev);
+ }
+
+# check multi-line statement indentation matches previous line
+ if ($prevline =~ /^\+(\t*)(if \(|$Ident\().*(\&\&|\|\||,)\s*$/) {
+ $prevline =~ /^\+(\t*)(.*)$/;
+ my $oldindent = $1;
+ my $rest = $2;
+
+ my $pos = pos_last_openparen($rest);
+ if ($pos >= 0) {
+ $line =~ /^\+([ \t]*)/;
+ my $newindent = $1;
+
+ my $goodtabindent = $oldindent .
+ "\t" x ($pos / 8) .
+ " " x ($pos % 8);
+ my $goodspaceindent = $oldindent . " " x $pos;
+
+ if ($newindent ne $goodtabindent &&
+ $newindent ne $goodspaceindent) {
+ CHK("PARENTHESIS_ALIGNMENT",
+ "Alignment should match open parenthesis\n" . $hereprev);
+ }
+ }
+ }
+
# check for spaces at the beginning of a line.
# Exceptions:
# 1) within comments
From: "John W. Linville" <[email protected]>
Date: Mon, 20 Feb 2012 15:37:40 -0500
> Here is another batch of fixes intended for 3.3. Most of the fixes
> this time are for Bluetooth.
Pulled, but please read the bluetooth changes more carefully in the
future, there were a lot of coding style errors introduced this
time around.