staging: r8188eu: fix warnings reported by checkpatch

PATCH In review Jaehee Park 3 participants 12 messages Apr 21, 2022 – Apr 27, 2022 Staging

lore

Share & tools

lore

Jaehee Park

author

2022-04-21 19:20 UTC #0 lore raw

v4 cover staging: r8188eu: fix warnings reported by checkpatch

These patches address style issues found by checkpatch in the core/rtw_mlme.c file.

Jaehee Park (7):
staging: r8188eu: remove unnecessary braces in single statement block
staging: r8188eu: remove spaces before tabs
staging: r8188eu: remove 'added by' author comments
staging: r8188eu: place constants on the right side of tests
staging: r8188eu: replace spaces with tabs
staging: r8188eu: correct typo in comments
staging: r8188eu: remove unused else condition

 drivers/staging/r8188eu/core/rtw_mlme.c | 45 +++++++++++--------------
 1 file changed, 19 insertions(+), 26 deletions(-)

Signature · 1 line

2.25.1

Jaehee Park

author

2022-04-21 19:20 UTC #1 lore raw

PATCH v4 1/7 staging: r8188eu: remove unnecessary braces in single statement block

Remove braces for single statement block to minimize the number of empty lines, without loss of readability. Issue found with checkpatch.
WARNING: braces {} are not necessary for single statement blocks

Signed-off-by Jaehee Park <[email protected]>

---
 drivers/staging/r8188eu/core/rtw_mlme.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

drivers/staging/r8188eu/core/rtw_mlme.c +1 -2

index 3e9882f89f76..d3f4d7cdfa08 100644
@@ -112,9 +112,8 @@ void _rtw_free_mlme_priv(struct mlme_priv *pmlmepriv)
 
 	rtw_free_mlme_priv_ie_data(pmlmepriv);
 
-	if (pmlmepriv) {
+	if (pmlmepriv)
 		vfree(pmlmepriv->free_bss_buf);
-	}
 
 }

Signature · 1 line

2.25.1

Fabio M. De Francesco

2022-04-21 19:52 UTC #2 lore raw

Re: [PATCH v4 1/7] staging: r8188eu: remove unnecessary braces in single statement block

drivers/staging/

Quoted 10 lines · On giovedì 21 aprile 2022 21:20:26 CEST Jaehee Park wrote:

Remove braces for single statement block to minimize the number of empty lines, without loss of readability. Issue found with checkpatch.
WARNING: braces {} are not necessary for single statement blocks
Signed-off-by Jaehee Park <[email protected]>
---
 drivers/staging/r8188eu/core/rtw_mlme.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/staging/ renamed from drivers/staging/r8188eu/core/rtw_mlme.c +0 -0

r8188eu/core/rtw_mlme.c

Quoted 17 lines

index 3e9882f89f76..d3f4d7cdfa08 100644

drivers/staging/r8188eu/core/rtw_mlme.c +1 -2

@@ -112,9 +112,8 @@ void _rtw_free_mlme_priv(struct mlme_priv *pmlmepriv)
 
 	rtw_free_mlme_priv_ie_data(pmlmepriv);
 
-	if (pmlmepriv) {
+	if (pmlmepriv)
 		vfree(pmlmepriv->free_bss_buf);
-	}
 
 }

Signature · 1 line

2.25.1

Hi Jaehee,

If you just want to remove the braces, your patch is good.

But... Did you ask yourself why this code needs to test if "pmlmepriv" is valid before calling vfree() on it? Actually, the code doesn't need this test, it just needs to call "vfree(pmlmepriv)" unconditionally.

OK, I know that this is not directly related to your patch or any other that is explicitly made just for removing unnecessary braces...

vfree() behaves as a no-op function on NULL pointers, therefore there is no need to test whether or not "pmlmepriv" is valid.

Thanks,

Fabio M. De Francesco

2022-04-21 23:20 UTC #3 lore raw

Re: [PATCH v4 1/7] staging: r8188eu: remove unnecessary braces in single statement block

Quoted 10 lines · On giovedì 21 aprile 2022 21:52:14 CEST Fabio M. De Francesco wrote:

