All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.