linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] staging: r8188eu: fix warnings reported by checkpatch
@ 2022-04-13 20:11 Jaehee Park
  2022-04-13 20:11 ` [PATCH 1/6] staging: r8188eu: remove unnecessary braces in single statement block Jaehee Park
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: Jaehee Park @ 2022-04-13 20:11 UTC (permalink / raw)
  To: Larry.Finger
  Cc: phil, gregkh, linux-staging, linux-kernel, outreachy, jhpark1013

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

-- 
2.25.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/6] staging: r8188eu: remove unnecessary braces in single statement block
  2022-04-13 20:11 [PATCH 0/6] staging: r8188eu: fix warnings reported by checkpatch Jaehee Park
@ 2022-04-13 20:11 ` Jaehee Park
  2022-04-13 20:24   ` Pavel Skripkin
  2022-04-13 20:11 ` [PATCH 2/6] staging: r8188eu: remove spaces before tabs Jaehee Park
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Jaehee Park @ 2022-04-13 20:11 UTC (permalink / raw)
  To: Larry.Finger
  Cc: phil, gregkh, linux-staging, linux-kernel, outreachy, jhpark1013

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 <jhpark1013@gmail.com>
---
 drivers/staging/r8188eu/core/rtw_mlme.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
index 3e9882f89f76..d3f4d7cdfa08 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme.c
@@ -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);
-	}
 
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/6] staging: r8188eu: remove spaces before tabs
  2022-04-13 20:11 [PATCH 0/6] staging: r8188eu: fix warnings reported by checkpatch Jaehee Park
  2022-04-13 20:11 ` [PATCH 1/6] staging: r8188eu: remove unnecessary braces in single statement block Jaehee Park
@ 2022-04-13 20:11 ` Jaehee Park
  2022-04-13 20:11 ` [PATCH 3/6] staging: r8188eu: remove 'added by' author comments Jaehee Park
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Jaehee Park @ 2022-04-13 20:11 UTC (permalink / raw)
  To: Larry.Finger
  Cc: phil, gregkh, linux-staging, linux-kernel, outreachy, jhpark1013

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

Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
---
 drivers/staging/r8188eu/core/rtw_mlme.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
index d3f4d7cdfa08..2cfd8e8d74a4 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme.c
@@ -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 */
 /*  */
 /*  */
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/6] staging: r8188eu: remove 'added by' author comments
  2022-04-13 20:11 [PATCH 0/6] staging: r8188eu: fix warnings reported by checkpatch Jaehee Park
  2022-04-13 20:11 ` [PATCH 1/6] staging: r8188eu: remove unnecessary braces in single statement block Jaehee Park
  2022-04-13 20:11 ` [PATCH 2/6] staging: r8188eu: remove spaces before tabs Jaehee Park
@ 2022-04-13 20:11 ` Jaehee Park
  2022-04-13 20:28   ` Pavel Skripkin
  2022-04-13 20:11 ` [PATCH 4/6] staging: r8188eu: place constants on the right side of tests Jaehee Park
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Jaehee Park @ 2022-04-13 20:11 UTC (permalink / raw)
  To: Larry.Finger
  Cc: phil, gregkh, linux-staging, linux-kernel, outreachy, jhpark1013,
	Alison Schofield

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.

Suggested-by: Alison Schofield <alison.schofield@intel.com>
Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
---
 drivers/staging/r8188eu/core/rtw_mlme.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
index 2cfd8e8d74a4..5adef9b9108d 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme.c
@@ -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: */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/6] staging: r8188eu: place constants on the right side of tests
  2022-04-13 20:11 [PATCH 0/6] staging: r8188eu: fix warnings reported by checkpatch Jaehee Park
                   ` (2 preceding siblings ...)
  2022-04-13 20:11 ` [PATCH 3/6] staging: r8188eu: remove 'added by' author comments Jaehee Park
@ 2022-04-13 20:11 ` Jaehee Park
  2022-04-14  1:16   ` Joe Perches
  2022-04-13 20:11 ` [PATCH 5/6] staging: r8188eu: replace spaces with tabs Jaehee Park
  2022-04-13 20:11 ` [PATCH 6/6] staging: r8188eu: correct typo in comments Jaehee Park
  5 siblings, 1 reply; 17+ messages in thread
