staging: r8188eu: fix warnings reported by checkpatch

PATCH Superseded Jaehee Park 3 participants 17 messages Apr 13, 2022 – Apr 15, 2022 Staging

lore

Share & tools

lore

Jaehee Park

author

2022-04-13 20:11 UTC #0 lore raw

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

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

Signature · 1 line

2.25.1

Jaehee Park

author

2022-04-13 20:11 UTC #1 lore raw

PATCH 1/6 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

Jaehee Park

author

2022-04-13 20:11 UTC #6 lore raw

PATCH 2/6 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-13 20:11 UTC #7 lore raw

PATCH 3/6 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. Git will keep history so these comments can be removed from the code.

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-13 20:11 UTC #10 lore raw

PATCH 4/6 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-13 20:11 UTC #13 lore raw

PATCH 5/6 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-13 20:11 UTC #16 lore raw

PATCH 6/6 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

Pavel Skripkin

2022-04-13 20:24 UTC #2 lore raw

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

In reply to Jaehee Park

Hi Jaehee,

Quoted 14 lines · Jaehee Park

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 +0 -0
index 3e9882f89f76..d3f4d7cdfa08 100644

drivers/staging/r8188eu/core/rtw_mlme.c void _rtw_free_mlme_priv(struct mlme_priv *pmlmepriv) · line 117

Quoted 8 lines

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

If pmlmepriv is equal to NULL we would die in rtw_free_mlme_priv_ie_data(), so this check is just redundant

With regards,
Pavel Skripkin

Pavel Skripkin

2022-04-13 20:28 UTC #8 lore raw

Re: [PATCH 3/6] staging: r8188eu: remove 'added by' author comments

In reply to Jaehee Park

Hi Jaehee,

Quoted 4 lines · Jaehee Park

Author comments "Added by Albert" and "Added by Annie" are sprinkled through the file. Git will keep history so these comments can be removed from the code.

these people are not in the git log, since this driver was added in 2021. I am afraid they are not even in Larry's GH repo log.

Anyway these comments are not so useful, so patch is OK.

With regards,
Pavel Skripkin

Pavel Skripkin

2022-04-13 20:34 UTC #14 lore raw

Re: [PATCH 5/6] staging: r8188eu: replace spaces with tabs

In reply to Jaehee Park

Hi Jaehee,

Quoted 13 lines · 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 +0 -0
index b943fb190e4c..7a90fe826d1d 100644

drivers/staging/r8188eu/core/rtw_mlme.c void rtw_surveydone_event_callback(struct adapter *adapter, u8 *pbuf) · line 728

Quoted 8 lines

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

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

Not related to your patch, but looks like s_ret can't be 2.

rtw_select_and_join_from_scanned_queue

rtw_joinbss_cmd

rtw_enqueue_cmd
_rtw_enqueue_cmd <- always returns SUCCESS

Other functions from calltrace may return _FAIL, but _FAIL is not equal 2, right?

With regards,
Pavel Skripkin

Joe Perches

2022-04-14 01:17 UTC #11 lore raw

Re: [PATCH 4/6] staging: r8188eu: place constants on the right side of tests

In reply to Jaehee Park

Quoted 4 lines · 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.

[]

drivers/staging/r8188eu/core/rtw_mlme.c

Quoted 1 line

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

[]

drivers/staging/r8188eu/core/rtw_mlme.c void _rtw_roaming(struct adapter *padapter, struct wlan_network *tgt_network) · line 2001

Quoted 6 lines

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

@@ -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) {

Do you think this change is the same test?

What happens if rtw_to_roaming returns 0?

[]

drivers/staging/r8188eu/core/rtw_mlme.c void _rtw_roaming(struct adapter *padapter, struct wlan_network *tgt_network) · line 2013

Quoted 2 lines

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

-				if (0 < pmlmepriv->to_roaming) {
+				if (pmlmepriv->to_roaming > 0) {

here too

Quoted 3 lines

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

 					continue;
 				} else {
 					rtw_indicate_disconnect(padapter);

Jaehee Park

author

2022-04-14 19:41 UTC #3 lore raw

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

In reply to Pavel Skripkin

Quoted 12 lines · Pavel Skripkin

Hi Jaehee,
Quoted 9 lines · Jaehee Park
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 14 lines

drivers/staging/r8188eu/core/rtw_mlme.c void _rtw_free_mlme_priv(struct mlme_priv *pmlmepriv) · line 115
Quoted 10 lines
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);
-	}
If pmlmepriv is equal to NULL we would die in rtw_free_mlme_priv_ie_data(), so this check is just redundant

Hi Pavel, thank you for your comment! If I'm removing this if statement, should I include vfree(pmlmepriv->free_bss_buf) in rtw_free_mlme_priv_ie_data?

Quoted 5 lines

With regards,
Pavel Skripkin

Jaehee Park

author

2022-04-14 19:42 UTC #9 lore raw

Re: [PATCH 3/6] staging: r8188eu: remove 'added by' author comments

In reply to Pavel Skripkin

Quoted 8 lines · Pavel Skripkin

Hi Jaehee,
Quoted 4 lines · Jaehee Park
Author comments "Added by Albert" and "Added by Annie" are sprinkled through the file. Git will keep history so these comments can be removed from the code.

Quoted 4 lines

these people are not in the git log, since this driver was added in 2021. I am afraid they are not even in Larry's GH repo log.
Anyway these comments are not so useful, so patch is OK.

Thank you, I'll update the patch log message.

Quoted 3 lines

With regards,
Pavel Skripkin

Jaehee Park

author

2022-04-14 19:43 UTC #15 lore raw

Re: [PATCH 5/6] staging: r8188eu: replace spaces with tabs

In reply to Pavel Skripkin

Quoted 29 lines · Pavel Skripkin

Hi Jaehee,
Quoted 13 lines · 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 +0 -0
index b943fb190e4c..7a90fe826d1d 100644
drivers/staging/r8188eu/core/rtw_mlme.c void rtw_surveydone_event_callback(struct adapter *adapter, u8 *pbuf) · line 728
Quoted 8 lines
drivers/staging/r8188eu/core/rtw_mlme.c +1 -1
@@ -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);
Not related to your patch, but looks like s_ret can't be 2.
rtw_select_and_join_from_scanned_queue

