staging: r8188eu: fix warnings reported by checkpatch

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

lore

Share & tools

lore

Jaehee Park

author

2022-04-15 02:48 UTC #0 lore raw

v2 cover staging: r8188eu: fix warnings reported by checkpatch

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

changes in v2:
- changed the first patch to from removing braces around single-lined statements, to a patch that removes the entire if-statement (because it was redundant). The member within that statement is not being used so this patch has been changed to a patch that removes this unused member.
- the log of the third patch has edited.

Jaehee Park (6):
staging: r8188eu: remove unused member free_bss_buf
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/include/rtw_mlme.h |  1 -
 drivers/staging/r8188eu/core/rtw_mlme.c    | 46 +++++++++-------------
 2 files changed, 18 insertions(+), 29 deletions(-)

Signature · 1 line

2.25.1

Jaehee Park

author

2022-04-15 02:48 UTC #1 lore raw

PATCH v2 1/6 staging: r8188eu: remove unused member free_bss_buf

The free_bss_buf member of pmlmepriv is unused. Remove all related lines.

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

---
 drivers/staging/r8188eu/include/rtw_mlme.h | 1 -
 drivers/staging/r8188eu/core/rtw_mlme.c    | 7 -------
 2 files changed, 8 deletions(-)

drivers/staging/r8188eu/include/rtw_mlme.h +0 -1

index 1dc1fbf049af..0f03ac43079c 100644
@@ -319,7 +319,6 @@ struct mlme_priv {
 	struct list_head *pscanned;
 	struct __queue free_bss_pool;
 	struct __queue scanned_queue;
-	u8 *free_bss_buf;
 	u8	key_mask; /* use to restore wep key after hal_init */
 	u32	num_of_scanned;

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

index 3e9882f89f76..aed868d1d47b 100644
@@ -61,7 +61,6 @@ static int _rtw_init_mlme_priv(struct adapter *padapter)
 		res = _FAIL;
 		goto exit;
 	}
-	pmlmepriv->free_bss_buf = pbuf;
 
 	pnetwork = (struct wlan_network *)pbuf;

@@ -109,13 +108,7 @@ void rtw_free_mlme_priv_ie_data(struct mlme_priv *pmlmepriv)
 
 void _rtw_free_mlme_priv(struct mlme_priv *pmlmepriv)
 {
-
 	rtw_free_mlme_priv_ie_data(pmlmepriv);
-
-	if (pmlmepriv) {
-		vfree(pmlmepriv->free_bss_buf);
-	}
-
 }
 
 struct	wlan_network *_rtw_alloc_network(struct	mlme_priv *pmlmepriv)/* _queue *free_queue) */

Signature · 1 line

2.25.1

Fabio M. De Francesco

2022-04-15 04:29 UTC #2 lore raw

Re: [PATCH v2 1/6] staging: r8188eu: remove unused member free_bss_buf

drivers/

Quoted 11 lines · On venerdì 15 aprile 2022 04:48:32 CEST Jaehee Park wrote:

The free_bss_buf member of pmlmepriv is unused. Remove all related lines.
Suggested-by Pavel Skripkin <[email protected]>
Signed-off-by Jaehee Park <[email protected]>
---
 drivers/staging/r8188eu/include/rtw_mlme.h | 1 -
 drivers/staging/r8188eu/core/rtw_mlme.c    | 7 -------
 2 files changed, 8 deletions(-)
drivers/ renamed from drivers/staging/r8188eu/include/rtw_mlme.h +0 -0

staging/r8188eu/include/rtw_mlme.h

drivers/staging/

Quoted 12 lines

