staging: rtl8712: Refactor conditionals

PATCH In review Narcisa Ana Maria Vasile 3 participants 23 messages Mar 10, 2017 – Mar 21, 2017 Staging

lore

Share & tools

lore

Narcisa Ana Maria Vasile

author

2017-03-10 15:59 UTC #0 lore raw

cover staging: rtl8712: Refactor conditionals

This patchset reduces indentation level by refactoring conditionals. The patches were done using Cocinelle.

Narcisa Ana Maria Vasile (4):
staging: rtl8712: Refactor conditionals in wpa_set_encryption
staging: rtl8712: Refactor conditional in r8711_wx_get_freq
staging: rtl8712: Refactor conditionals in r8711_wx_get_rate
staging: rtl8712: Refactor conditional in r871x_get_ap_info

 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 142 +++++++++++++-------------
 1 file changed, 70 insertions(+), 72 deletions(-)

Signature · 1 line

1.9.1

Narcisa Ana Maria Vasile

author

2017-03-10 15:59 UTC #1 lore raw

PATCH 1/4 staging: rtl8712: Refactor conditionals in wpa_set_encryption

Reduce the indentation level in wpa_set_encryption. This was done using the following Cocinelle script:

@disable neg_if@
expression e,E;
statement S;
@@

*if (e)
S
else { return -E; }

@disable neg_if@
expression e,E;
statement S;
identifier l;
@@

*if (e)
S
else { rc = -E; goto l; }

Signed-off-by Narcisa Ana Maria Vasile <[email protected]>

---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 42 +++++++++++++--------------
 1 file changed, 20 insertions(+), 22 deletions(-)

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +20 ~~-22~~

