Skip to contentSkip to conversation

net: fix and typo's

PATCH In review Roel Kluin 3 participants 5 messages Oct 26, 2007

Subsystems (4)
Networking Network drivers Networking drivers (wireless) Arm/Microchip (AT91) SoC support
lore
Share & tools
lore

Roel Kluin

author
#0 lore raw

PATCH 1/1 net: fix and typo's

A few patches with changes to net code. I have sent these to the lkml previously, but they were not yet merged. I am fairly new to kernel programming, so it is possible that I make some mistakes. I'll explain my rationale, please nack if incorrect, an additional bit of explanation is appreciated even more.

The condition '!x & y,' does make little sense: the '!' has a higher priority than '&'. It behaves therefore like '!x && y'. In the case bitanding flags, however, '!(x & y)' appears to be desired.

Warning: the change of '!x & y,' to '!(x & y)' may change behavior. if not desired, I propose changing this to '!x && y', to make it explicitly clear.

These '&' typo's can be spotted with:
a="A-Za-z0-9_"
git-grep "\![^$a()]*[$a]\+\([$a.]*\|->\)*\(\[[$a.]*\]\)\?[ \W]*&[^&]\+"

--
Fix priority mistakes similar to '!x & y'

Signed-off-by Roel Kluin <[email protected]>

---

drivers/net/e1000e/82571.c +1 -1
index cf70522..14141a5 100644
@@ -283,7 +283,7 @@ static s32 e1000_get_invariants_82571(struct e1000_adapter *adapter)
 			adapter->flags &= ~FLAG_HAS_WOL;
 		/* quad ports only support WoL on port A */
 		if (adapter->flags & FLAG_IS_QUAD_PORT &&
-		    (!adapter->flags & FLAG_IS_QUAD_PORT_A))
+		    (!(adapter->flags & FLAG_IS_QUAD_PORT_A)))
 			adapter->flags &= ~FLAG_HAS_WOL;
 		break;
drivers/net/wireless/airo.c +1 -1
index 074055e..e3eca6d 100644
@@ -6407,7 +6407,7 @@ static int airo_set_encode(struct net_device *dev,
 			set_wep_key(local, index, NULL, 0, perm, 1);
 		} else
 			/* Don't complain if only change the mode */
-			if(!dwrq->flags & IW_ENCODE_MODE) {
+			if(!(dwrq->flags & IW_ENCODE_MODE)) {
 				return -EINVAL;
 			}
 	}
drivers/net/wireless/atmel.c +1 -1
index 059ce3f..57cc7e5 100644
@@ -1759,7 +1759,7 @@ static int atmel_set_encode(struct net_device *dev,
 			priv->default_key = index;
 		} else
 			/* Don't complain if only change the mode */
-			if (!dwrq->flags & IW_ENCODE_MODE) {
+			if (!(dwrq->flags & IW_ENCODE_MODE)) {
 				return -EINVAL;
 			}
 	}
drivers/net/wireless/libertas/wext.c +1 -1
index c6f5aa3..d93438c 100644
@@ -1380,7 +1380,7 @@ static int wlan_get_encodeext(struct net_device *dev,
 		index = adapter->wep_tx_keyidx;
 	}
 