index 1dc1fbf049af..0f03ac43079c 100644
drivers/staging/r8188eu/include/rtw_mlme.h +0 -1
@@ -319,7 +319,6 @@ struct mlme_priv {
 	struct list_head *pscanned;
 	struct __queue free_bss_pool;
 	struct __queue scanned_queue;
-	u8 *free_bss_buf;
 	u8	key_mask; /* use to restore wep key after hal_init */
 	u32	num_of_scanned;
 
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 static int _rtw_init_mlme_priv(struct adapter

Quoted 4 lines

index 3e9882f89f76..aed868d1d47b 100644
drivers/staging/r8188eu/core/rtw_mlme.c +0 -0
@@ -61,7 +61,6 @@ static int _rtw_init_mlme_priv(struct adapter 

*padapter)

drivers/staging/r8188eu/core/rtw_mlme.c static int _rtw_init_mlme_priv(struct adapter *padapter) · line 64

Quoted 4 lines

drivers/staging/r8188eu/core/rtw_mlme.c +0 -1
 		res = _FAIL;
 		goto exit;
 	}
-	pmlmepriv->free_bss_buf = pbuf;

Hi Jaehee,

"pmlmepriv->free_bss_buf" is assigned with "pbuf". The latter is a pointer to virtually contiguous memory which was allocated by vmalloc() or vzalloc() (I didn't check, but the vfree() in _rtw_free_mlme_priv() tells me that indeed it was).

drivers/staging/r8188eu/core/rtw_mlme.c void rtw_free_mlme_priv_ie_data(struct mlme_priv

Quoted 3 lines

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

 	pnetwork = (struct wlan_network *)pbuf;

@@ -109,13 +108,7 @@ void rtw_free_mlme_priv_ie_data(struct mlme_priv

*pmlmepriv)

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

Quoted 9 lines

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

 void _rtw_free_mlme_priv(struct mlme_priv *pmlmepriv)
 {
-
 	rtw_free_mlme_priv_ie_data(pmlmepriv);
-
-	if (pmlmepriv) {
-		vfree(pmlmepriv->free_bss_buf);
-	}

Therefore, here you are causing a memory leak, which is something you should avoid :)

Why did you delete that call to vfree()?

I think that you are misunderstanding what Pavel said. Even if it were true that the code makes no use of that region of memory (again, I didn't check), nevertheless, that memory was allocated somewhere and its address is now in "pmlmepriv->free_bss_buf".

If you can confirm that this memory is allocated for no purpose you should also remove the call to vmalloc() / vzalloc().

Thanks,

Fabio M. De Francesco

drivers/staging/r8188eu/core/rtw_mlme.c void rtw_free_mlme_priv_ie_data(struct mlme_priv *pmlmepriv)

Quoted 4 lines

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

-
 }
 
 struct	wlan_network *_rtw_alloc_network(struct	mlme_priv *pmlmepriv)/*

_queue *free_queue) */

Quoted 3 lines

Signature · 1 line
2.25.1

Jaehee Park

author

2022-04-17 20:14 UTC #3 lore raw

Re: [PATCH v2 1/6] staging: r8188eu: remove unused member free_bss_buf

Quoted 60 lines · Fabio M. De Francesco

drivers/
Quoted 11 lines · On venerdì 15 aprile 2022 04:48:32 CEST Jaehee Park wrote:
The free_bss_buf member of pmlmepriv is unused. Remove all related lines.
Suggested-by Pavel Skripkin <[email protected]>
Signed-off-by Jaehee Park <[email protected]>
---
 drivers/staging/r8188eu/include/rtw_mlme.h | 1 -
 drivers/staging/r8188eu/core/rtw_mlme.c    | 7 -------
 2 files changed, 8 deletions(-)
drivers/ renamed from drivers/staging/r8188eu/include/rtw_mlme.h +0 -0
staging/r8188eu/include/rtw_mlme.h
drivers/staging/
Quoted 12 lines
index 1dc1fbf049af..0f03ac43079c 100644
drivers/staging/r8188eu/include/rtw_mlme.h +0 -1
@@ -319,7 +319,6 @@ struct mlme_priv {
 	struct list_head *pscanned;
 	struct __queue free_bss_pool;
 	struct __queue scanned_queue;
-	u8 *free_bss_buf;
 	u8	key_mask; /* use to restore wep key after hal_init */
 	u32	num_of_scanned;
 
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 static int _rtw_init_mlme_priv(struct adapter
Quoted 4 lines
index 3e9882f89f76..aed868d1d47b 100644
drivers/staging/r8188eu/core/rtw_mlme.c +0 -0
@@ -61,7 +61,6 @@ static int _rtw_init_mlme_priv(struct adapter 
*padapter)
drivers/staging/r8188eu/core/rtw_mlme.c static int _rtw_init_mlme_priv(struct adapter *padapter) · line 64
Quoted 4 lines
drivers/staging/r8188eu/core/rtw_mlme.c +0 -1
 		res = _FAIL;
 		goto exit;
 	}
-	pmlmepriv->free_bss_buf = pbuf;
Hi Jaehee,
"pmlmepriv->free_bss_buf" is assigned with "pbuf". The latter is a pointer to virtually contiguous memory which was allocated by vmalloc() or vzalloc() (I didn't check, but the vfree() in _rtw_free_mlme_priv() tells me that indeed it was).
drivers/staging/r8188eu/core/rtw_mlme.c void rtw_free_mlme_priv_ie_data(struct mlme_priv
Quoted 3 lines
drivers/staging/r8188eu/core/rtw_mlme.c +0 -0
 	pnetwork = (struct wlan_network *)pbuf;
 
@@ -109,13 +108,7 @@ void rtw_free_mlme_priv_ie_data(struct mlme_priv 
*pmlmepriv)
drivers/staging/r8188eu/core/rtw_mlme.c void rtw_free_mlme_priv_ie_data(struct mlme_priv *pmlmepriv) · line 117
Quoted 9 lines
drivers/staging/r8188eu/core/rtw_mlme.c +0 -5
 void _rtw_free_mlme_priv(struct mlme_priv *pmlmepriv)
 {
-
 	rtw_free_mlme_priv_ie_data(pmlmepriv);
-
-	if (pmlmepriv) {
-		vfree(pmlmepriv->free_bss_buf);
-	}
Therefore, here you are causing a memory leak, which is something you should avoid :)

Quoted 7 lines

Why did you delete that call to vfree()?
I think that you are misunderstanding what Pavel said. Even if it were true that the code makes no use of that region of memory (again, I didn't check), nevertheless, that memory was allocated somewhere and its address is now in "pmlmepriv->free_bss_buf".

My understanding of Pavel's response is the free_bss_buf member of the pmlmepriv structure wasn't being used anywhere and that the rtw_free_mlme_riv_ie_data function frees the memory of the pmlmepriv structure so the second check is redundant.

However, as Fabio said, the free_bss_buf member is being used and pbuf memory is not being freed.
So I'll revert the patch as it was originally (which was just removing the {} around the single if statement).

Quoted 17 lines

If you can confirm that this memory is allocated for no purpose you should also remove the call to vmalloc() / vzalloc().
Thanks,
Fabio M. De Francesco
drivers/staging/r8188eu/core/rtw_mlme.c void rtw_free_mlme_priv_ie_data(struct mlme_priv *pmlmepriv)
Quoted 4 lines
drivers/staging/r8188eu/core/rtw_mlme.c +0 -1
-
 }
 
 struct	wlan_network *_rtw_alloc_network(struct	mlme_priv *pmlmepriv)/* 
_queue *free_queue) */
Quoted 3 lines
Signature · 1 line
2.25.1

Pavel Skripkin

2022-04-17 20:16 UTC #4 lore raw

Re: [PATCH v2 1/6] staging: r8188eu: remove unused member free_bss_buf

Hi Jaehee,

Quoted 10 lines · Jaehee Park

My understanding of Pavel's response is the free_bss_buf member of the pmlmepriv structure wasn't being used anywhere and that the rtw_free_mlme_riv_ie_data function frees the memory of the pmlmepriv structure so the second check is redundant.
However, as Fabio said, the free_bss_buf member is being used and pbuf memory is not being freed.
So I'll revert the patch as it was originally (which was just removing the {} around the single if statement).

Why just pbuf allocation can't be removed? This memory is just unused, isn't it?

With regards,
Pavel Skripkin

Jaehee Park

author

2022-04-17 20:42 UTC #5 lore raw

Re: [PATCH v2 1/6] staging: r8188eu: remove unused member free_bss_buf

Quoted 14 lines · Pavel Skripkin

Hi Jaehee,
Quoted 10 lines · Jaehee Park
My understanding of Pavel's response is the free_bss_buf member of the pmlmepriv structure wasn't being used anywhere and that the rtw_free_mlme_riv_ie_data function frees the memory of the pmlmepriv structure so the second check is redundant.
However, as Fabio said, the free_bss_buf member is being used and pbuf memory is not being freed.
So I'll revert the patch as it was originally (which was just removing the {} around the single if statement).

Quoted 8 lines

Why just pbuf allocation can't be removed? This memory is just unused, isn't it?
With regards,
Pavel Skripkin

The free_bss_buf member is unused. So it can just be removed right? I guess I'm confused by what Pablo is saying about causing a memory leak by getting rid of the pointer to the memory allocated by pbuf. Sorry if I misunderstood.

Thanks,
Jaehee

Fabio M. De Francesco

2022-04-17 21:14 UTC #6 lore raw

Re: [PATCH v2 1/6] staging: r8188eu: remove unused member free_bss_buf

Quoted 5 lines · On domenica 17 aprile 2022 22:42:00 CEST Jaehee Park wrote:

Quoted 4 lines · Pavel Skripkin
Hi Jaehee,
Quoted 1 line · Jaehee Park
My understanding of Pavel's response is the free_bss_buf member of

the

Quoted 5 lines

Quoted 5 lines
Quoted 5 lines
pmlmepriv structure wasn't being used anywhere and that the rtw_free_mlme_riv_ie_data function frees the memory of the pmlmepriv structure so the second check is redundant.
However, as Fabio said, the free_bss_buf member is being used and

pbuf

Quoted 2 lines

Quoted 2 lines
Quoted 2 lines
memory is not being freed.
So I'll revert the patch as it was originally (which was just

removing

Quoted 1 line

Quoted 1 line
Quoted 1 line
the {} around the single if statement).

No, Jaehee. This is not what I said :)

Quoted 3 lines

Quoted 3 lines
Quoted 1 line
Why just pbuf allocation can't be removed? This memory is just

unused,

Quoted 1 line

Quoted 1 line
isn't it?

What Pavel said is what I said, but using a different argumentation.

Quoted 7 lines

Quoted 4 lines
With regards,
Pavel Skripkin
The free_bss_buf member is unused.

Correct.

Quoted 1 line

So it can just be removed right?

No.

Quoted 2 lines

I guess I'm confused by what Pablo is saying about causing a memory leak

A memory leak is caused when you allocate some memory and then you lose any reference to its address so that it cannot be freed. Right?

Quoted 1 line

by getting rid of the pointer to the memory allocated by pbuf.

No.

Quoted 1 line

Sorry if I misunderstood.

No problem. Let's rewind...

"pbuf" is assigned with the address of some memory allocated with a call to vzalloc(). Since "pbuf" is a local variable, you see that the above- mentioned address is stored in free_bss_buf using the line "pmlmepriv-

Quoted 1 line

free_bss_buf = pbuf". Is it clear?

Well, you decided to delete the line that calls vfree(pmlmepriv-

Quoted 1 line

free_bss_buf). At this point you have that memory leak.

Pavel noted that pmlmepriv->free_bss_buf is unused, but it contains the address of a region of memory that was allocated for no purpose.

Therefore, a correct patch should also remove the allocation that was made using kzalloc(). If you merely remove the line with vfree() you cause a memory leak.

Please don't revert your patch. Just fix it with a new version that also delete the line where "pbuf" is assigned with the value returned by kzalloc().

I hope that now I've been clearer.

Thanks,

Fabio

Quoted 3 lines

Thanks,
Jaehee

Fabio M. De Francesco

2022-04-17 22:01 UTC #7 lore raw

Re: [PATCH v2 1/6] staging: r8188eu: remove unused member free_bss_buf

Quoted 9 lines · On domenica 17 aprile 2022 23:13:50 CEST Fabio M. De Francesco wrote:

Quoted 5 lines · On domenica 17 aprile 2022 22:42:00 CEST Jaehee Park wrote:
Quoted 4 lines · Pavel Skripkin
Hi Jaehee,
Quoted 1 line · Jaehee Park
My understanding of Pavel's response is the free_bss_buf member of
the
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
pmlmepriv structure wasn't being used anywhere and that the rtw_free_mlme_riv_ie_data function frees the memory of the

pmlmepriv

Quoted 31 lines

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
structure so the second check is redundant.
However, as Fabio said, the free_bss_buf member is being used and
pbuf
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
memory is not being freed.
So I'll revert the patch as it was originally (which was just
removing
Quoted 1 line
Quoted 1 line
Quoted 1 line
the {} around the single if statement).
No, Jaehee. This is not what I said :)
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
Why just pbuf allocation can't be removed? This memory is just
unused,
Quoted 1 line
Quoted 1 line
isn't it?
What Pavel said is what I said, but using a different argumentation.
Quoted 7 lines
Quoted 4 lines
With regards,
Pavel Skripkin
The free_bss_buf member is unused.
Correct.
Quoted 1 line
So it can just be removed right?

Quoted 7 lines

No.
Quoted 2 lines
I guess I'm confused by what Pablo is saying about causing a memory leak
A memory leak is caused when you allocate some memory and then you lose

any

Quoted 11 lines

reference to its address so that it cannot be freed. Right?
Quoted 1 line
by getting rid of the pointer to the memory allocated by pbuf.
No.
Quoted 1 line
Sorry if I misunderstood.
No problem. Let's rewind...
"pbuf" is assigned with the address of some memory allocated with a call

Quoted 11 lines

vzalloc(). Since "pbuf" is a local variable, you see that the above- mentioned address is stored in free_bss_buf using the line "pmlmepriv-
Quoted 1 line
free_bss_buf = pbuf". Is it clear?
Well, you decided to delete the line that calls vfree(pmlmepriv-
Quoted 1 line
free_bss_buf). At this point you have that memory leak.
Pavel noted that pmlmepriv->free_bss_buf is unused, but it contains the address of a region of memory that was allocated for no purpose.
Therefore, a correct patch should also remove the allocation that was

made

Quoted 1 line

using kzalloc().

Sorry I made a typo: kzalloc() -> vzalloc().

Quoted 6 lines

If you merely remove the line with vfree() you cause a memory leak.
Please don't revert your patch. Just fix it with a new version that also delete the line where "pbuf" is assigned with the value returned by kzalloc().

Same here: kzalloc() -> vzalloc().

Quoted 2 lines

I hope that now I've been clearer.

Did you find out where is the line that calls vzalloc() and returns the address to the local variable called "ptr"?

As, said before. You should delete it too, otherwise you lose that region of memory until the driver is un-linked by "modprobe -r <driver>" or the system is shutdown.

Fabio

Jaehee Park

author

2022-04-18 04:49 UTC #8 lore raw

Re: [PATCH v2 1/6] staging: r8188eu: remove unused member free_bss_buf

In reply to Fabio M. De Francesco

Quoted 63 lines · Fabio M. De Francesco

Quoted 5 lines · On domenica 17 aprile 2022 22:42:00 CEST Jaehee Park wrote:
Quoted 4 lines · Pavel Skripkin
Hi Jaehee,
Quoted 1 line · Jaehee Park
My understanding of Pavel's response is the free_bss_buf member of
the
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
pmlmepriv structure wasn't being used anywhere and that the rtw_free_mlme_riv_ie_data function frees the memory of the pmlmepriv structure so the second check is redundant.
However, as Fabio said, the free_bss_buf member is being used and
pbuf
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
memory is not being freed.
So I'll revert the patch as it was originally (which was just
removing
Quoted 1 line
Quoted 1 line
Quoted 1 line
the {} around the single if statement).
No, Jaehee. This is not what I said :)
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
Why just pbuf allocation can't be removed? This memory is just
unused,
Quoted 1 line
Quoted 1 line
isn't it?
What Pavel said is what I said, but using a different argumentation.
Quoted 7 lines
Quoted 4 lines
With regards,
Pavel Skripkin
The free_bss_buf member is unused.
Correct.
Quoted 1 line
So it can just be removed right?
No.
Quoted 2 lines
I guess I'm confused by what Pablo is saying about causing a memory leak
A memory leak is caused when you allocate some memory and then you lose any reference to its address so that it cannot be freed. Right?
Quoted 1 line
by getting rid of the pointer to the memory allocated by pbuf.
No.
Quoted 1 line
Sorry if I misunderstood.
No problem. Let's rewind...
"pbuf" is assigned with the address of some memory allocated with a call to vzalloc(). Since "pbuf" is a local variable, you see that the above- mentioned address is stored in free_bss_buf using the line "pmlmepriv-
Quoted 1 line
free_bss_buf = pbuf". Is it clear?
Well, you decided to delete the line that calls vfree(pmlmepriv-

Quoted 8 lines

Quoted 1 line
free_bss_buf). At this point you have that memory leak.
Pavel noted that pmlmepriv->free_bss_buf is unused, but it contains the address of a region of memory that was allocated for no purpose.
Therefore, a correct patch should also remove the allocation that was made using kzalloc(). If you merely remove the line with vfree() you cause a memory leak.

