2022-10-20 02:27:27

by Emily Peri

[permalink] [raw]
Subject: [PATCH 0/3] staging: rtl8723bs: clean up driver code in rtw_ioctl_set

Created patchset to fix all checkpatch warnings in rtw_ioctl_set

Emily Peri (3):
staging: rtl8723bs: fix white space warnings
staging: rtl8723bs: align block comment stars
staging: rtl8723bs: remove unnecessary parenthesis

drivers/staging/rtl8723bs/core/rtw_ioctl_set.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)

--
2.34.1


2022-10-20 02:27:28

by Emily Peri

[permalink] [raw]
Subject: [PATCH 1/3] staging: rtl8723bs: fix white space warnings

Fix the following checkpatch warnings in rtw_ioctl_set:
1) Add missing blankline after declaration
2) Replace spaces used for indent with tab
3) Remove space before tab

Signed-off-by: Emily Peri <[email protected]>
---
drivers/staging/rtl8723bs/core/rtw_ioctl_set.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_ioctl_set.c b/drivers/staging/rtl8723bs/core/rtw_ioctl_set.c
index 8c11daff2d59..354e61a8f2bd 100644
--- a/drivers/staging/rtl8723bs/core/rtw_ioctl_set.c
+++ b/drivers/staging/rtl8723bs/core/rtw_ioctl_set.c
@@ -78,6 +78,7 @@ u8 rtw_do_join(struct adapter *padapter)
goto exit;
} else {
int select_ret;
+
spin_unlock_bh(&(pmlmepriv->scanned_queue.lock));
select_ret = rtw_select_and_join_from_scanned_queue(pmlmepriv);
if (select_ret == _SUCCESS) {
@@ -311,7 +312,7 @@ u8 rtw_set_802_11_infrastructure_mode(struct adapter *padapter,
if ((*pold_state == Ndis802_11Infrastructure) || (*pold_state == Ndis802_11IBSS)) {
if (check_fwstate(pmlmepriv, _FW_LINKED) == true)
rtw_indicate_disconnect(padapter); /* will clr Linked_state; before this function, we must have checked whether issue dis-assoc_cmd or not */
- }
+ }

*pold_state = networktype;

@@ -367,7 +368,7 @@ u8 rtw_set_802_11_disassociate(struct adapter *padapter)

u8 rtw_set_802_11_bssid_list_scan(struct adapter *padapter, struct ndis_802_11_ssid *pssid, int ssid_max_num)
{
- struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
+ struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
u8 res = true;

if (!padapter) {
--
2.34.1

2022-10-20 02:27:29

by Emily Peri

[permalink] [raw]
Subject: [PATCH 2/3] staging: rtl8723bs: align block comment stars

Align '*' on each line of block comment in rtw_ioctl_set.
Issue found by checkpatch.

Signed-off-by: Emily Peri <[email protected]>
---
drivers/staging/rtl8723bs/core/rtw_ioctl_set.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_ioctl_set.c b/drivers/staging/rtl8723bs/core/rtw_ioctl_set.c
index 354e61a8f2bd..ac957035bf1a 100644
--- a/drivers/staging/rtl8723bs/core/rtw_ioctl_set.c
+++ b/drivers/staging/rtl8723bs/core/rtw_ioctl_set.c
@@ -463,11 +463,11 @@ u8 rtw_set_802_11_add_wep(struct adapter *padapter, struct ndis_802_11_wep *wep)
}

/*
-* rtw_get_cur_max_rate -
-* @adapter: pointer to struct adapter structure
-*
-* Return 0 or 100Kbps
-*/
+ * rtw_get_cur_max_rate -
+ * @adapter: pointer to struct adapter structure
+ *
+ * Return 0 or 100Kbps
+ */
u16 rtw_get_cur_max_rate(struct adapter *adapter)
{
int i = 0;
--
2.34.1

2022-10-20 02:28:01

by Emily Peri

[permalink] [raw]
Subject: [PATCH 3/3] staging: rtl8723bs: remove unnecessary parenthesis

Remove extra parenthesis in conditional statement in rtw_ioctl_set.
Issue found by checkpatch.

Signed-off-by: Emily Peri <[email protected]>
---
drivers/staging/rtl8723bs/core/rtw_ioctl_set.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_ioctl_set.c b/drivers/staging/rtl8723bs/core/rtw_ioctl_set.c
index ac957035bf1a..32194dabd587 100644
--- a/drivers/staging/rtl8723bs/core/rtw_ioctl_set.c
+++ b/drivers/staging/rtl8723bs/core/rtw_ioctl_set.c
@@ -160,7 +160,7 @@ u8 rtw_set_802_11_ssid(struct adapter *padapter, struct ndis_802_11_ssid *ssid)
if (check_fwstate(pmlmepriv, _FW_LINKED|WIFI_ADHOC_MASTER_STATE) == true) {
if ((pmlmepriv->assoc_ssid.ssid_length == ssid->ssid_length) &&
(!memcmp(&pmlmepriv->assoc_ssid.ssid, ssid->ssid, ssid->ssid_length))) {
- if ((check_fwstate(pmlmepriv, WIFI_STATION_STATE) == false)) {
+ if (check_fwstate(pmlmepriv, WIFI_STATION_STATE) == false) {
if (rtw_is_same_ibss(padapter, pnetwork) == false) {
/* if in WIFI_ADHOC_MASTER_STATE | WIFI_ADHOC_STATE, create bss or rejoin again */
rtw_disassoc_cmd(padapter, 0, true);
--
2.34.1

2022-10-20 04:28:09

by Praveen Kumar

[permalink] [raw]
Subject: Re: [PATCH 1/3] staging: rtl8723bs: fix white space warnings

On 20-10-2022 07:40, Emily Peri wrote:
> Fix the following checkpatch warnings in rtw_ioctl_set:
> 1) Add missing blankline after declaration
> 2) Replace spaces used for indent with tab
> 3) Remove space before tab
>
> Signed-off-by: Emily Peri <[email protected]>
> ---
> drivers/staging/rtl8723bs/core/rtw_ioctl_set.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_ioctl_set.c b/drivers/staging/rtl8723bs/core/rtw_ioctl_set.c
> index 8c11daff2d59..354e61a8f2bd 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_ioctl_set.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_ioctl_set.c
> @@ -78,6 +78,7 @@ u8 rtw_do_join(struct adapter *padapter)
> goto exit;
> } else {
> int select_ret;
> +
> spin_unlock_bh(&(pmlmepriv->scanned_queue.lock));
> select_ret = rtw_select_and_join_from_scanned_queue(pmlmepriv);
> if (select_ret == _SUCCESS) {
> @@ -311,7 +312,7 @@ u8 rtw_set_802_11_infrastructure_mode(struct adapter *padapter,
> if ((*pold_state == Ndis802_11Infrastructure) || (*pold_state == Ndis802_11IBSS)) {
> if (check_fwstate(pmlmepriv, _FW_LINKED) == true)
> rtw_indicate_disconnect(padapter); /* will clr Linked_state; before this function, we must have checked whether issue dis-assoc_cmd or not */
> - }
> + }

I think indentation is wrong here, it should be only 1 tab instead of 2 tabs ?

>
> *pold_state = networktype;
>
> @@ -367,7 +368,7 @@ u8 rtw_set_802_11_disassociate(struct adapter *padapter)
>
> u8 rtw_set_802_11_bssid_list_scan(struct adapter *padapter, struct ndis_802_11_ssid *pssid, int ssid_max_num)
> {
> - struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
> + struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
> u8 res = true;
>
> if (!padapter) {

Regards,

~Praveen.

2022-10-20 05:18:48

by Emily Peri

[permalink] [raw]
Subject: Re: [PATCH 1/3] staging: rtl8723bs: fix white space warnings

On Thu, Oct 20, 2022 at 09:34:07AM +0530, Praveen Kumar wrote:
> On 20-10-2022 07:40, Emily Peri wrote:
> > Fix the following checkpatch warnings in rtw_ioctl_set:
> > 1) Add missing blankline after declaration
> > 2) Replace spaces used for indent with tab
> > 3) Remove space before tab
> >
> > Signed-off-by: Emily Peri <[email protected]>
> > ---
> > drivers/staging/rtl8723bs/core/rtw_ioctl_set.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8723bs/core/rtw_ioctl_set.c b/drivers/staging/rtl8723bs/core/rtw_ioctl_set.c
> > index 8c11daff2d59..354e61a8f2bd 100644
> > --- a/drivers/staging/rtl8723bs/core/rtw_ioctl_set.c
> > +++ b/drivers/staging/rtl8723bs/core/rtw_ioctl_set.c
> > @@ -78,6 +78,7 @@ u8 rtw_do_join(struct adapter *padapter)
> > goto exit;
> > } else {
> > int select_ret;
> > +
> > spin_unlock_bh(&(pmlmepriv->scanned_queue.lock));
> > select_ret = rtw_select_and_join_from_scanned_queue(pmlmepriv);
> > if (select_ret == _SUCCESS) {
> > @@ -311,7 +312,7 @@ u8 rtw_set_802_11_infrastructure_mode(struct adapter *padapter,
> > if ((*pold_state == Ndis802_11Infrastructure) || (*pold_state == Ndis802_11IBSS)) {
> > if (check_fwstate(pmlmepriv, _FW_LINKED) == true)
> > rtw_indicate_disconnect(padapter); /* will clr Linked_state; before this function, we must have checked whether issue dis-assoc_cmd or not */
> > - }
> > + }
>
> I think indentation is wrong here, it should be only 1 tab instead of 2 tabs ?

Thanks Praveen for the feedback! But I think this if-statement is nested inside
another one beginning at line 295. Can you confirm?


> >
> > *pold_state = networktype;
> >
> > @@ -367,7 +368,7 @@ u8 rtw_set_802_11_disassociate(struct adapter *padapter)
> >
> > u8 rtw_set_802_11_bssid_list_scan(struct adapter *padapter, struct ndis_802_11_ssid *pssid, int ssid_max_num)
> > {
> > - struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
> > + struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
> > u8 res = true;
> >
> > if (!padapter) {
>
> Regards,
>
> ~Praveen.

Now that I think about-- in this last edit I removed a space to appease
checkpatch, but maybe I actually should have removed the tab?

Thanks!
Emily

2022-10-20 05:41:07

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/3] staging: rtl8723bs: fix white space warnings

On Wed, Oct 19, 2022 at 07:10:51PM -0700, Emily Peri wrote:
> Fix the following checkpatch warnings in rtw_ioctl_set:
> 1) Add missing blankline after declaration
> 2) Replace spaces used for indent with tab
> 3) Remove space before tab

When you have to list the different things you do, you should really
break that up into individual patches. This should be 3.

thanks,

greg k-h

2022-10-20 05:58:37

by Emily Peri

[permalink] [raw]
Subject: Re: [PATCH 1/3] staging: rtl8723bs: fix white space warnings

On Thu, Oct 20, 2022 at 06:57:03AM +0200, Greg KH wrote:
> On Wed, Oct 19, 2022 at 07:10:51PM -0700, Emily Peri wrote:
> > Fix the following checkpatch warnings in rtw_ioctl_set:
> > 1) Add missing blankline after declaration
> > 2) Replace spaces used for indent with tab
> > 3) Remove space before tab
>
> When you have to list the different things you do, you should really
> break that up into individual patches. This should be 3.
>
> thanks,
>
> greg k-h

Thanks greg for the feedback! I got the idea to put them all in the same
patch from the PatchPhilosophy guide, which suggested combining a bunch
of white space corrections into one commit. But, it sounds like I
misinterpreted what the guide was saying. Maybe if there are identical
warnings, such as "trailing white space" in the driver code, could those
corrections go together in a single patch?

2022-10-20 06:16:46

by Michael Straube

[permalink] [raw]
Subject: Re: [PATCH 1/3] staging: rtl8723bs: fix white space warnings

On 10/20/22 07:05, Emily Peri wrote:
>>>
>>> *pold_state = networktype;
>>>
>>> @@ -367,7 +368,7 @@ u8 rtw_set_802_11_disassociate(struct adapter *padapter)
>>>
>>> u8 rtw_set_802_11_bssid_list_scan(struct adapter *padapter, struct ndis_802_11_ssid *pssid, int ssid_max_num)
>>> {
>>> - struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
>>> + struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
>>> u8 res = true;
>>>
>>> if (!padapter) {
>>
>> Regards,
>>
>> ~Praveen.
>
> Now that I think about-- in this last edit I removed a space to appease
> checkpatch, but maybe I actually should have removed the tab?

Hi Emily,

you are right, just use spaces.

struct mlme_priv *pmlmepriv = &padapter->mlmepriv;

thanks,
Michael

2022-10-20 08:02:31

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/3] staging: rtl8723bs: fix white space warnings

On Wed, Oct 19, 2022 at 10:28:42PM -0700, Emily Peri wrote:
> On Thu, Oct 20, 2022 at 06:57:03AM +0200, Greg KH wrote:
> > On Wed, Oct 19, 2022 at 07:10:51PM -0700, Emily Peri wrote:
> > > Fix the following checkpatch warnings in rtw_ioctl_set:
> > > 1) Add missing blankline after declaration
> > > 2) Replace spaces used for indent with tab
> > > 3) Remove space before tab
> >
> > When you have to list the different things you do, you should really
> > break that up into individual patches. This should be 3.
> >
> > thanks,
> >
> > greg k-h
>
> Thanks greg for the feedback! I got the idea to put them all in the same
> patch from the PatchPhilosophy guide, which suggested combining a bunch
> of white space corrections into one commit. But, it sounds like I
> misinterpreted what the guide was saying. Maybe if there are identical
> warnings, such as "trailing white space" in the driver code, could those
> corrections go together in a single patch?
>

Yes, you are correct, you could do a "fix all trailing whitespace in
this file" in one patch.

thanks,

greg k-h