From: Jaehee Park @ 2022-04-13 20:11 UTC (permalink / raw)
  To: Larry.Finger
  Cc: phil, gregkh, linux-staging, linux-kernel, outreachy, jhpark1013

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 <jhpark1013@gmail.com>
---
 drivers/staging/r8188eu/core/rtw_mlme.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
index 5adef9b9108d..b943fb190e4c 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme.c
@@ -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);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 5/6] staging: r8188eu: replace spaces with tabs
  2022-04-13 20:11 [PATCH 0/6] staging: r8188eu: fix warnings reported by checkpatch Jaehee Park
                   ` (3 preceding siblings ...)
  2022-04-13 20:11 ` [PATCH 4/6] staging: r8188eu: place constants on the right side of tests Jaehee Park
@ 2022-04-13 20:11 ` Jaehee Park
  2022-04-13 20:34   ` Pavel Skripkin
  2022-04-13 20:11 ` [PATCH 6/6] staging: r8188eu: correct typo in comments Jaehee Park
  5 siblings, 1 reply; 17+ messages in thread
From: Jaehee Park @ 2022-04-13 20:11 UTC (permalink / raw)
  To: Larry.Finger
  Cc: phil, gregkh, linux-staging, linux-kernel, outreachy, jhpark1013

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

Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
---
 drivers/staging/r8188eu/core/rtw_mlme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
index b943fb190e4c..7a90fe826d1d 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme.c
@@ -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);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 6/6] staging: r8188eu: correct typo in comments
  2022-04-13 20:11 [PATCH 0/6] staging: r8188eu: fix warnings reported by checkpatch Jaehee Park
                   ` (4 preceding siblings ...)
  2022-04-13 20:11 ` [PATCH 5/6] staging: r8188eu: replace spaces with tabs Jaehee Park
@ 2022-04-13 20:11 ` Jaehee Park
  5 siblings, 0 replies; 17+ messages in thread
From: Jaehee Park @ 2022-04-13 20:11 UTC (permalink / raw)
  To: Larry.Finger
  Cc: phil, gregkh, linux-staging, linux-kernel, outreachy, jhpark1013

Correct misspellings in the comments. Issue found with checkpatch.

Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
---
 drivers/staging/r8188eu/core/rtw_mlme.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
index 7a90fe826d1d..d422ce87ba7c 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme.c
@@ -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)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/6] staging: r8188eu: remove unnecessary braces in single statement block
  2022-04-13 20:11 ` [PATCH 1/6] staging: r8188eu: remove unnecessary braces in single statement block Jaehee Park
@ 2022-04-13 20:24   ` Pavel Skripkin
  2022-04-14 19:41     ` Jaehee Park
  0 siblings, 1 reply; 17+ messages in thread
From: Pavel Skripkin @ 2022-04-13 20:24 UTC (permalink / raw)
  To: Jaehee Park, Larry.Finger
  Cc: phil, gregkh, linux-staging, linux-kernel, outreachy

Hi Jaehee,

On 4/13/22 23:11, 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 <jhpark1013@gmail.com>
> ---
>   drivers/staging/r8188eu/core/rtw_mlme.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
> index 3e9882f89f76..d3f4d7cdfa08 100644
> --- a/drivers/staging/r8188eu/core/rtw_mlme.c
> +++ b/drivers/staging/r8188eu/core/rtw_mlme.c
> @@ -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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/6] staging: r8188eu: remove 'added by' author comments
  2022-04-13 20:11 ` [PATCH 3/6] staging: r8188eu: remove 'added by' author comments Jaehee Park
@ 2022-04-13 20:28   ` Pavel Skripkin
  2022-04-14 19:41     ` Jaehee Park
  0 siblings, 1 reply; 17+ messages in thread
From: Pavel Skripkin @ 2022-04-13 20:28 UTC (permalink / raw)
  To: Jaehee Park, Larry.Finger
  Cc: phil, gregkh, linux-staging, linux-kernel, outreachy, Alison Schofield

Hi Jaehee,

