[PATCH 0/4] staging: rtl8712: Refactor conditionals

[PATCH 0/4] staging: rtl8712: Refactor conditionals

[Narcisa Ana Maria Vasile]

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(-)

-- 
1.9.1

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

[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 <narcisaanamaria12@gmail.com>
---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 42 +++++++++++++--------------
 1 file changed, 20 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index 0322795..aff3628 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -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);
-- 
1.9.1

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

[Julia Lawall]

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

> 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 <narcisaanamaria12@gmail.com>
> ---
>  drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 42 +++++++++++++--------------
>  1 file changed, 20 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> index 0322795..aff3628 100644
> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> @@ -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);

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

julia

> +		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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> 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.
>

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

[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 <narcisaanamaria12@gmail.com>
---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index aff3628..add46ff 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -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;
 }
 
-- 
1.9.1

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

[Julia Lawall]

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

> 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 <narcisaanamaria12@gmail.com>
> ---
>  drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> index aff3628..add46ff 100644
> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> @@ -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


>  		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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> 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.
>

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

[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 <narcisaanamaria12@gmail.com>
---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 72 +++++++++++++--------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index add46ff..7ba8cf6 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -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;
 }
 
-- 
1.9.1

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

[Julia Lawall]

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

> 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.

> 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 <narcisaanamaria12@gmail.com>
> ---
>  drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 72 +++++++++++++--------------
>  1 file changed, 36 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> index add46ff..7ba8cf6 100644
> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> @@ -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);

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


> +	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);

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

julia

> +		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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> 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.
>

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

[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 <narcisaanamaria12@gmail.com>
---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index 7ba8cf6..0b73f67 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -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

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

[Julia Lawall]

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

> 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 <julia.lawall@lip6.fr>

>
> @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 <narcisaanamaria12@gmail.com>
> ---
>  drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> index 7ba8cf6..0b73f67 100644
> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> @@ -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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> 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.
>

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

[Narcisa Ana Maria Vasile]

On Fri, Mar 10, 2017 at 07:08:59PM +0100, Julia Lawall wrote:
> 
> 
> On Fri, 10 Mar 2017, Narcisa Ana Maria Vasile wrote:
> 
> > 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 <julia.lawall@lip6.fr>
> 
 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
> >
> > @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 <narcisaanamaria12@gmail.com>
> > ---
> >  drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> > index 7ba8cf6..0b73f67 100644
> > --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> > +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> > @@ -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 outreachy-kernel+unsubscribe@googlegroups.com.
> > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > 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.
> >

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

[Julia Lawall]

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

> On Fri, Mar 10, 2017 at 07:08:59PM +0100, Julia Lawall wrote:
> >
> >
> > On Fri, 10 Mar 2017, Narcisa Ana Maria Vasile wrote:
> >
> > > 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 <julia.lawall@lip6.fr>
> >
>  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

>
>  Thanks,
>  Narcisa
> > >
> > > @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 <narcisaanamaria12@gmail.com>
> > > ---
> > >  drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 12 ++++++------
> > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> > > index 7ba8cf6..0b73f67 100644
> > > --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> > > +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> > > @@ -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 outreachy-kernel+unsubscribe@googlegroups.com.
> > > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > > 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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> 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.
>

Re: [Outreachy kernel] [PATCH 0/4] staging: rtl8712: Refactor conditionals

[Julia Lawall]

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

> 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


> 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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> 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.
>

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

[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(-)

-- 
1.9.1

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

[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.

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 <narcisaanamaria12@gmail.com>
---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 39 ++++++++++++---------------
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index 0322795..60c97f7 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -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;
-- 
1.9.1

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

[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 <narcisaanamaria12@gmail.com>
---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index 60c97f7..38c1703 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -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;
 }
 
-- 
1.9.1

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

[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 <narcisaanamaria12@gmail.com>
---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 69 +++++++++++++--------------
 1 file changed, 33 insertions(+), 36 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index 38c1703..3d6e818 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -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;
 }
 
-- 
1.9.1

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

[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 <narcisaanamaria12@gmail.com>
---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index 3d6e818..e30a5be 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -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;
-- 
1.9.1

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

[Greg KH]

On Sat, Mar 18, 2017 at 08:33:58PM +0200, Narcisa Ana Maria Vasile wrote:
> 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

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

[PATCH v3 0/4] staging: rtl8712: Refactor conditionals to reduce indentation

[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 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(-)

-- 
1.9.1

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

[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.

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 <narcisaanamaria12@gmail.com>
---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 39 ++++++++++++---------------
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index 0322795..60c97f7 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -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;
-- 
1.9.1

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

[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 <narcisaanamaria12@gmail.com>
---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index 60c97f7..38c1703 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -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;
 }
 
-- 
1.9.1

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

[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 <narcisaanamaria12@gmail.com>
---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 69 +++++++++++++--------------
 1 file changed, 33 insertions(+), 36 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index 38c1703..3d6e818 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -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;
 }
 
-- 
1.9.1

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

[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 <narcisaanamaria12@gmail.com>
---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index 3d6e818..e30a5be 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -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;
-- 
1.9.1