Quoted 9 lines · On giovedì 21 aprile 2022 21:20:26 CEST Jaehee Park wrote:
Remove braces for single statement block to minimize the number of empty lines, without loss of readability. Issue found with checkpatch.
WARNING: braces {} are not necessary for single statement blocks
Signed-off-by Jaehee Park <[email protected]>
---
 drivers/staging/r8188eu/core/rtw_mlme.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Quoted 6 lines

drivers/staging/
Quoted 1 line
drivers/staging/ renamed from drivers/staging/r8188eu/core/rtw_mlme.c +0 -0
r8188eu/core/rtw_mlme.c
drivers/staging/r8188eu/core/rtw_mlme.c void _rtw_free_mlme_priv(struct mlme_priv
Quoted 4 lines
index 3e9882f89f76..d3f4d7cdfa08 100644
drivers/staging/r8188eu/core/rtw_mlme.c +0 -0
@@ -112,9 +112,8 @@ void _rtw_free_mlme_priv(struct mlme_priv 

*pmlmepriv)

Quoted 16 lines

Quoted 13 lines
drivers/staging/r8188eu/core/rtw_mlme.c +1 -2
 	rtw_free_mlme_priv_ie_data(pmlmepriv);
 
-	if (pmlmepriv) {
+	if (pmlmepriv)
 		vfree(pmlmepriv->free_bss_buf);
-	}
 
 }
 
Signature · 1 line
2.25.1
Hi Jaehee,

Quoted 5 lines

If you just want to remove the braces, your patch is good.
But... Did you ask yourself why this code needs to test if "pmlmepriv" is valid before calling vfree() on it? Actually, the code doesn't need this test, it just needs to call "vfree(pmlmepriv)" unconditionally.

When copy-pasting I overlooked a couple of things... However the argument is the same: the code doesn't need that 'if' statement before vfree().

I wanted to write "[] it just needs to call "vfree(pmlmepriv-

Quoted 1 line

free_bss_buf)" unconditionally".

I apologize for not reading twice before sending my email too soon after writing and moving sentences around.

Quoted 2 lines

OK, I know that this is not directly related to your patch or any other that is explicitly made just for removing unnecessary braces...

"pmlmepriv" is valid because we know it from the previous call to rtw_free_mlme_priv_ie_data(). Therefore the code can dereference "pmlmepriv".

Furthermore, even if "free_bss_buf" were NULL it wouldn't matter because "vfree(pmlmepriv->free_bss_buf)" would be a no-op.

Sorry if this inaccurate wording might have confused you about what I was trying to say.

Thanks,

Fabio

Jaehee Park

author

2022-04-21 19:20 UTC #4 lore raw

PATCH v4 2/7 staging: r8188eu: remove spaces before tabs

In reply to Jaehee Park

Delete spaces before tabs in the comments. Issue found with checkpatch.
WARNING: please, no space before tabs

Signed-off-by Jaehee Park <[email protected]>

---
 drivers/staging/r8188eu/core/rtw_mlme.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

drivers/staging/r8188eu/core/rtw_mlme.c +7 -7