On 4/13/22 23:11, Jaehee Park wrote:
> 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/6] staging: r8188eu: replace spaces with tabs
  2022-04-13 20:11 ` [PATCH 5/6] staging: r8188eu: replace spaces with tabs Jaehee Park
@ 2022-04-13 20:34   ` Pavel Skripkin
  2022-04-14 19:43     ` Jaehee Park
  0 siblings, 1 reply; 17+ messages in thread
From: Pavel Skripkin @ 2022-04-13 20:34 UTC (permalink / raw)
  To: Jaehee Park, Larry.Finger
  Cc: phil, gregkh, linux-staging, linux-kernel, outreachy

Hi Jaehee,

On 4/13/22 23:11, Jaehee Park wrote:
> Use tabs instead of spaces. Issue found with checkpatch.
> WARNING: suspect code indent for conditional statements
> 
> Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
> ---
>   drivers/staging/r8188eu/core/rtw_mlme.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
> index b943fb190e4c..7a90fe826d1d 100644
> --- a/drivers/staging/r8188eu/core/rtw_mlme.c
> +++ b/drivers/staging/r8188eu/core/rtw_mlme.c
> @@ -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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/6] staging: r8188eu: place constants on the right side of tests
  2022-04-13 20:11 ` [PATCH 4/6] staging: r8188eu: place constants on the right side of tests Jaehee Park
@ 2022-04-14  1:16   ` Joe Perches
  2022-04-14 19:46     ` Jaehee Park
  0 siblings, 1 reply; 17+ messages in thread
From: Joe Perches @ 2022-04-14  1:16 UTC (permalink / raw)
  To: Jaehee Park, Larry.Finger
  Cc: phil, gregkh, linux-staging, linux-kernel, outreachy

On Wed, 2022-04-13 at 16:11 -0400, Jaehee Park wrote:
> 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.
[]
> diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
[]
> @@ -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?

[]

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

here too

>  					continue;
>  				} else {
>  					rtw_indicate_disconnect(padapter);



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/6] staging: r8188eu: remove unnecessary braces in single statement block
  2022-04-13 20:24   ` Pavel Skripkin
@ 2022-04-14 19:41     ` Jaehee Park
  2022-04-14 19:50       ` Pavel Skripkin
  0 siblings, 1 reply; 17+ messages in thread
From: Jaehee Park @ 2022-04-14 19:41 UTC (permalink / raw)
  To: Pavel Skripkin
  Cc: Larry.Finger, phil, gregkh, linux-staging, linux-kernel, outreachy

On Wed, Apr 13, 2022 at 11:24:46PM +0300, Pavel Skripkin wrote:
> Hi Jaehee,
> 
> On 4/13/22 23:11, 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 <jhpark1013@gmail.com>
> > ---
> >   drivers/staging/r8188eu/core/rtw_mlme.c | 3 +--
> >   1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
> > index 3e9882f89f76..d3f4d7cdfa08 100644
> > --- a/drivers/staging/r8188eu/core/rtw_mlme.c
> > +++ b/drivers/staging/r8188eu/core/rtw_mlme.c
> > @@ -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?

> 
> 
> 
> With regards,
> Pavel Skripkin

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/6] staging: r8188eu: remove 'added by' author comments
  2022-04-13 20:28   ` Pavel Skripkin
@ 2022-04-14 19:41     ` Jaehee Park
  0 siblings, 0 replies; 17+ messages in thread
From: Jaehee Park @ 2022-04-14 19:41 UTC (permalink / raw)
  To: Pavel Skripkin
  Cc: Larry.Finger, phil, gregkh, linux-staging, linux-kernel,
	outreachy, Alison Schofield

On Wed, Apr 13, 2022 at 11:28:31PM +0300, Pavel Skripkin wrote:
> Hi Jaehee,
> 
> On 4/13/22 23:11, Jaehee Park wrote:
> > 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.

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

> 
> With regards,
> Pavel Skripkin

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/6] staging: r8188eu: replace spaces with tabs
  2022-04-13 20:34   ` Pavel Skripkin
@ 2022-04-14 19:43     ` Jaehee Park
  0 siblings, 0 replies; 17+ messages in thread
From: Jaehee Park @ 2022-04-14 19:43 UTC (permalink / raw)
  To: Pavel Skripkin
  Cc: Larry.Finger, phil, gregkh, linux-staging, linux-kernel, outreachy

On Wed, Apr 13, 2022 at 11:34:14PM +0300, Pavel Skripkin wrote:
> Hi Jaehee,
> 
> On 4/13/22 23:11, Jaehee Park wrote:
> > Use tabs instead of spaces. Issue found with checkpatch.
> > WARNING: suspect code indent for conditional statements
> > 
> > Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
> > ---
> >   drivers/staging/r8188eu/core/rtw_mlme.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
> > index b943fb190e4c..7a90fe826d1d 100644
> > --- a/drivers/staging/r8188eu/core/rtw_mlme.c
> > +++ b/drivers/staging/r8188eu/core/rtw_mlme.c
> > @@ -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?
> 

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. 

> 
> 
> With regards,
> Pavel Skripkin

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/6] staging: r8188eu: place constants on the right side of tests
  2022-04-14  1:16   ` Joe Perches