-	if (!ext->ext_flags & IW_ENCODE_EXT_GROUP_KEY &&
+	if (!(ext->ext_flags & IW_ENCODE_EXT_GROUP_KEY) &&
 	    ext->alg != IW_ENCODE_ALG_WEP) {
 		if (index != 0 || adapter->mode != IW_MODE_INFRA)
 			goto out;
drivers/net/wireless/p54common.c +1 -1
index 1437db0..8ee1453 100644
@@ -374,7 +374,7 @@ static void p54_rx_frame_sent(struct ieee80211_hw *dev, struct sk_buff *skb)
 			if ((entry_hdr->magic1 & cpu_to_le16(0x4000)) != 0)
 				pad = entry_data->align[0];
 
-			if (!status.control.flags & IEEE80211_TXCTL_NO_ACK) {
+			if (!(status.control.flags & IEEE80211_TXCTL_NO_ACK)) {
 				if (!(payload->status & 0x01))
 					status.flags |= IEEE80211_TX_STATUS_ACK;
 				else
drivers/net/wireless/prism54/isl_ioctl.c +2 -2
index 6d80ca4..b9d0073 100644
@@ -1118,7 +1118,7 @@ prism54_set_encode(struct net_device *ndev, struct iw_request_info *info,
 			    mgt_set_request(priv, DOT11_OID_DEFKEYID, 0,
 					    &index);
 		} else {
-			if (!dwrq->flags & IW_ENCODE_MODE) {
+			if (!(dwrq->flags & IW_ENCODE_MODE)) {
 				/* we cannot do anything. Complain. */
 				return -EINVAL;
 			}

@@ -2610,7 +2610,7 @@ prism2_ioctl_set_encryption(struct net_device *dev,
 			    mgt_set_request(priv, DOT11_OID_DEFKEYID, 0,
 					    &index);
 		} else {
-			if (!param->u.crypt.flags & IW_ENCODE_MODE) {
+			if (!(param->u.crypt.flags & IW_ENCODE_MODE)) {
 				/* we cannot do anything. Complain. */
 				return -EINVAL;
 			}
drivers/net/wireless/zd1211rw/zd_rf_uw2453.c +1 -1
index 857dcf3..3c1cca4 100644
@@ -403,7 +403,7 @@ static int uw2453_init_hw(struct zd_rf *rf)
 		if (r)
 			return r;
 
-		if (!intr_status & 0xf) {
+		if (!(intr_status & 0xf)) {
 			dev_dbg_f(zd_chip_dev(chip),
 				"PLL locked on configuration %d\n", i);
 			found_config = i;
net/ieee80211/ieee80211_wx.c +1 -1
index d309e8f..623489a 100644
@@ -709,7 +709,7 @@ int ieee80211_wx_get_encodeext(struct ieee80211_device *ieee,
 	} else
 		idx = ieee->tx_keyidx;
 
-	if (!ext->ext_flags & IW_ENCODE_EXT_GROUP_KEY &&
+	if (!(ext->ext_flags & IW_ENCODE_EXT_GROUP_KEY) &&
 	    ext->alg != IW_ENCODE_ALG_WEP)
 		if (idx != 0 || ieee->iw_mode != IW_MODE_INFRA)
 			return -EINVAL;
#1 lore raw
Quoted 28 lines · Roel Kluin

A few patches with changes to net code. I have sent these to the lkml previously, but they were not yet merged. I am fairly new to kernel programming, so it is possible that I make some mistakes. I'll explain my rationale, please nack if incorrect, an additional bit of explanation is appreciated even more.

The condition '!x & y,' does make little sense: the '!' has a higher priority than '&'. It behaves therefore like '!x && y'. In the case bitanding flags, however, '!(x & y)' appears to be desired.

Warning: the change of '!x & y,' to '!(x & y)' may change behavior. if not desired, I propose changing this to '!x && y', to make it explicitly clear.

These '&' typo's can be spotted with:
a="A-Za-z0-9_"
git-grep "\![^$a()]*[$a]\+\([$a.]*\|->\)*\(\[[$a.]*\]\)\?[ \W]*&[^&]\+"

--
Fix priority mistakes similar to '!x & y'

Signed-off-by Roel Kluin <[email protected]>

---

drivers/net/e1000e/82571.c +0 -0
index cf70522..14141a5 100644

drivers/net/e1000e/82571.c static s32 e1000_get_invariants_82571(struct e1000_adapter *adapter) · line 289

Quoted 8 lines
drivers/net/e1000e/82571.c +1 -1
@@ -283,7 +283,7 @@ static s32 e1000_get_invariants_82571(struct e1000_adapter *adapter)
 			adapter->flags &= ~FLAG_HAS_WOL;
 		/* quad ports only support WoL on port A */
 		if (adapter->flags & FLAG_IS_QUAD_PORT &&
-		    (!adapter->flags & FLAG_IS_QUAD_PORT_A))
+		    (!(adapter->flags & FLAG_IS_QUAD_PORT_A)))
 			adapter->flags &= ~FLAG_HAS_WOL;
 		break;

Ack this e1000e change here!

Auke

(PS since there was only 1 netdriver patch here and the rest is wireless, I would have suggested splitting this patch up in two and sending them to the wireless maintainer and netdevice maintainer separately. But I'm sure this will get picked up anyway.)

author
#2 lore raw
Quoted 6 lines · Auke

Ack this e1000e change here!

(PS since there was only 1 netdriver patch here and the rest is wireless, I would have suggested splitting this patch up in two and sending them to the wireless maintainer and netdevice maintainer separately. But I'm sure this will get picked up anyway.)

Ok, I've split the patch and sent the rest to wireless. Here's this single fix again you acked.

--
Fix priority mistakes similar to '!x & y'

Signed-off-by Roel Kluin <[email protected]>

---

drivers/net/e1000e/82571.c +1 -1
index cf70522..14141a5 100644
@@ -283,7 +283,7 @@ static s32 e1000_get_invariants_82571(struct e1000_adapter *adapter)
 			adapter->flags &= ~FLAG_HAS_WOL;
 		/* quad ports only support WoL on port A */
 		if (adapter->flags & FLAG_IS_QUAD_PORT &&
-		    (!adapter->flags & FLAG_IS_QUAD_PORT_A))
+		    (!(adapter->flags & FLAG_IS_QUAD_PORT_A)))
 			adapter->flags &= ~FLAG_HAS_WOL;
 		break;
#3 lore raw
Quoted 11 lines · Roel Kluin
Quoted 6 lines · Auke

Ack this e1000e change here!

(PS since there was only 1 netdriver patch here and the rest is wireless, I would have suggested splitting this patch up in two and sending them to the wireless maintainer and netdevice maintainer separately. But I'm sure this will get picked up anyway.)

Ok, I've split the patch and sent the rest to wireless. Here's this single fix again you acked.

OK, bedankt Roel!

I'll forward this to Jeff for merging.

Cheers,

Auke

Quoted 18 lines

--
Fix priority mistakes similar to '!x & y'

Signed-off-by Roel Kluin <[email protected]>

---

drivers/net/e1000e/82571.c +1 -1
index cf70522..14141a5 100644
@@ -283,7 +283,7 @@ static s32 e1000_get_invariants_82571(struct e1000_adapter *adapter)
 			adapter->flags &= ~FLAG_HAS_WOL;
 		/* quad ports only support WoL on port A */
 		if (adapter->flags & FLAG_IS_QUAD_PORT &&
-		    (!adapter->flags & FLAG_IS_QUAD_PORT_A))
+		    (!(adapter->flags & FLAG_IS_QUAD_PORT_A)))
 			adapter->flags &= ~FLAG_HAS_WOL;
 		break;
#4 lore raw
In reply to Roel Kluin
Quoted 14 lines · On Fri, 26 Oct 2007 19:07:10 +0200 Roel Kluin <[email protected]> wrote:

A few patches with changes to net code. I have sent these to the lkml previously, but they were not yet merged. I am fairly new to kernel programming, so it is possible that I make some mistakes. I'll explain my rationale, please nack if incorrect, an additional bit of explanation is appreciated even more.

The condition '!x & y,' does make little sense: the '!' has a higher priority than '&'. It behaves therefore like '!x && y'. In the case bitanding flags, however, '!(x & y)' appears to be desired.

Warning: the change of '!x & y,' to '!(x & y)' may change behavior. if not desired, I propose changing this to '!x && y', to make it explicitly clear.

Quoted 8 lines

These '&' typo's can be spotted with:
a="A-Za-z0-9_"
git-grep "\![^$a()]*[$a]\+\([$a.]*\|->\)*\(\[[$a.]*\]\)\?[ \W]*&[^&]\+"

--
Fix priority mistakes similar to '!x & y'

Signed-off-by Roel Kluin <[email protected]>

I agree these look good, but maybe they should be broken into separate patches to make bisection of any induced regressions easier.

Stephen Hemminger <[email protected]>