Hi Fabio, Thank you so much for explaining this so patiently! This makes sense. I'll remove the pbuf vzalloc.
I think I was having trouble because of of how pnetwork was defined in this function. I'll have to think a little more about how to intialize it.
Thanks,
Jaehee

Quoted 18 lines

Please don't revert your patch. Just fix it with a new version that also delete the line where "pbuf" is assigned with the value returned by kzalloc().
I hope that now I've been clearer.
Thanks,
Fabio
Quoted 3 lines
Thanks,
Jaehee

Jaehee Park

author

2022-04-15 02:48 UTC #9 lore raw

PATCH v2 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 aed868d1d47b..1620ca181bf7 100644
@@ -567,8 +567,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)

@@ -903,9 +903,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;

@@ -1628,8 +1628,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-15 02:48 UTC #10 lore raw

PATCH v2 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. These comments are not useful and can be removed.

Reviewed-by Pavel Skripkin <[email protected]>

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 1620ca181bf7..bb9d595a90b9 100644
@@ -903,7 +903,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) {

@@ -1622,9 +1621,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

Pavel Skripkin

2022-04-17 20:23 UTC #11 lore raw

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

Hi Jaehee,

Quoted 4 lines · 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.
Reviewed-by Pavel Skripkin <[email protected]>

I don't remember giving that tag, but now I am OK with it, since patch looks good

Please, don't put random Reviewed-by tags in your commit message, since it's not how Reviewed-by tags work

Quoted 2 lines

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

With regards,
Pavel Skripkin

Jaehee Park

author

2022-04-17 20:49 UTC #12 lore raw

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

Quoted 6 lines · Pavel Skripkin

Hi Jaehee,
Quoted 3 lines · 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.

Quoted 7 lines

Quoted 1 line
Reviewed-by Pavel Skripkin <[email protected]>
I don't remember giving that tag, but now I am OK with it, since patch looks good
Please, don't put random Reviewed-by tags in your commit message, since it's not how Reviewed-by tags work

Got it. Thanks

Quoted 9 lines

Quoted 3 lines
Suggested-by Alison Schofield <[email protected]>
Signed-off-by Jaehee Park <[email protected]>
With regards,
Pavel Skripkin

Jaehee Park

author

2022-04-15 02:48 UTC #13 lore raw

PATCH v2 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 bb9d595a90b9..d57f2ffa069e 100644
@@ -715,7 +715,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);