Quoted 9 lines

rtw_joinbss_cmd
rtw_enqueue_cmd
_rtw_enqueue_cmd <- always returns SUCCESS
Other functions from calltrace may return _FAIL, but _FAIL is not equal 2, right?

Thank you for your comment. Since _rtw_enqueue_cmd cn't return 2, should we replace 2 with _FAIL? I can make this change in another patch.

Quoted 4 lines

With regards,
Pavel Skripkin

Jaehee Park

author

2022-04-14 19:46 UTC #12 lore raw

Re: [PATCH 4/6] staging: r8188eu: place constants on the right side of tests

In reply to Joe Perches

Quoted 12 lines · Joe Perches

Quoted 4 lines · 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.
[]
drivers/staging/r8188eu/core/rtw_mlme.c
Quoted 1 line
drivers/staging/r8188eu/core/rtw_mlme.c +0 -0
[]
Quoted 4 lines
drivers/staging/r8188eu/core/rtw_mlme.c +0 -0
@@ -1997,19 +1998,19 @@ void _rtw_roaming(struct adapter *padapter, struct wlan_network *tgt_network)
 	else
 		pnetwork = &pmlmepriv->cur_network;
 

Quoted 7 lines

drivers/staging/r8188eu/core/rtw_mlme.c void _rtw_roaming(struct adapter *padapter, struct wlan_network *tgt_network) · line 2001
Quoted 2 lines
drivers/staging/r8188eu/core/rtw_mlme.c +1 -1
-	if (0 < rtw_to_roaming(padapter)) {
+	if (rtw_to_roaming(padapter) > 0) {
Do you think this change is the same test?
What happens if rtw_to_roaming returns 0?

Hi Joe, Thank you for your comments.
If the roaming trying times is none it wouldn't need to associate with ssids. And we wouldn't need to go into this loop.
I think this change is the same-- am I missing something? Thanks,
Jaehee

Quoted 12 lines

[]
drivers/staging/r8188eu/core/rtw_mlme.c void _rtw_roaming(struct adapter *padapter, struct wlan_network *tgt_network) · line 2013
Quoted 2 lines
drivers/staging/r8188eu/core/rtw_mlme.c +1 -1
-				if (0 < pmlmepriv->to_roaming) {
+				if (pmlmepriv->to_roaming > 0) {
here too
Quoted 3 lines
drivers/staging/r8188eu/core/rtw_mlme.c +0 -0
 					continue;
 				} else {
 					rtw_indicate_disconnect(padapter);

Pavel Skripkin

2022-04-14 19:50 UTC #4 lore raw

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

In reply to Jaehee Park

Hi Jaehee,

Quoted 11 lines · Jaehee Park

Quoted 10 lines
drivers/staging/r8188eu/core/rtw_mlme.c void _rtw_free_mlme_priv(struct mlme_priv *pmlmepriv) · line 115
Quoted 6 lines
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);
-	}
If pmlmepriv is equal to NULL we would die in rtw_free_mlme_priv_ie_data(), so this check is just redundant

Quoted 4 lines

Hi Pavel, thank you for your comment! If I'm removing this if statement, should I include vfree(pmlmepriv->free_bss_buf) in rtw_free_mlme_priv_ie_data?

Simple grep shows, that this member is just unused

1 drivers/staging/r8188eu/core/rtw_mlme.c|64 col 13| pmlmepriv->free_bss_buf = pbuf; 2 drivers/staging/r8188eu/core/rtw_mlme.c|116 col 20| vfree(pmlmepriv->free_bss_buf); 3 drivers/staging/r8188eu/include/rtw_mlme.h|322 col 6| u8 *free_bss_buf;

so looks like you can just remove free_bss_buf and all related lines.

I hope I haven't overlooked something

With regards,
Pavel Skripkin

Jaehee Park

author

2022-04-15 02:51 UTC #5 lore raw

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

Quoted 29 lines · Pavel Skripkin

Hi Jaehee,
Quoted 11 lines · Jaehee Park
Quoted 10 lines
drivers/staging/r8188eu/core/rtw_mlme.c void _rtw_free_mlme_priv(struct mlme_priv *pmlmepriv) · line 115
Quoted 6 lines
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);
-	}
If pmlmepriv is equal to NULL we would die in rtw_free_mlme_priv_ie_data(), so this check is just redundant
Quoted 4 lines
Hi Pavel, thank you for your comment! If I'm removing this if statement, should I include vfree(pmlmepriv->free_bss_buf) in rtw_free_mlme_priv_ie_data?
Hm
Simple grep shows, that this member is just unused
1 drivers/staging/r8188eu/core/rtw_mlme.c|64 col 13| pmlmepriv->free_bss_buf = pbuf;
2 drivers/staging/r8188eu/core/rtw_mlme.c|116 col 20| vfree(pmlmepriv->free_bss_buf);
3 drivers/staging/r8188eu/include/rtw_mlme.h|322 col 6| u8 *free_bss_buf;

Quoted 4 lines

so looks like you can just remove free_bss_buf and all related lines.
I hope I haven't overlooked something

Hi Pavel, Thank you for your review! I have sent a second version of the patchset.
Thanks,
Jaehee

Quoted 4 lines

With regards,
Pavel Skripkin