@ 2022-04-14 19:46     ` Jaehee Park
  0 siblings, 0 replies; 17+ messages in thread
From: Jaehee Park @ 2022-04-14 19:46 UTC (permalink / raw)
  To: Joe Perches
  Cc: Larry.Finger, phil, gregkh, linux-staging, linux-kernel, outreachy

On Wed, Apr 13, 2022 at 06:16:58PM -0700, Joe Perches wrote:
> On Wed, 2022-04-13 at 16:11 -0400, Jaehee Park wrote:
> > 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.
> []
> > diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
> []
> > @@ -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?
> 

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

> []
> 
> > -				if (0 < pmlmepriv->to_roaming) {
> > +				if (pmlmepriv->to_roaming > 0) {
> 
> here too
> 
> >  					continue;
> >  				} else {
> >  					rtw_indicate_disconnect(padapter);
> 
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/6] staging: r8188eu: remove unnecessary braces in single statement block
  2022-04-14 19:41     ` Jaehee Park
@ 2022-04-14 19:50       ` Pavel Skripkin
  2022-04-15  2:51         ` Jaehee Park
  0 siblings, 1 reply; 17+ messages in thread
From: Pavel Skripkin @ 2022-04-14 19:50 UTC (permalink / raw)
  To: Jaehee Park
  Cc: Larry.Finger, phil, gregkh, linux-staging, linux-kernel, outreachy


[-- Attachment #1.1: Type: text/plain, Size: 1040 bytes --]

Hi Jaehee,

On 4/14/22 22:41, Jaehee Park wrote:
>> > @@ -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?
> 

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;

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/6] staging: r8188eu: remove unnecessary braces in single statement block
  2022-04-14 19:50       ` Pavel Skripkin
@ 2022-04-15  2:51         ` Jaehee Park
  0 siblings, 0 replies; 17+ messages in thread
From: Jaehee Park @ 2022-04-15  2:51 UTC (permalink / raw)
  To: Pavel Skripkin
  Cc: Larry.Finger, phil, gregkh, linux-staging, linux-kernel, outreachy

On Thu, Apr 14, 2022 at 10:50:52PM +0300, Pavel Skripkin wrote:
> Hi Jaehee,
> 
> On 4/14/22 22:41, Jaehee Park wrote:
> > > > @@ -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?
> > 
> 
> 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;
> 
> 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
> 
> 
> With regards,
> Pavel Skripkin




^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2022-04-15  2:51 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-13 20:11 [PATCH 0/6] staging: r8188eu: fix warnings reported by checkpatch Jaehee Park
2022-04-13 20:11 ` [PATCH 1/6] staging: r8188eu: remove unnecessary braces in single statement block Jaehee Park
2022-04-13 20:24   ` Pavel Skripkin
2022-04-14 19:41     ` Jaehee Park
2022-04-14 19:50       ` Pavel Skripkin
2022-04-15  2:51         ` Jaehee Park
2022-04-13 20:11 ` [PATCH 2/6] staging: r8188eu: remove spaces before tabs Jaehee Park
2022-04-13 20:11 ` [PATCH 3/6] staging: r8188eu: remove 'added by' author comments Jaehee Park
2022-04-13 20:28   ` Pavel Skripkin
2022-04-14 19:41     ` Jaehee Park
2022-04-13 20:11 ` [PATCH 4/6] staging: r8188eu: place constants on the right side of tests Jaehee Park
2022-04-14  1:16   ` Joe Perches
2022-04-14 19:46     ` Jaehee Park
2022-04-13 20:11 ` [PATCH 5/6] staging: r8188eu: replace spaces with tabs Jaehee Park
2022-04-13 20:34   ` Pavel Skripkin
2022-04-14 19:43     ` Jaehee Park
2022-04-13 20:11 ` [PATCH 6/6] staging: r8188eu: correct typo in comments Jaehee Park

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).