@@ -723,7 +723,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);

@@ -1964,7 +1965,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);
 		}

@@ -1991,19 +1992,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

Fabio M. De Francesco

2022-04-15 04:57 UTC #14 lore raw

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

drivers/staging/

Quoted 11 lines · On venerdì 15 aprile 2022 04:48:35 CEST 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.
Signed-off-by Jaehee Park <[email protected]>
---
 drivers/staging/r8188eu/core/rtw_mlme.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
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_surveydone_event_callback(struct adapter

Quoted 4 lines

index bb9d595a90b9..d57f2ffa069e 100644
drivers/staging/r8188eu/core/rtw_mlme.c +0 -0
@@ -715,7 +715,7 @@ void rtw_surveydone_event_callback(struct adapter	

*adapter, u8 *pbuf)

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

Quoted 3 lines

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

 			set_fwstate(pmlmepriv, _FW_UNDER_LINKING);
 			pmlmepriv->to_join = false;
 			s_ret =

rtw_select_and_join_from_scanned_queue(pmlmepriv);

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

Quoted 3 lines

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

-			if (_SUCCESS == s_ret) {
+			if (s_ret == _SUCCESS) {
 			     _set_timer(&pmlmepriv->assoc_timer,

MAX_JOIN_TIMEOUT);

drivers/staging/r8188eu/core/rtw_mlme.c void rtw_surveydone_event_callback(struct adapter · line 720

Quoted 1 line

drivers/staging/r8188eu/core/rtw_mlme.c +0 -0
 			} else if (s_ret == 2) { /* there is no need 

to wait for join */

drivers/staging/r8188eu/core/rtw_mlme.c void rtw_surveydone_event_callback(struct adapter · line 721

Quoted 1 line

drivers/staging/r8188eu/core/rtw_mlme.c +0 -0
 				_clr_fwstate_(pmlmepriv, 

_FW_UNDER_LINKING);

As Pavel pointed out, "s_ret" can not ever be equal to '2'; this could have been an opportunity to remove a couple of lines of dead code and add one more patch to your series.

Thanks,

Fabio

Jaehee Park

author

2022-04-15 02:48 UTC #15 lore raw

PATCH v2 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 d57f2ffa069e..38ae8be675c4 100644
@@ -716,7 +716,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-15 02:48 UTC #16 lore raw

PATCH v2 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 38ae8be675c4..58c7d6cc4a3f 100644
@@ -186,7 +186,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)
 {

@@ -322,7 +322,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)
 {

@@ -905,7 +905,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;

@@ -1299,7 +1299,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)

@@ -1334,7 +1334,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