index 0322795..aff3628 100644
@@ -378,14 +378,14 @@ static int wpa_set_encryption(struct net_device *dev, struct ieee_param *param,
 	if (param_len != (u32)((u8 *) param->u.crypt.key - (u8 *)param) +
 			 param->u.crypt.key_len)
 		return -EINVAL;
-	if (is_broadcast_ether_addr(param->sta_addr)) {
-		if (param->u.crypt.idx >= WEP_KEYS) {
-			/* for large key indices, set the default (0) */
-			param->u.crypt.idx = 0;
-		}
-	} else {
+	if (!is_broadcast_ether_addr(param->sta_addr))
 		return -EINVAL;
+
+	if (param->u.crypt.idx >= WEP_KEYS) {
+		/* for large key indices, set the default (0) */
+		param->u.crypt.idx = 0;
 	}
+
 	if (strcmp(param->u.crypt.alg, "WEP") == 0) {
 		netdev_info(dev, "r8712u: %s: crypt.alg = WEP\n", __func__);
 		padapter->securitypriv.ndisencryptstatus =

@@ -396,24 +396,22 @@ static int wpa_set_encryption(struct net_device *dev, struct ieee_param *param,
 		wep_key_len = param->u.crypt.key_len;
 		if (wep_key_idx >= WEP_KEYS)
 			wep_key_idx = 0;
-		if (wep_key_len > 0) {
-			wep_key_len = wep_key_len <= 5 ? 5 : 13;
-			pwep = kzalloc(sizeof(*pwep), GFP_ATOMIC);
-			if (!pwep)
-				return -ENOMEM;
-			pwep->KeyLength = wep_key_len;
-			pwep->Length = wep_key_len +
-				 FIELD_OFFSET(struct NDIS_802_11_WEP,
-				 KeyMaterial);
-			if (wep_key_len == 13) {
-				padapter->securitypriv.PrivacyAlgrthm =
-					 _WEP104_;
-				padapter->securitypriv.XGrpPrivacy =
-					 _WEP104_;
-			}
-		} else {
+		if (wep_key_len <= 0)
 			return -EINVAL;
+
+		wep_key_len = wep_key_len <= 5 ? 5 : 13;
+		pwep = kzalloc(sizeof(*pwep), GFP_ATOMIC);
+		if (!pwep)
+			return -ENOMEM;
+		pwep->KeyLength = wep_key_len;
+		pwep->Length = wep_key_len +
+			 FIELD_OFFSET(struct NDIS_802_11_WEP,
+			 KeyMaterial);
+		if (wep_key_len == 13) {
+			padapter->securitypriv.PrivacyAlgrthm = _WEP104_;
+			padapter->securitypriv.XGrpPrivacy = _WEP104_;
 		}
+
 		pwep->KeyIndex = wep_key_idx;
 		pwep->KeyIndex |= 0x80000000;
 		memcpy(pwep->KeyMaterial, param->u.crypt.key, pwep->KeyLength);

Signature · 1 line

1.9.1

Julia Lawall

2017-03-10 18:21 UTC #2 lore raw

Re: [Outreachy kernel] [PATCH 1/4] staging: rtl8712: Refactor conditionals in wpa_set_encryption

Quoted 73 lines · Narcisa Ana Maria Vasile

Reduce the indentation level in wpa_set_encryption. This was done using the following Cocinelle script:

@disable neg_if@
expression e,E;
statement S;
@@

*if (e)
S
else { return -E; }

@disable neg_if@
expression e,E;
statement S;
identifier l;
@@

*if (e)
S
else { rc = -E; goto l; }

Signed-off-by Narcisa Ana Maria Vasile <[email protected]>

---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 42 +++++++++++++--------------
 1 file changed, 20 insertions(+), 22 deletions(-)

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +7 ~~-22~~

index 0322795..aff3628 100644
@@ -378,14 +378,14 @@ static int wpa_set_encryption(struct net_device *dev, struct ieee_param *param,
 	if (param_len != (u32)((u8 *) param->u.crypt.key - (u8 *)param) +
 			 param->u.crypt.key_len)
 		return -EINVAL;
-	if (is_broadcast_ether_addr(param->sta_addr)) {
-		if (param->u.crypt.idx >= WEP_KEYS) {
-			/* for large key indices, set the default (0) */
-			param->u.crypt.idx = 0;
-		}
-	} else {
+	if (!is_broadcast_ether_addr(param->sta_addr))
 		return -EINVAL;
+
+	if (param->u.crypt.idx >= WEP_KEYS) {
+		/* for large key indices, set the default (0) */
+		param->u.crypt.idx = 0;
 	}
+
 	if (strcmp(param->u.crypt.alg, "WEP") == 0) {
 		netdev_info(dev, "r8712u: %s: crypt.alg = WEP\n", __func__);
 		padapter->securitypriv.ndisencryptstatus =

@@ -396,24 +396,22 @@ static int wpa_set_encryption(struct net_device *dev, struct ieee_param *param,
 		wep_key_len = param->u.crypt.key_len;
 		if (wep_key_idx >= WEP_KEYS)
 			wep_key_idx = 0;
-		if (wep_key_len > 0) {
-			wep_key_len = wep_key_len <= 5 ? 5 : 13;
-			pwep = kzalloc(sizeof(*pwep), GFP_ATOMIC);
-			if (!pwep)
-				return -ENOMEM;
-			pwep->KeyLength = wep_key_len;
-			pwep->Length = wep_key_len +
-				 FIELD_OFFSET(struct NDIS_802_11_WEP,
-				 KeyMaterial);
-			if (wep_key_len == 13) {
-				padapter->securitypriv.PrivacyAlgrthm =
-					 _WEP104_;
-				padapter->securitypriv.XGrpPrivacy =
-					 _WEP104_;
-			}
-		} else {
+		if (wep_key_len <= 0)

drivers/staging/rtl8712/rtl871x_ioctl_linux.c static int wpa_set_encryption(struct net_device *dev, struct ieee_param *param, · line 409

Quoted 10 lines

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +9 -0

 			return -EINVAL;
+
+		wep_key_len = wep_key_len <= 5 ? 5 : 13;
+		pwep = kzalloc(sizeof(*pwep), GFP_ATOMIC);
+		if (!pwep)
+			return -ENOMEM;
+		pwep->KeyLength = wep_key_len;
+		pwep->Length = wep_key_len +
+			 FIELD_OFFSET(struct NDIS_802_11_WEP,
+			 KeyMaterial);

The layout can be improved here. At least KeyMaterial can move up one line.

julia

Quoted 18 lines

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +4 -1
+		if (wep_key_len == 13) {
+			padapter->securitypriv.PrivacyAlgrthm = _WEP104_;
+			padapter->securitypriv.XGrpPrivacy = _WEP104_;
 		}
+
 		pwep->KeyIndex = wep_key_idx;
 		pwep->KeyIndex |= 0x80000000;
 		memcpy(pwep->KeyMaterial, param->u.crypt.key, pwep->KeyLength);
--
1.9.1
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/9f0c0614b6cc60771b82e96cd3973b4eca2c0acb.1489161001.git.narcisaanamaria12%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

Narcisa Ana Maria Vasile

author

2017-03-10 15:59 UTC #3 lore raw

PATCH 2/4 staging: rtl8712: Refactor conditional in r8711_wx_get_freq

In reply to Narcisa Ana Maria Vasile

Reduce one level of indentation in r8711_wx_get_freq. This was done using the following Coccinelle script:

@disable neg_if@
expression e,E;
statement S;
@@

*if (e)
S
else { return -E; }

@disable neg_if@
expression e,E;
statement S;
identifier l;
@@

*if (e)
S
else { rc = -E; goto l; }

Signed-off-by Narcisa Ana Maria Vasile <[email protected]>

---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +7 -7

index aff3628..add46ff 100644
@@ -698,14 +698,14 @@ static int r8711_wx_get_freq(struct net_device *dev,
 	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
 	struct wlan_bssid_ex *pcur_bss = &pmlmepriv->cur_network.network;
 
-	if (check_fwstate(pmlmepriv, _FW_LINKED)) {
-		wrqu->freq.m = ieee80211_wlan_frequencies[
-			       pcur_bss->Configuration.DSConfig - 1] * 100000;
-		wrqu->freq.e = 1;
-		wrqu->freq.i = pcur_bss->Configuration.DSConfig;
-	} else {
+	if (!check_fwstate(pmlmepriv, _FW_LINKED))
 		return -ENOLINK;
-	}
+
+	wrqu->freq.m = ieee80211_wlan_frequencies[
+		       pcur_bss->Configuration.DSConfig - 1] * 100000;
+	wrqu->freq.e = 1;
+	wrqu->freq.i = pcur_bss->Configuration.DSConfig;
+
 	return 0;
 }

Signature · 1 line

1.9.1

Julia Lawall

2017-03-10 18:19 UTC #4 lore raw

Re: [Outreachy kernel] [PATCH 2/4] staging: rtl8712: Refactor conditional in r8711_wx_get_freq

Quoted 32 lines · Narcisa Ana Maria Vasile

Reduce one level of indentation in r8711_wx_get_freq. This was done using the following Coccinelle script:
@disable neg_if@
expression e,E;
statement S;
@@
*if (e)
S
else { return -E; }
@disable neg_if@
expression e,E;
statement S;
identifier l;
@@
*if (e)
S
else { rc = -E; goto l; }
Signed-off-by Narcisa Ana Maria Vasile <[email protected]>
---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
drivers/staging/rtl8712/rtl871x_ioctl_linux.c +0 -0
index aff3628..add46ff 100644

drivers/staging/rtl8712/rtl871x_ioctl_linux.c static int r8711_wx_get_freq(struct net_device *dev, · line 701

Quoted 10 lines

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +1 -6

@@ -698,14 +698,14 @@ static int r8711_wx_get_freq(struct net_device *dev,
 	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
 	struct wlan_bssid_ex *pcur_bss = &pmlmepriv->cur_network.network;
 
-	if (check_fwstate(pmlmepriv, _FW_LINKED)) {
-		wrqu->freq.m = ieee80211_wlan_frequencies[
-			       pcur_bss->Configuration.DSConfig - 1] * 100000;
-		wrqu->freq.e = 1;
-		wrqu->freq.i = pcur_bss->Configuration.DSConfig;
-	} else {
+	if (!check_fwstate(pmlmepriv, _FW_LINKED))

Off topic, but check_fwstate could have return type bool - currently it's u8.

julia

Quoted 21 lines

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +6 -2
 		return -ENOLINK;
-	}
+
+	wrqu->freq.m = ieee80211_wlan_frequencies[
+		       pcur_bss->Configuration.DSConfig - 1] * 100000;
+	wrqu->freq.e = 1;
+	wrqu->freq.i = pcur_bss->Configuration.DSConfig;
+
 	return 0;
 }
 
--
1.9.1
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/be3d4e4736100b797ab54a73cfbc568d0d389535.1489161001.git.narcisaanamaria12%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

Narcisa Ana Maria Vasile

author

2017-03-10 16:00 UTC #5 lore raw

PATCH 3/4 staging: rtl8712: Refactor conditionals in r8711_wx_get_rate

In reply to Narcisa Ana Maria Vasile

Reduce indentation level in r8711_wx_get_rate. This was done using the following Coccinelle script:

@disable neg_if@
expression e,E;
statement S;
@@

*if (e)
S
else { return -E; }

@disable neg_if@
expression e,E;
statement S;
identifier l;
@@

*if (e)
S
else { rc = -E; goto l; }

Signed-off-by Narcisa Ana Maria Vasile <[email protected]>

---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 72 +++++++++++++--------------
 1 file changed, 36 insertions(+), 36 deletions(-)

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +36 ~~-36~~

index add46ff..7ba8cf6 100644
@@ -1409,44 +1409,44 @@ static int r8711_wx_get_rate(struct net_device *dev,
 	u16 mcs_rate = 0;
 
 	i = 0;
-	if (check_fwstate(pmlmepriv, _FW_LINKED | WIFI_ADHOC_MASTER_STATE)) {
-		p = r8712_get_ie(&pcur_bss->IEs[12],
-				 _HT_CAPABILITY_IE_, &ht_ielen,
-		    pcur_bss->IELength - 12);
-		if (p && ht_ielen > 0) {
-			ht_cap = true;
-			pht_capie = (struct ieee80211_ht_cap *)(p + 2);
-			memcpy(&mcs_rate, pht_capie->supp_mcs_set, 2);
-			bw_40MHz = (le16_to_cpu(pht_capie->cap_info) &
-				    IEEE80211_HT_CAP_SUP_WIDTH) ? 1 : 0;
-			short_GI = (le16_to_cpu(pht_capie->cap_info) &
-				    (IEEE80211_HT_CAP_SGI_20 |
-				    IEEE80211_HT_CAP_SGI_40)) ? 1 : 0;
-		}
-		while ((pcur_bss->rates[i] != 0) &&
-			(pcur_bss->rates[i] != 0xFF)) {
-			rate = pcur_bss->rates[i] & 0x7F;
-			if (rate > max_rate)
-				max_rate = rate;
-			wrqu->bitrate.fixed = 0;	/* no auto select */
-			wrqu->bitrate.value = rate * 500000;
-			i++;
-		}
-		if (ht_cap) {
-			if (mcs_rate & 0x8000 /* MCS15 */
-				&&
-				rf_type == RTL8712_RF_2T2R)
-				max_rate = (bw_40MHz) ? ((short_GI) ? 300 :
-					    270) : ((short_GI) ? 144 : 130);
-			else /* default MCS7 */
-				max_rate = (bw_40MHz) ? ((short_GI) ? 150 :
-					    135) : ((short_GI) ? 72 : 65);
-			max_rate *= 2; /* Mbps/2 */
-		}
-		wrqu->bitrate.value = max_rate * 500000;
-	} else {
+	if (!check_fwstate(pmlmepriv, _FW_LINKED | WIFI_ADHOC_MASTER_STATE))
 		return -ENOLINK;
+
+	p = r8712_get_ie(&pcur_bss->IEs[12],
+			 _HT_CAPABILITY_IE_, &ht_ielen,
+	    pcur_bss->IELength - 12);
+	if (p && ht_ielen > 0) {
+		ht_cap = true;
+		pht_capie = (struct ieee80211_ht_cap *)(p + 2);
+		memcpy(&mcs_rate, pht_capie->supp_mcs_set, 2);
+		bw_40MHz = (le16_to_cpu(pht_capie->cap_info) &
+			    IEEE80211_HT_CAP_SUP_WIDTH) ? 1 : 0;
+		short_GI = (le16_to_cpu(pht_capie->cap_info) &
+			    (IEEE80211_HT_CAP_SGI_20 |
+			    IEEE80211_HT_CAP_SGI_40)) ? 1 : 0;
 	}
+	while ((pcur_bss->rates[i] != 0) &&
+		(pcur_bss->rates[i] != 0xFF)) {
+		rate = pcur_bss->rates[i] & 0x7F;
+		if (rate > max_rate)
+			max_rate = rate;
+		wrqu->bitrate.fixed = 0;	/* no auto select */
+		wrqu->bitrate.value = rate * 500000;
+		i++;
+	}
+	if (ht_cap) {
+		if (mcs_rate & 0x8000 /* MCS15 */
+			&&
+			rf_type == RTL8712_RF_2T2R)
+			max_rate = (bw_40MHz) ? ((short_GI) ? 300 :
+				    270) : ((short_GI) ? 144 : 130);
+		else /* default MCS7 */
+			max_rate = (bw_40MHz) ? ((short_GI) ? 150 :
+				    135) : ((short_GI) ? 72 : 65);
+		max_rate *= 2; /* Mbps/2 */
+	}
+	wrqu->bitrate.value = max_rate * 500000;
+
 	return 0;
 }

Signature · 1 line

1.9.1

Julia Lawall

2017-03-10 18:16 UTC #6 lore raw

Re: [Outreachy kernel] [PATCH 3/4] staging: rtl8712: Refactor conditionals in r8711_wx_get_rate

Quoted 1 line · Narcisa Ana Maria Vasile

Reduce indentation level in r8711_wx_get_rate.

The description does not seem very specific. And the function name is not really necessary - one can see that directly from the patch.

It could be better to say something like: Invert if test to be able to clear out the error handling code immediately, and then continue with the rest of the function without excessive indentation.

Perhaps not ideal, but it could give someone a better idea of what to look for when looking at the patch.

These patches have the disadvantage that they seem to change a lot of lines of code, even though it is just an indentation issue. So it is not so easy to see what is going on.

Quoted 66 lines

This was done using the following Coccinelle script:

@disable neg_if@
expression e,E;
statement S;
@@

*if (e)
S
else { return -E; }

@disable neg_if@
expression e,E;
statement S;
identifier l;
@@

*if (e)
S
else { rc = -E; goto l; }

Signed-off-by Narcisa Ana Maria Vasile <[email protected]>

---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 72 +++++++++++++--------------
 1 file changed, 36 insertions(+), 36 deletions(-)

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +0 ~~-32~~

index add46ff..7ba8cf6 100644
@@ -1409,44 +1409,44 @@ static int r8711_wx_get_rate(struct net_device *dev,
 	u16 mcs_rate = 0;
 
 	i = 0;
-	if (check_fwstate(pmlmepriv, _FW_LINKED | WIFI_ADHOC_MASTER_STATE)) {
-		p = r8712_get_ie(&pcur_bss->IEs[12],
-				 _HT_CAPABILITY_IE_, &ht_ielen,
-		    pcur_bss->IELength - 12);
-		if (p && ht_ielen > 0) {
-			ht_cap = true;
-			pht_capie = (struct ieee80211_ht_cap *)(p + 2);
-			memcpy(&mcs_rate, pht_capie->supp_mcs_set, 2);
-			bw_40MHz = (le16_to_cpu(pht_capie->cap_info) &
-				    IEEE80211_HT_CAP_SUP_WIDTH) ? 1 : 0;
-			short_GI = (le16_to_cpu(pht_capie->cap_info) &
-				    (IEEE80211_HT_CAP_SGI_20 |
-				    IEEE80211_HT_CAP_SGI_40)) ? 1 : 0;
-		}
-		while ((pcur_bss->rates[i] != 0) &&
-			(pcur_bss->rates[i] != 0xFF)) {
-			rate = pcur_bss->rates[i] & 0x7F;
-			if (rate > max_rate)
-				max_rate = rate;
-			wrqu->bitrate.fixed = 0;	/* no auto select */
-			wrqu->bitrate.value = rate * 500000;
-			i++;
-		}
-		if (ht_cap) {
-			if (mcs_rate & 0x8000 /* MCS15 */
-				&&
-				rf_type == RTL8712_RF_2T2R)
-				max_rate = (bw_40MHz) ? ((short_GI) ? 300 :
-					    270) : ((short_GI) ? 144 : 130);
-			else /* default MCS7 */
-				max_rate = (bw_40MHz) ? ((short_GI) ? 150 :
-					    135) : ((short_GI) ? 72 : 65);

drivers/staging/rtl8712/rtl871x_ioctl_linux.c static int r8711_wx_get_rate(struct net_device *dev, · line 1417

Quoted 10 lines

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +5 -4

-			max_rate *= 2; /* Mbps/2 */
-		}
-		wrqu->bitrate.value = max_rate * 500000;
-	} else {
+	if (!check_fwstate(pmlmepriv, _FW_LINKED | WIFI_ADHOC_MASTER_STATE))
 		return -ENOLINK;
+
+	p = r8712_get_ie(&pcur_bss->IEs[12],
+			 _HT_CAPABILITY_IE_, &ht_ielen,
+	    pcur_bss->IELength - 12);

This call could be laid out better. Try to put as much on one line as possible without going over 80 characters.

Quoted 18 lines

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +17 -0

+	if (p && ht_ielen > 0) {
+		ht_cap = true;
+		pht_capie = (struct ieee80211_ht_cap *)(p + 2);
+		memcpy(&mcs_rate, pht_capie->supp_mcs_set, 2);
+		bw_40MHz = (le16_to_cpu(pht_capie->cap_info) &
+			    IEEE80211_HT_CAP_SUP_WIDTH) ? 1 : 0;
+		short_GI = (le16_to_cpu(pht_capie->cap_info) &
+			    (IEEE80211_HT_CAP_SGI_20 |
+			    IEEE80211_HT_CAP_SGI_40)) ? 1 : 0;
 	}
+	while ((pcur_bss->rates[i] != 0) &&
+		(pcur_bss->rates[i] != 0xFF)) {
+		rate = pcur_bss->rates[i] & 0x7F;
+		if (rate > max_rate)
+			max_rate = rate;
+		wrqu->bitrate.fixed = 0;	/* no auto select */
+		wrqu->bitrate.value = rate * 500000;
+		i++;

drivers/staging/rtl8712/rtl871x_ioctl_linux.c static int r8711_wx_get_rate(struct net_device *dev, · line 1445

Quoted 10 lines

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +10 -0

+	}
+	if (ht_cap) {
+		if (mcs_rate & 0x8000 /* MCS15 */
+			&&
+			rf_type == RTL8712_RF_2T2R)
+			max_rate = (bw_40MHz) ? ((short_GI) ? 300 :
+				    270) : ((short_GI) ? 144 : 130);
+		else /* default MCS7 */
+			max_rate = (bw_40MHz) ? ((short_GI) ? 150 :
+				    135) : ((short_GI) ? 72 : 65);

Consider moving 270 and 135 up a line, since you now have more horizontal space.

julia

Quoted 17 lines

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +4 -1
+		max_rate *= 2; /* Mbps/2 */
+	}
+	wrqu->bitrate.value = max_rate * 500000;
+
 	return 0;
 }
 
--
1.9.1
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/97c61822ad55d9e65bc292b6082d2247d9174916.1489161001.git.narcisaanamaria12%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

Narcisa Ana Maria Vasile

author

2017-03-10 16:00 UTC #7 lore raw

PATCH 4/4 staging: rtl8712: Refactor conditional in r871x_get_ap_info

In reply to Narcisa Ana Maria Vasile

Reduce indentation level in r871x_get_ap_info. This was done using the following Coccinelle script:

@disable neg_if@
expression e,E;
statement S;
@@

*if (e)
S
else { return -E; }

@disable neg_if@
expression e,E;
statement S;
identifier l;
@@

*if (e)
S
else { rc = -E; goto l; }

Signed-off-by Narcisa Ana Maria Vasile <[email protected]>

---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +6 -6

index 7ba8cf6..0b73f67 100644
@@ -1971,13 +1971,13 @@ static int r871x_get_ap_info(struct net_device *dev,
 			break;
 	}
 	pdata->flags = 0;
-	if (pdata->length >= 32) {
-		if (copy_from_user(data, pdata->pointer, 32))
-			return -EINVAL;
-		data[32] = 0;
-	} else {
+	if (pdata->length < 32)
 		return -EINVAL;
-	}
+
+	if (copy_from_user(data, pdata->pointer, 32))
+		return -EINVAL;
+	data[32] = 0;
+
 	spin_lock_irqsave(&(pmlmepriv->scanned_queue.lock), irqL);
 	phead = &queue->queue;
 	plist = phead->next;

Signature · 1 line

1.9.1

Julia Lawall

Acked-by on v1 4/4

2017-03-10 18:09 UTC #8 lore raw

Re: [Outreachy kernel] [PATCH 4/4] staging: rtl8712: Refactor conditional in r871x_get_ap_info

Quoted 2 lines · Narcisa Ana Maria Vasile

Reduce indentation level in r871x_get_ap_info. This was done using the following Coccinelle script:

Actually, this script doesn't do anything to the code. It just detects an opportunity. Otherwise,

Acked-by Julia Lawall <[email protected]>

Quoted 59 lines

@disable neg_if@
expression e,E;
statement S;
@@
*if (e)
S
else { return -E; }
@disable neg_if@
expression e,E;
statement S;
identifier l;
@@
*if (e)
S
else { rc = -E; goto l; }
Signed-off-by Narcisa Ana Maria Vasile <[email protected]>
---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/staging/rtl8712/rtl871x_ioctl_linux.c +6 -7
index 7ba8cf6..0b73f67 100644
@@ -1971,13 +1971,13 @@ static int r871x_get_ap_info(struct net_device *dev,
 			break;
 	}
 	pdata->flags = 0;
-	if (pdata->length >= 32) {
-		if (copy_from_user(data, pdata->pointer, 32))
-			return -EINVAL;
-		data[32] = 0;
-	} else {
+	if (pdata->length < 32)
 		return -EINVAL;
-	}
+
+	if (copy_from_user(data, pdata->pointer, 32))
+		return -EINVAL;
+	data[32] = 0;
+
 	spin_lock_irqsave(&(pmlmepriv->scanned_queue.lock), irqL);
 	phead = &queue->queue;
 	plist = phead->next;
--
1.9.1
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/51a2ef5e1c0eedf9700b43a83914983a6584ed24.1489161001.git.narcisaanamaria12%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

Narcisa Ana Maria Vasile

author

2017-03-10 20:37 UTC #9 lore raw

Re: [Outreachy kernel] [PATCH 4/4] staging: rtl8712: Refactor conditional in r871x_get_ap_info

Quoted 4 lines · Julia Lawall

On Fri, 10 Mar 2017, Narcisa Ana Maria Vasile wrote:

Quoted 8 lines

Quoted 2 lines
Reduce indentation level in r871x_get_ap_info. This was done using the following Coccinelle script:
Actually, this script doesn't do anything to the code. It just detects an opportunity. Otherwise,
Acked-by Julia Lawall <[email protected]>

Yes, what I meant is that the script helped me find this opportunity to improve a piece of code. I'll resend another version of this patchset based on the observations you made. Should I still include the Coccinelle script, but mention I used it to detect, not to change the code?

Thanks,
Narcisa

Quoted 59 lines

Quoted 59 lines
@disable neg_if@
expression e,E;
statement S;
@@
*if (e)
S
else { return -E; }
@disable neg_if@
expression e,E;
statement S;
identifier l;
@@
*if (e)
S
else { rc = -E; goto l; }
Signed-off-by Narcisa Ana Maria Vasile <[email protected]>
---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/staging/rtl8712/rtl871x_ioctl_linux.c +6 -7
index 7ba8cf6..0b73f67 100644
@@ -1971,13 +1971,13 @@ static int r871x_get_ap_info(struct net_device *dev,
 			break;
 	}
 	pdata->flags = 0;
-	if (pdata->length >= 32) {
-		if (copy_from_user(data, pdata->pointer, 32))
-			return -EINVAL;
-		data[32] = 0;
-	} else {
+	if (pdata->length < 32)
 		return -EINVAL;
-	}
+
+	if (copy_from_user(data, pdata->pointer, 32))
+		return -EINVAL;
+	data[32] = 0;
+
 	spin_lock_irqsave(&(pmlmepriv->scanned_queue.lock), irqL);
 	phead = &queue->queue;
 	plist = phead->next;
--
1.9.1
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/51a2ef5e1c0eedf9700b43a83914983a6584ed24.1489161001.git.narcisaanamaria12%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

Julia Lawall

2017-03-10 20:41 UTC #10 lore raw

Re: [Outreachy kernel] [PATCH 4/4] staging: rtl8712: Refactor conditional in r871x_get_ap_info

Quoted 11 lines · Narcisa Ana Maria Vasile

Quoted 10 lines · Julia Lawall
Quoted 2 lines · Narcisa Ana Maria Vasile
Reduce indentation level in r871x_get_ap_info. This was done using the following Coccinelle script:
Actually, this script doesn't do anything to the code. It just detects an opportunity. Otherwise,

Quoted 7 lines

Quoted 2 lines
Acked-by Julia Lawall <[email protected]>
Yes, what I meant is that the script helped me find this opportunity to improve a piece of code. I'll resend another version of this patchset based on the observations you made. Should I still include the Coccinelle script, but mention I used it to detect, not to change the code?

Yes, I think it would be a good idea to include it.

julia

Quoted 70 lines

Thanks,
Narcisa
Quoted 52 lines
Quoted 52 lines
@disable neg_if@
expression e,E;
statement S;
@@
*if (e)
S
else { return -E; }
@disable neg_if@
expression e,E;
statement S;
identifier l;
@@
*if (e)
S
else { rc = -E; goto l; }
Signed-off-by Narcisa Ana Maria Vasile <[email protected]>
---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/staging/rtl8712/rtl871x_ioctl_linux.c +6 -7
index 7ba8cf6..0b73f67 100644
@@ -1971,13 +1971,13 @@ static int r871x_get_ap_info(struct net_device *dev,
 			break;
 	}
 	pdata->flags = 0;
-	if (pdata->length >= 32) {
-		if (copy_from_user(data, pdata->pointer, 32))
-			return -EINVAL;
-		data[32] = 0;
-	} else {
+	if (pdata->length < 32)
 		return -EINVAL;
-	}
+
+	if (copy_from_user(data, pdata->pointer, 32))
+		return -EINVAL;
+	data[32] = 0;
+
 	spin_lock_irqsave(&(pmlmepriv->scanned_queue.lock), irqL);
 	phead = &queue->queue;
 	plist = phead->next;
--
1.9.1
Quoted 7 lines
Quoted 7 lines
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/51a2ef5e1c0eedf9700b43a83914983a6584ed24.1489161001.git.narcisaanamaria12%40gmail.com.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170310203659.GA4118%40nati-X550JK.
For more options, visit https://groups.google.com/d/optout.

Julia Lawall

2017-03-10 18:22 UTC #11 lore raw

In reply to Narcisa Ana Maria Vasile

Quoted 1 line · Narcisa Ana Maria Vasile

This patchset reduces indentation level by refactoring conditionals.

Refactor is a very vague word. It could be ok for the subject line, but the log message should give more detail.

julia

Quoted 21 lines

The patches were done using Cocinelle.
Narcisa Ana Maria Vasile (4):
staging: rtl8712: Refactor conditionals in wpa_set_encryption
staging: rtl8712: Refactor conditional in r8711_wx_get_freq
staging: rtl8712: Refactor conditionals in r8711_wx_get_rate
staging: rtl8712: Refactor conditional in r871x_get_ap_info
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 142 +++++++++++++-------------
 1 file changed, 70 insertions(+), 72 deletions(-)
--
1.9.1
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/cover.1489161001.git.narcisaanamaria12%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

Narcisa Ana Maria Vasile

author

2017-03-18 18:34 UTC #12 lore raw

v2 cover staging: rtl8712: Invert if statements to reduce indentation level

In reply to Narcisa Ana Maria Vasile

This patchset reduces indentation level by inverting if statements, in order to clear out the error handling code immediately, and then continue executing the function without the excessive indentation and additional else branches.

These pieces of code where indentation was improved were found using Coccinelle.

---
Changes in v2:
- Make the commit messages more clear
- Improve layout of the functions

Narcisa Ana Maria Vasile (4):
staging: rtl8712: Invert if statements to reduce indentation level
staging: rtl8712: Invert if statement reduce indentation level
staging: rtl8712: Invert if statements to reduce indentation level
staging: rtl8712: Invert if statement to reduce indentation level

 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 135 ++++++++++++--------------
 1 file changed, 63 insertions(+), 72 deletions(-)

Signature · 1 line

1.9.1

Narcisa Ana Maria Vasile

author

2017-03-18 18:34 UTC #13 lore raw

PATCH v2 1/4 staging: rtl8712: Invert if statements to reduce indentation level

Invert if statements to be able to return immediately in case of error, and to avoid additional else branch, and then continue with the rest of the function without excessive indentation.

This was found using the following Coccinelle script:

@disable neg_if@
expression e,E;
statement S;
@@

*if (e)
S
else { return -E; }

@disable neg_if@
expression e,E;
statement S;
identifier l;
@@

*if (e)
S
else { rc = -E; goto l; }

Signed-off-by Narcisa Ana Maria Vasile <[email protected]>

---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 39 ++++++++++++---------------
 1 file changed, 17 insertions(+), 22 deletions(-)

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +17 ~~-22~~

index 0322795..60c97f7 100644
@@ -378,13 +378,12 @@ static int wpa_set_encryption(struct net_device *dev, struct ieee_param *param,
 	if (param_len != (u32)((u8 *) param->u.crypt.key - (u8 *)param) +
 			 param->u.crypt.key_len)
 		return -EINVAL;
-	if (is_broadcast_ether_addr(param->sta_addr)) {
-		if (param->u.crypt.idx >= WEP_KEYS) {
-			/* for large key indices, set the default (0) */
-			param->u.crypt.idx = 0;
-		}
-	} else {
+	if (!is_broadcast_ether_addr(param->sta_addr))
 		return -EINVAL;
+
+	if (param->u.crypt.idx >= WEP_KEYS) {
+		/* for large key indices, set the default (0) */
+		param->u.crypt.idx = 0;
 	}
 	if (strcmp(param->u.crypt.alg, "WEP") == 0) {
 		netdev_info(dev, "r8712u: %s: crypt.alg = WEP\n", __func__);

@@ -396,23 +395,19 @@ static int wpa_set_encryption(struct net_device *dev, struct ieee_param *param,
 		wep_key_len = param->u.crypt.key_len;
 		if (wep_key_idx >= WEP_KEYS)
 			wep_key_idx = 0;
-		if (wep_key_len > 0) {
-			wep_key_len = wep_key_len <= 5 ? 5 : 13;
-			pwep = kzalloc(sizeof(*pwep), GFP_ATOMIC);
-			if (!pwep)
-				return -ENOMEM;
-			pwep->KeyLength = wep_key_len;
-			pwep->Length = wep_key_len +
-				 FIELD_OFFSET(struct NDIS_802_11_WEP,
-				 KeyMaterial);
-			if (wep_key_len == 13) {
-				padapter->securitypriv.PrivacyAlgrthm =
-					 _WEP104_;
-				padapter->securitypriv.XGrpPrivacy =
-					 _WEP104_;
-			}
-		} else {
+		if (wep_key_len <= 0)
 			return -EINVAL;
+
+		wep_key_len = wep_key_len <= 5 ? 5 : 13;
+		pwep = kzalloc(sizeof(*pwep), GFP_ATOMIC);
+		if (!pwep)
+			return -ENOMEM;
+		pwep->KeyLength = wep_key_len;
+		pwep->Length = wep_key_len +
+			FIELD_OFFSET(struct NDIS_802_11_WEP, KeyMaterial);
+		if (wep_key_len == 13) {
+			padapter->securitypriv.PrivacyAlgrthm = _WEP104_;
+			padapter->securitypriv.XGrpPrivacy = _WEP104_;
 		}
 		pwep->KeyIndex = wep_key_idx;
 		pwep->KeyIndex |= 0x80000000;

Signature · 1 line

1.9.1

Narcisa Ana Maria Vasile

author

2017-03-18 18:35 UTC #14 lore raw

PATCH v2 2/4 staging: rtl8712: Invert if statement reduce indentation level

In reply to Narcisa Ana Maria Vasile

Invert if statement to be able to return immediately in case of error, and to avoid additional else branch, and then continue with the rest of the function without excessive indentation.

This was found using the following Coccinelle script:

@disable neg_if@
expression e,E;
statement S;
@@

*if (e)

    S
    else { return -E; }

    @disable neg_if@
    expression e,E;
    statement S;
    identifier
    l;
    @@

    *if

(e)
S
else
{ rc = -E; goto l; }

Signed-off-by Narcisa Ana Maria Vasile <[email protected]>

---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +7 -7

index 60c97f7..38c1703 100644
@@ -695,14 +695,14 @@ static int r8711_wx_get_freq(struct net_device *dev,
 	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
 	struct wlan_bssid_ex *pcur_bss = &pmlmepriv->cur_network.network;
 
-	if (check_fwstate(pmlmepriv, _FW_LINKED)) {
-		wrqu->freq.m = ieee80211_wlan_frequencies[
-			       pcur_bss->Configuration.DSConfig - 1] * 100000;
-		wrqu->freq.e = 1;
-		wrqu->freq.i = pcur_bss->Configuration.DSConfig;
-	} else {
+	if (!check_fwstate(pmlmepriv, _FW_LINKED))
 		return -ENOLINK;
-	}
+
+	wrqu->freq.m = ieee80211_wlan_frequencies[
+		       pcur_bss->Configuration.DSConfig - 1] * 100000;
+	wrqu->freq.e = 1;
+	wrqu->freq.i = pcur_bss->Configuration.DSConfig;
+
 	return 0;
 }

Signature · 1 line

1.9.1

Narcisa Ana Maria Vasile

author

2017-03-18 18:35 UTC #15 lore raw

PATCH v2 3/4 staging: rtl8712: Invert if statements to reduce indentation level

In reply to Narcisa Ana Maria Vasile

Invert if statements to be able to return immediately in case of error, and to avoid additional else branch, and then continue with the rest of the function without excessive indentation.

Improve layout of function since there is more horizontal space now.

This was found using the following Coccinelle script:

@disable neg_if@
expression e,E;
statement S;
@@

*if (e)

    S
    else { return -E; }

    @disable neg_if@
    expression e,E;
    statement S;
    identifier l;
    @@

    *if

(e)
S
else
{ rc = -E; goto l; }

Signed-off-by Narcisa Ana Maria Vasile <[email protected]>

---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 69 +++++++++++++--------------
 1 file changed, 33 insertions(+), 36 deletions(-)

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +33 ~~-36~~

index 38c1703..3d6e818 100644
@@ -1406,44 +1406,41 @@ static int r8711_wx_get_rate(struct net_device *dev,
 	u16 mcs_rate = 0;
 
 	i = 0;
-	if (check_fwstate(pmlmepriv, _FW_LINKED | WIFI_ADHOC_MASTER_STATE)) {
-		p = r8712_get_ie(&pcur_bss->IEs[12],
-				 _HT_CAPABILITY_IE_, &ht_ielen,
-		    pcur_bss->IELength - 12);
-		if (p && ht_ielen > 0) {
-			ht_cap = true;
-			pht_capie = (struct ieee80211_ht_cap *)(p + 2);
-			memcpy(&mcs_rate, pht_capie->supp_mcs_set, 2);
-			bw_40MHz = (le16_to_cpu(pht_capie->cap_info) &
-				    IEEE80211_HT_CAP_SUP_WIDTH) ? 1 : 0;
-			short_GI = (le16_to_cpu(pht_capie->cap_info) &
-				    (IEEE80211_HT_CAP_SGI_20 |
-				    IEEE80211_HT_CAP_SGI_40)) ? 1 : 0;
-		}
-		while ((pcur_bss->rates[i] != 0) &&
-			(pcur_bss->rates[i] != 0xFF)) {
-			rate = pcur_bss->rates[i] & 0x7F;
-			if (rate > max_rate)
-				max_rate = rate;
-			wrqu->bitrate.fixed = 0;	/* no auto select */
-			wrqu->bitrate.value = rate * 500000;
-			i++;
-		}
-		if (ht_cap) {
-			if (mcs_rate & 0x8000 /* MCS15 */
-				&&
-				rf_type == RTL8712_RF_2T2R)
-				max_rate = (bw_40MHz) ? ((short_GI) ? 300 :
-					    270) : ((short_GI) ? 144 : 130);
-			else /* default MCS7 */
-				max_rate = (bw_40MHz) ? ((short_GI) ? 150 :
-					    135) : ((short_GI) ? 72 : 65);
-			max_rate *= 2; /* Mbps/2 */
-		}
-		wrqu->bitrate.value = max_rate * 500000;
-	} else {
+	if (!check_fwstate(pmlmepriv, _FW_LINKED | WIFI_ADHOC_MASTER_STATE))
 		return -ENOLINK;
+	p = r8712_get_ie(&pcur_bss->IEs[12], _HT_CAPABILITY_IE_, &ht_ielen,
+			 pcur_bss->IELength - 12);
+	if (p && ht_ielen > 0) {
+		ht_cap = true;
+		pht_capie = (struct ieee80211_ht_cap *)(p + 2);
+		memcpy(&mcs_rate, pht_capie->supp_mcs_set, 2);
+		bw_40MHz = (le16_to_cpu(pht_capie->cap_info) &
+			    IEEE80211_HT_CAP_SUP_WIDTH) ? 1 : 0;
+		short_GI = (le16_to_cpu(pht_capie->cap_info) &
+			    (IEEE80211_HT_CAP_SGI_20 |
+			    IEEE80211_HT_CAP_SGI_40)) ? 1 : 0;
+	}
+	while ((pcur_bss->rates[i] != 0) &&
+	       (pcur_bss->rates[i] != 0xFF)) {
+		rate = pcur_bss->rates[i] & 0x7F;
+		if (rate > max_rate)
+			max_rate = rate;
+		wrqu->bitrate.fixed = 0;	/* no auto select */
+		wrqu->bitrate.value = rate * 500000;
+		i++;
+	}
+	if (ht_cap) {
+		if (mcs_rate & 0x8000 /* MCS15 */
+		    &&
+		    rf_type == RTL8712_RF_2T2R)
+			max_rate = (bw_40MHz) ? ((short_GI) ? 300 : 270) :
+			((short_GI) ? 144 : 130);
+		else /* default MCS7 */
+			max_rate = (bw_40MHz) ? ((short_GI) ? 150 : 135) :
+			((short_GI) ? 72 : 65);
+		max_rate *= 2; /* Mbps/2 */
 	}
+	wrqu->bitrate.value = max_rate * 500000;
 	return 0;
 }

Signature · 1 line

1.9.1

Narcisa Ana Maria Vasile

author

2017-03-18 18:35 UTC #16 lore raw

PATCH v2 4/4 staging: rtl8712: Invert if statement to reduce indentation level

In reply to Narcisa Ana Maria Vasile

Invert if statement to be able to return immediately in case of error, and to avoid additional else branch, and then continue with the rest of the function without excessive indentation.

This was found using the following Coccinelle script:

@disable neg_if@
expression e,E;
statement S;
@@

*if (e)

    S
    else { return -E; }

    @disable neg_if@
    expression e,E;
    statement S;
    identifier l;
    @@

    *if

(e)
S
else
{ rc = -E; goto l; }

Signed-off-by Narcisa Ana Maria Vasile <[email protected]>

---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +5 -6

index 3d6e818..e30a5be 100644
@@ -1965,13 +1965,12 @@ static int r871x_get_ap_info(struct net_device *dev,
 			break;
 	}
 	pdata->flags = 0;
-	if (pdata->length >= 32) {
-		if (copy_from_user(data, pdata->pointer, 32))
-			return -EINVAL;
-		data[32] = 0;
-	} else {
+	if (pdata->length < 32)
 		return -EINVAL;
-	}
+	if (copy_from_user(data, pdata->pointer, 32))
+		return -EINVAL;
+	data[32] = 0;
+
 	spin_lock_irqsave(&(pmlmepriv->scanned_queue.lock), irqL);
 	phead = &queue->queue;
 	plist = phead->next;

Signature · 1 line

1.9.1

Greg KH

maintainer

2017-03-21 07:44 UTC #17 lore raw

Re: [PATCH v2 0/4] staging: rtl8712: Invert if statements to reduce indentation level

In reply to Narcisa Ana Maria Vasile

Quoted 13 lines · Narcisa Ana Maria Vasile

This patchset reduces indentation level by inverting if statements, in order to clear out the error handling code immediately, and then continue executing the function without the excessive indentation and additional else branches.
These pieces of code where indentation was improved were found using Coccinelle.
---
Changes in v2:
- Make the commit messages more clear
- Improve layout of the functions

Quoted 5 lines

Narcisa Ana Maria Vasile (4):
staging: rtl8712: Invert if statements to reduce indentation level
staging: rtl8712: Invert if statement reduce indentation level
staging: rtl8712: Invert if statements to reduce indentation level
staging: rtl8712: Invert if statement to reduce indentation level

These are 4 patches, that all do different things, yet the subject line is pretty much identical. Please fix it up to be more unique.

thanks,

greg k-h

Narcisa Ana Maria Vasile

author

2017-03-21 22:52 UTC #18 lore raw

v3 cover staging: rtl8712: Refactor conditionals to reduce indentation

In reply to Narcisa Ana Maria Vasile

These were found using Coccinelle.

---
Changes in v3:
- Improve commit messages to better represent the patches
Changes in v2:
- Make the commit messages more clear
- Improve layout of the functions

Narcisa Ana Maria Vasile (4):
staging: rtl8712: Invert if statements to reduce indentation level
staging: rtl8712: Invert the test on check_fwstate() to reduce
indentation
staging: rtl8712: Restructure code for clarity
staging: rtl8712: Invert comparison to reduce indentation

 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 135 ++++++++++++--------------
 1 file changed, 63 insertions(+), 72 deletions(-)

Signature · 1 line

1.9.1

Narcisa Ana Maria Vasile

author

2017-03-21 22:52 UTC #19 lore raw

PATCH v3 1/4 staging: rtl8712: Invert if statements to reduce indentation level

Invert if statements to be able to return immediately in case of error, and to avoid additional else branch, and then continue with the rest of the function without excessive indentation.

This was found using the following Coccinelle script:

@disable neg_if@
expression e,E;
statement S;
@@

*if (e)
S
else { return -E; }

@disable neg_if@
expression e,E;
statement S;
identifier l;
@@

*if (e)
S
else { rc = -E; goto l; }

Signed-off-by Narcisa Ana Maria Vasile <[email protected]>

---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 39 ++++++++++++---------------
 1 file changed, 17 insertions(+), 22 deletions(-)

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +17 ~~-22~~

index 0322795..60c97f7 100644
@@ -378,13 +378,12 @@ static int wpa_set_encryption(struct net_device *dev, struct ieee_param *param,
 	if (param_len != (u32)((u8 *) param->u.crypt.key - (u8 *)param) +
 			 param->u.crypt.key_len)
 		return -EINVAL;
-	if (is_broadcast_ether_addr(param->sta_addr)) {
-		if (param->u.crypt.idx >= WEP_KEYS) {
-			/* for large key indices, set the default (0) */
-			param->u.crypt.idx = 0;
-		}
-	} else {
+	if (!is_broadcast_ether_addr(param->sta_addr))
 		return -EINVAL;
+
+	if (param->u.crypt.idx >= WEP_KEYS) {
+		/* for large key indices, set the default (0) */
+		param->u.crypt.idx = 0;
 	}
 	if (strcmp(param->u.crypt.alg, "WEP") == 0) {
 		netdev_info(dev, "r8712u: %s: crypt.alg = WEP\n", __func__);

@@ -396,23 +395,19 @@ static int wpa_set_encryption(struct net_device *dev, struct ieee_param *param,
 		wep_key_len = param->u.crypt.key_len;
 		if (wep_key_idx >= WEP_KEYS)
 			wep_key_idx = 0;
-		if (wep_key_len > 0) {
-			wep_key_len = wep_key_len <= 5 ? 5 : 13;
-			pwep = kzalloc(sizeof(*pwep), GFP_ATOMIC);
-			if (!pwep)
-				return -ENOMEM;
-			pwep->KeyLength = wep_key_len;
-			pwep->Length = wep_key_len +
-				 FIELD_OFFSET(struct NDIS_802_11_WEP,
-				 KeyMaterial);
-			if (wep_key_len == 13) {
-				padapter->securitypriv.PrivacyAlgrthm =
-					 _WEP104_;
-				padapter->securitypriv.XGrpPrivacy =
-					 _WEP104_;
-			}
-		} else {
+		if (wep_key_len <= 0)
 			return -EINVAL;
+
+		wep_key_len = wep_key_len <= 5 ? 5 : 13;
+		pwep = kzalloc(sizeof(*pwep), GFP_ATOMIC);
+		if (!pwep)
+			return -ENOMEM;
+		pwep->KeyLength = wep_key_len;
+		pwep->Length = wep_key_len +
+			FIELD_OFFSET(struct NDIS_802_11_WEP, KeyMaterial);
+		if (wep_key_len == 13) {
+			padapter->securitypriv.PrivacyAlgrthm = _WEP104_;
+			padapter->securitypriv.XGrpPrivacy = _WEP104_;
 		}
 		pwep->KeyIndex = wep_key_idx;
 		pwep->KeyIndex |= 0x80000000;

Signature · 1 line

1.9.1

Narcisa Ana Maria Vasile

author

2017-03-21 22:53 UTC #20 lore raw

PATCH v3 2/4 staging: rtl8712: Invert the test on check_fwstate() to reduce indentation

In reply to Narcisa Ana Maria Vasile

Negate if condition to be able to return immediately in case of error, and then continue with the rest of the function
without extra indentation level.

This was found using the following Coccinelle script:

@disable neg_if@
expression e,E;
statement S;
@@

*if (e)

    S
    else { return -E; }

    @disable neg_if@
    expression e,E;
    statement S;
    identifier
    l;
    @@

    *if

(e)
S
else
{ rc = -E; goto l; }

Signed-off-by Narcisa Ana Maria Vasile <[email protected]>

---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +7 -7

index 60c97f7..38c1703 100644
@@ -695,14 +695,14 @@ static int r8711_wx_get_freq(struct net_device *dev,
 	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
 	struct wlan_bssid_ex *pcur_bss = &pmlmepriv->cur_network.network;
 
-	if (check_fwstate(pmlmepriv, _FW_LINKED)) {
-		wrqu->freq.m = ieee80211_wlan_frequencies[
-			       pcur_bss->Configuration.DSConfig - 1] * 100000;
-		wrqu->freq.e = 1;
-		wrqu->freq.i = pcur_bss->Configuration.DSConfig;
-	} else {
+	if (!check_fwstate(pmlmepriv, _FW_LINKED))
 		return -ENOLINK;
-	}
+
+	wrqu->freq.m = ieee80211_wlan_frequencies[
+		       pcur_bss->Configuration.DSConfig - 1] * 100000;
+	wrqu->freq.e = 1;
+	wrqu->freq.i = pcur_bss->Configuration.DSConfig;
+
 	return 0;
 }

Signature · 1 line

1.9.1

Narcisa Ana Maria Vasile

author

2017-03-21 22:53 UTC #21 lore raw

PATCH v3 3/4 staging: rtl8712: Restructure code for clarity

In reply to Narcisa Ana Maria Vasile

Invert if statements to be able to return immediately in case of error, and to avoid additional else branch.

Improve layout of function since there is more horizontal space now.

This was found using the following Coccinelle script:

@disable neg_if@
expression e,E;
statement S;
@@

*if (e)

    S
    else { return -E; }

    @disable neg_if@
    expression e,E;
    statement S;
    identifier l;
    @@

    *if

(e)
S
else
{ rc = -E; goto l; }

Signed-off-by Narcisa Ana Maria Vasile <[email protected]>

---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 69 +++++++++++++--------------
 1 file changed, 33 insertions(+), 36 deletions(-)

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +33 ~~-36~~

index 38c1703..3d6e818 100644
@@ -1406,44 +1406,41 @@ static int r8711_wx_get_rate(struct net_device *dev,
 	u16 mcs_rate = 0;
 
 	i = 0;
-	if (check_fwstate(pmlmepriv, _FW_LINKED | WIFI_ADHOC_MASTER_STATE)) {
-		p = r8712_get_ie(&pcur_bss->IEs[12],
-				 _HT_CAPABILITY_IE_, &ht_ielen,
-		    pcur_bss->IELength - 12);
-		if (p && ht_ielen > 0) {
-			ht_cap = true;
-			pht_capie = (struct ieee80211_ht_cap *)(p + 2);
-			memcpy(&mcs_rate, pht_capie->supp_mcs_set, 2);
-			bw_40MHz = (le16_to_cpu(pht_capie->cap_info) &
-				    IEEE80211_HT_CAP_SUP_WIDTH) ? 1 : 0;
-			short_GI = (le16_to_cpu(pht_capie->cap_info) &
-				    (IEEE80211_HT_CAP_SGI_20 |
-				    IEEE80211_HT_CAP_SGI_40)) ? 1 : 0;
-		}
-		while ((pcur_bss->rates[i] != 0) &&
-			(pcur_bss->rates[i] != 0xFF)) {
-			rate = pcur_bss->rates[i] & 0x7F;
-			if (rate > max_rate)
-				max_rate = rate;
-			wrqu->bitrate.fixed = 0;	/* no auto select */
-			wrqu->bitrate.value = rate * 500000;
-			i++;
-		}
-		if (ht_cap) {
-			if (mcs_rate & 0x8000 /* MCS15 */
-				&&
-				rf_type == RTL8712_RF_2T2R)
-				max_rate = (bw_40MHz) ? ((short_GI) ? 300 :
-					    270) : ((short_GI) ? 144 : 130);
-			else /* default MCS7 */
-				max_rate = (bw_40MHz) ? ((short_GI) ? 150 :
-					    135) : ((short_GI) ? 72 : 65);
-			max_rate *= 2; /* Mbps/2 */
-		}
-		wrqu->bitrate.value = max_rate * 500000;
-	} else {
+	if (!check_fwstate(pmlmepriv, _FW_LINKED | WIFI_ADHOC_MASTER_STATE))
 		return -ENOLINK;
+	p = r8712_get_ie(&pcur_bss->IEs[12], _HT_CAPABILITY_IE_, &ht_ielen,
+			 pcur_bss->IELength - 12);
+	if (p && ht_ielen > 0) {
+		ht_cap = true;
+		pht_capie = (struct ieee80211_ht_cap *)(p + 2);
+		memcpy(&mcs_rate, pht_capie->supp_mcs_set, 2);
+		bw_40MHz = (le16_to_cpu(pht_capie->cap_info) &
+			    IEEE80211_HT_CAP_SUP_WIDTH) ? 1 : 0;
+		short_GI = (le16_to_cpu(pht_capie->cap_info) &
+			    (IEEE80211_HT_CAP_SGI_20 |
+			    IEEE80211_HT_CAP_SGI_40)) ? 1 : 0;
+	}
+	while ((pcur_bss->rates[i] != 0) &&
+	       (pcur_bss->rates[i] != 0xFF)) {
+		rate = pcur_bss->rates[i] & 0x7F;
+		if (rate > max_rate)
+			max_rate = rate;
+		wrqu->bitrate.fixed = 0;	/* no auto select */
+		wrqu->bitrate.value = rate * 500000;
+		i++;
+	}
+	if (ht_cap) {
+		if (mcs_rate & 0x8000 /* MCS15 */
+		    &&
+		    rf_type == RTL8712_RF_2T2R)
+			max_rate = (bw_40MHz) ? ((short_GI) ? 300 : 270) :
+			((short_GI) ? 144 : 130);
+		else /* default MCS7 */
+			max_rate = (bw_40MHz) ? ((short_GI) ? 150 : 135) :
+			((short_GI) ? 72 : 65);
+		max_rate *= 2; /* Mbps/2 */
 	}
+	wrqu->bitrate.value = max_rate * 500000;
 	return 0;
 }

Signature · 1 line

1.9.1

Narcisa Ana Maria Vasile

author

2017-03-21 22:53 UTC #22 lore raw

PATCH v3 4/4 staging: rtl8712: Invert comparison to reduce indentation

In reply to Narcisa Ana Maria Vasile

Check the condition: "pdata->length < 32" first, to be able to return immediately in case of error and then continue with the rest of the function without one extra indentation level.

This was found using the following Coccinelle script:

@disable neg_if@
expression e,E;
statement S;
@@

*if (e)

    S
    else { return -E; }

    @disable neg_if@
    expression e,E;
    statement S;
    identifier l;
    @@

    *if

(e)
S
else
{ rc = -E; goto l; }

Signed-off-by Narcisa Ana Maria Vasile <[email protected]>

---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

drivers/staging/rtl8712/rtl871x_ioctl_linux.c +5 -6

index 3d6e818..e30a5be 100644
@@ -1965,13 +1965,12 @@ static int r871x_get_ap_info(struct net_device *dev,
 			break;
 	}
 	pdata->flags = 0;
-	if (pdata->length >= 32) {
-		if (copy_from_user(data, pdata->pointer, 32))
-			return -EINVAL;
-		data[32] = 0;
-	} else {
+	if (pdata->length < 32)
 		return -EINVAL;
-	}
+	if (copy_from_user(data, pdata->pointer, 32))
+		return -EINVAL;
+	data[32] = 0;
+
 	spin_lock_irqsave(&(pmlmepriv->scanned_queue.lock), irqL);
 	phead = &queue->queue;
 	plist = phead->next;

Signature · 1 line

1.9.1