2022-04-14 16:33:58

by Jaehee Park

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

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

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

2022-04-14 19:01:38

by Joe Perches

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

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

2022-04-16 00:14:35

by Jaehee Park

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

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