index d3f4d7cdfa08..2cfd8e8d74a4 100644
@@ -573,8 +573,8 @@ static void rtw_add_network(struct adapter *adapter,
 
 /* select the desired network based on the capability of the (i)bss. */
 /*  check items:	(1) security */
-/* 			(2) network_type */
-/* 			(3) WMM */
+/*			(2) network_type */
+/*			(3) WMM */
 /*			(4) HT */
 /*			(5) others */
 static bool rtw_is_desired_network(struct adapter *adapter, struct wlan_network *pnetwork)

@@ -909,9 +909,9 @@ static struct sta_info *rtw_joinbss_update_stainfo(struct adapter *padapter, str
 			memset((u8 *)&psta->dot11txpn, 0, sizeof(union pn48));
 			memset((u8 *)&psta->dot11rxpn, 0, sizeof(union pn48));
 		}
-		/* 	Commented by Albert 2012/07/21 */
-		/* 	When doing the WPS, the wps_ie_len won't equal to 0 */
-		/* 	And the Wi-Fi driver shouldn't allow the data packet to be tramsmitted. */
+		/*	Commented by Albert 2012/07/21 */
+		/*	When doing the WPS, the wps_ie_len won't equal to 0 */
+		/*	And the Wi-Fi driver shouldn't allow the data packet to be tramsmitted. */
 		if (padapter->securitypriv.wps_ie_len != 0) {
 			psta->ieee8021x_blocked = true;
 			padapter->securitypriv.wps_ie_len = 0;

@@ -1634,8 +1634,8 @@ int rtw_restruct_wmm_ie(struct adapter *adapter, u8 *in_ie, u8 *out_ie, uint in_
 /*  */
 /*  Search by BSSID, */
 /*  Return Value: */
-/* 		-1		:if there is no pre-auth key in the  table */
-/* 		>= 0		:if there is pre-auth key, and   return the entry id */
+/*		-1		:if there is no pre-auth key in the  table */
+/*		>= 0		:if there is pre-auth key, and   return the entry id */
 /*  */
 /*  */

Signature · 1 line

2.25.1

Jaehee Park

author

2022-04-21 19:20 UTC #5 lore raw

PATCH v4 3/7 staging: r8188eu: remove 'added by' author comments

In reply to Jaehee Park

Author comments "Added by Albert" and "Added by Annie" are sprinkled through the file. These comments are not useful and can be removed.

Signed-off-by Jaehee Park <[email protected]>

---
 drivers/staging/r8188eu/core/rtw_mlme.c | 4 ----
 1 file changed, 4 deletions(-)

drivers/staging/r8188eu/core/rtw_mlme.c +0 -4

index 2cfd8e8d74a4..5adef9b9108d 100644
@@ -909,7 +909,6 @@ static struct sta_info *rtw_joinbss_update_stainfo(struct adapter *padapter, str
 			memset((u8 *)&psta->dot11txpn, 0, sizeof(union pn48));
 			memset((u8 *)&psta->dot11rxpn, 0, sizeof(union pn48));
 		}
-		/*	Commented by Albert 2012/07/21 */
 		/*	When doing the WPS, the wps_ie_len won't equal to 0 */
 		/*	And the Wi-Fi driver shouldn't allow the data packet to be tramsmitted. */
 		if (padapter->securitypriv.wps_ie_len != 0) {

@@ -1628,9 +1627,6 @@ int rtw_restruct_wmm_ie(struct adapter *adapter, u8 *in_ie, u8 *out_ie, uint in_
 	return ielength;
 }
 
-/*  */
-/*  Ported from 8185: IsInPreAuthKeyList(). (Renamed from SecIsInPreAuthKeyList(), 2006-10-13.) */
-/*  Added by Annie, 2006-05-07. */
 /*  */
 /*  Search by BSSID, */
 /*  Return Value: */

Signature · 1 line

2.25.1

Jaehee Park

author

2022-04-21 19:20 UTC #6 lore raw

PATCH v4 4/7 staging: r8188eu: place constants on the right side of tests

In reply to Jaehee Park

To comply with the linux coding style, place constants on the right side of the test in comparisons. Issue found with checkpatch.
WARNING: Comparisons should place the constant on the right side of the test.

Signed-off-by Jaehee Park <[email protected]>

---
 drivers/staging/r8188eu/core/rtw_mlme.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

drivers/staging/r8188eu/core/rtw_mlme.c +7 -6

index 5adef9b9108d..b943fb190e4c 100644
@@ -721,7 +721,7 @@ void rtw_surveydone_event_callback(struct adapter	*adapter, u8 *pbuf)
 			set_fwstate(pmlmepriv, _FW_UNDER_LINKING);
 			pmlmepriv->to_join = false;
 			s_ret = rtw_select_and_join_from_scanned_queue(pmlmepriv);
-			if (_SUCCESS == s_ret) {
+			if (s_ret == _SUCCESS) {
 			     _set_timer(&pmlmepriv->assoc_timer, MAX_JOIN_TIMEOUT);
 			} else if (s_ret == 2) { /* there is no need to wait for join */
 				_clr_fwstate_(pmlmepriv, _FW_UNDER_LINKING);

@@ -729,7 +729,8 @@ void rtw_surveydone_event_callback(struct adapter	*adapter, u8 *pbuf)
 			} else {
 				if (rtw_to_roaming(adapter) != 0) {
 					if (--pmlmepriv->to_roaming == 0 ||
-					    _SUCCESS != rtw_sitesurvey_cmd(adapter, &pmlmepriv->assoc_ssid, 1, NULL, 0)) {
+					    rtw_sitesurvey_cmd(adapter, &pmlmepriv->assoc_ssid,
+							       1, NULL, 0) != _SUCCESS) {
 						rtw_set_roaming(adapter, 0);
 						rtw_free_assoc_resources(adapter, 1);
 						rtw_indicate_disconnect(adapter);

@@ -1970,7 +1971,7 @@ void rtw_issue_addbareq_cmd(struct adapter *padapter, struct xmit_frame *pxmitfr
 		issued = (phtpriv->agg_enable_bitmap >> priority) & 0x1;
 		issued |= (phtpriv->candidate_tid_bitmap >> priority) & 0x1;
 
-		if (0 == issued) {
+		if (issued == 0) {
 			psta->htpriv.candidate_tid_bitmap |= BIT((u8)priority);
 			rtw_addbareq_cmd(padapter, (u8)priority, pattrib->ra);
 		}

@@ -1997,19 +1998,19 @@ void _rtw_roaming(struct adapter *padapter, struct wlan_network *tgt_network)
 	else
 		pnetwork = &pmlmepriv->cur_network;
 
-	if (0 < rtw_to_roaming(padapter)) {
+	if (rtw_to_roaming(padapter) > 0) {
 		memcpy(&pmlmepriv->assoc_ssid, &pnetwork->network.Ssid, sizeof(struct ndis_802_11_ssid));
 
 		pmlmepriv->assoc_by_bssid = false;
 
 		while (1) {
 			do_join_r = rtw_do_join(padapter);
-			if (_SUCCESS == do_join_r) {
+			if (do_join_r == _SUCCESS) {
 				break;
 			} else {
 				pmlmepriv->to_roaming--;
 
-				if (0 < pmlmepriv->to_roaming) {
+				if (pmlmepriv->to_roaming > 0) {
 					continue;
 				} else {
 					rtw_indicate_disconnect(padapter);

Signature · 1 line

2.25.1

Jaehee Park

author

2022-04-21 19:20 UTC #7 lore raw

PATCH v4 5/7 staging: r8188eu: replace spaces with tabs

In reply to Jaehee Park

Use tabs instead of spaces. Issue found with checkpatch.
WARNING: suspect code indent for conditional statements

Signed-off-by Jaehee Park <[email protected]>

---
 drivers/staging/r8188eu/core/rtw_mlme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/staging/r8188eu/core/rtw_mlme.c +1 -1

index b943fb190e4c..7a90fe826d1d 100644
@@ -722,7 +722,7 @@ void rtw_surveydone_event_callback(struct adapter	*adapter, u8 *pbuf)
 			pmlmepriv->to_join = false;
 			s_ret = rtw_select_and_join_from_scanned_queue(pmlmepriv);
 			if (s_ret == _SUCCESS) {
-			     _set_timer(&pmlmepriv->assoc_timer, MAX_JOIN_TIMEOUT);
+				_set_timer(&pmlmepriv->assoc_timer, MAX_JOIN_TIMEOUT);
 			} else if (s_ret == 2) { /* there is no need to wait for join */
 				_clr_fwstate_(pmlmepriv, _FW_UNDER_LINKING);
 				rtw_indicate_connect(adapter);

Signature · 1 line

2.25.1

Jaehee Park

author

2022-04-21 19:20 UTC #8 lore raw

PATCH v4 6/7 staging: r8188eu: correct typo in comments

In reply to Jaehee Park

Correct misspellings in the comments. Issue found with checkpatch.

Signed-off-by Jaehee Park <[email protected]>

---
 drivers/staging/r8188eu/core/rtw_mlme.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

drivers/staging/r8188eu/core/rtw_mlme.c +5 -5

index 7a90fe826d1d..d422ce87ba7c 100644
@@ -192,7 +192,7 @@ void _rtw_free_network_nolock(struct	mlme_priv *pmlmepriv, struct wlan_network *
 /*
 	return the wlan_network with the matching addr
 
-	Shall be calle under atomic context... to avoid possible racing condition...
+	Shall be called under atomic context... to avoid possible racing condition...
 */
 struct wlan_network *_rtw_find_network(struct __queue *scanned_queue, u8 *addr)
 {

@@ -328,7 +328,7 @@ void rtw_free_network_queue(struct adapter *dev, u8 isfreeall)
 /*
 	return the wlan_network with the matching addr
 
-	Shall be calle under atomic context... to avoid possible racing condition...
+	Shall be called under atomic context... to avoid possible racing condition...
 */
 struct	wlan_network *rtw_find_network(struct __queue *scanned_queue, u8 *addr)
 {

@@ -911,7 +911,7 @@ static struct sta_info *rtw_joinbss_update_stainfo(struct adapter *padapter, str
 			memset((u8 *)&psta->dot11rxpn, 0, sizeof(union pn48));
 		}
 		/*	When doing the WPS, the wps_ie_len won't equal to 0 */
-		/*	And the Wi-Fi driver shouldn't allow the data packet to be tramsmitted. */
+		/*	And the Wi-Fi driver shouldn't allow the data packet to be transmitted. */
 		if (padapter->securitypriv.wps_ie_len != 0) {
 			psta->ieee8021x_blocked = true;
 			padapter->securitypriv.wps_ie_len = 0;

@@ -1305,7 +1305,7 @@ void rtw_stadel_event_callback(struct adapter *adapter, u8 *pbuf)
 }
 
 /*
-* _rtw_join_timeout_handler - Timeout/faliure handler for CMD JoinBss
+* _rtw_join_timeout_handler - Timeout/failure handler for CMD JoinBss
 * @adapter: pointer to struct adapter structure
 */
 void _rtw_join_timeout_handler (struct adapter *adapter)

@@ -1340,7 +1340,7 @@ void _rtw_join_timeout_handler (struct adapter *adapter)
 }
 
 /*
-* rtw_scan_timeout_handler - Timeout/Faliure handler for CMD SiteSurvey
+* rtw_scan_timeout_handler - Timeout/Failure handler for CMD SiteSurvey
 * @adapter: pointer to struct adapter structure
 */
 void rtw_scan_timeout_handler (struct adapter *adapter)

Signature · 1 line

2.25.1

Jaehee Park

author

2022-04-21 19:20 UTC #9 lore raw

PATCH v4 7/7 staging: r8188eu: remove unused else condition

In reply to Jaehee Park

s_ret cannot return '2' so this else condition is unused. Remove this unnecessary else statement.

Signed-off-by Jaehee Park <[email protected]>

---
 drivers/staging/r8188eu/core/rtw_mlme.c | 3 ---
 1 file changed, 3 deletions(-)

drivers/staging/r8188eu/core/rtw_mlme.c +0 -3

index d422ce87ba7c..29b8b101180b 100644
@@ -723,9 +723,6 @@ void rtw_surveydone_event_callback(struct adapter	*adapter, u8 *pbuf)
 			s_ret = rtw_select_and_join_from_scanned_queue(pmlmepriv);
 			if (s_ret == _SUCCESS) {
 				_set_timer(&pmlmepriv->assoc_timer, MAX_JOIN_TIMEOUT);
-			} else if (s_ret == 2) { /* there is no need to wait for join */
-				_clr_fwstate_(pmlmepriv, _FW_UNDER_LINKING);
-				rtw_indicate_connect(adapter);
 			} else {
 				if (rtw_to_roaming(adapter) != 0) {
 					if (--pmlmepriv->to_roaming == 0 ||

Signature · 1 line

2.25.1

Greg KH

maintainer

2022-04-22 14:45 UTC #10 lore raw

Re: [PATCH v4 7/7] staging: r8188eu: remove unused else condition

Quoted 2 lines · Jaehee Park

s_ret cannot return '2' so this else condition is unused. Remove this unnecessary else statement.

s_ret is not a function, it is a value. rtw_select_and_join_from_scanned_queue() is the function here.

Can you fix up the wording in this changelog text and resubmit?

thanks,

greg k-h

Jaehee

author

2022-04-27 15:37 UTC #11 lore raw

Re: [PATCH v4 7/7] staging: r8188eu: remove unused else condition

Quoted 5 lines · Greg KH

Quoted 2 lines · Jaehee Park
s_ret cannot return '2' so this else condition is unused. Remove this unnecessary else statement.

Quoted 8 lines

s_ret is not a function, it is a value. rtw_select_and_join_from_scanned_queue() is the function here.
Can you fix up the wording in this changelog text and resubmit?
thanks,
greg k-h

Sorry about the delay! I've sent the revised patch just now. Thanks,
Jaehee