[PATCH 4/4] staging: rtl8188eu: include: placed constant on the right side of the test in comparisons

[PATCH 4/4] staging: rtl8188eu: include: placed constant on the right side of the test in comparisons

[B K Karthik]

[-- Attachment #1: Type: text/plain, Size: 1644 bytes --]

placed constant on the right side of the test
to fix warnings issued by checkpatch

Signed-off-by: B K Karthik <karthik.bk2000@live.com>
---
 drivers/staging/rtl8188eu/include/rtw_mlme.h | 4 ++--
 drivers/staging/rtl8188eu/include/wifi.h     | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8188eu/include/rtw_mlme.h b/drivers/staging/rtl8188eu/include/rtw_mlme.h
index 010f0c42368a..1b74b32b8a81 100644
--- a/drivers/staging/rtl8188eu/include/rtw_mlme.h
+++ b/drivers/staging/rtl8188eu/include/rtw_mlme.h
@@ -266,7 +266,7 @@ static inline void set_fwstate(struct mlme_priv *pmlmepriv, int state)
 {
 	pmlmepriv->fw_state |= state;
 	/* FOR HW integration */
-	if (_FW_UNDER_SURVEY == state)
+	if (state == _FW_UNDER_SURVEY)
 		pmlmepriv->bScanInProcess = true;
 }
 
@@ -274,7 +274,7 @@ static inline void _clr_fwstate_(struct mlme_priv *pmlmepriv, int state)
 {
 	pmlmepriv->fw_state &= ~state;
 	/* FOR HW integration */
-	if (_FW_UNDER_SURVEY == state)
+	if (state == _FW_UNDER_SURVEY)
 		pmlmepriv->bScanInProcess = false;
 }
 
diff --git a/drivers/staging/rtl8188eu/include/wifi.h b/drivers/staging/rtl8188eu/include/wifi.h
index 677827900607..dcef3da21970 100644
--- a/drivers/staging/rtl8188eu/include/wifi.h
+++ b/drivers/staging/rtl8188eu/include/wifi.h
@@ -326,7 +326,7 @@ static inline unsigned char *get_hdr_bssid(unsigned char *pframe)
 
 static inline int IsFrameTypeCtrl(unsigned char *pframe)
 {
-	if (WIFI_CTRL_TYPE == GetFrameType(pframe))
+	if (GetFrameType(pframe) == WIFI_CTRL_TYPE)
 		return true;
 	else
 		return false;
-- 
2.20.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 4/4] staging: rtl8188eu: include: placed constant on the right side of the test in comparisons

[Joe Perches]

On Sat, 2020-07-18 at 05:18 -0400, B K Karthik wrote:
> placed constant on the right side of the test
> to fix warnings issued by checkpatch
[]
> diff --git a/drivers/staging/rtl8188eu/include/wifi.h b/drivers/staging/rtl8188eu/include/wifi.h
[]
> @@ -326,7 +326,7 @@ static inline unsigned char *get_hdr_bssid(unsigned char *pframe)
>  
>  static inline int IsFrameTypeCtrl(unsigned char *pframe)
>  {
> -	if (WIFI_CTRL_TYPE == GetFrameType(pframe))
> +	if (GetFrameType(pframe) == WIFI_CTRL_TYPE)
>  		return true;
>  	else
>  		return false;

Always try to improve code instead of merely shutting
up checkpatch warnings.

This function should likely be written:

static inline bool IsFrameTypeCtrl(unsigned char *pframe)
{
	return GetFrameType(pframe) == WIFI_CTRL_TYPE;
}

and given it's used only once, it might be expanded
in that place and removed altogether.

Something like:

(and the memcmp below could be ether_addr_equal instead
 but I'm too lazy to find out if the addresses are both
 guaranteed to be __aligned(2) which is likely)

---
 drivers/staging/rtl8188eu/hal/rtl8188e_rxdesc.c | 8 +++++---
 drivers/staging/rtl8188eu/include/wifi.h        | 7 -------
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_rxdesc.c b/drivers/staging/rtl8188eu/hal/rtl8188e_rxdesc.c
index 7d0135fde795..a2994f9ecbde 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188e_rxdesc.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188e_rxdesc.c
@@ -144,10 +144,12 @@ void update_recvframe_phyinfo_88e(struct recv_frame *precvframe,
 
 	wlanhdr = precvframe->pkt->data;
 
-	pkt_info.bPacketMatchBSSID = ((!IsFrameTypeCtrl(wlanhdr)) &&
-		!pattrib->icv_err && !pattrib->crc_err &&
+	pkt_info.bPacketMatchBSSID =
+		GetFrameType(wlanhdr) != WIFI_CTRL_TYPE &&
+		!pattrib->icv_err &&
+		!pattrib->crc_err &&
 		!memcmp(get_hdr_bssid(wlanhdr),
-		 get_bssid(&padapter->mlmepriv), ETH_ALEN));
+			get_bssid(&padapter->mlmepriv), ETH_ALEN);
 
 	pkt_info.bPacketToSelf = pkt_info.bPacketMatchBSSID &&
 				 (!memcmp(get_da(wlanhdr),
diff --git a/drivers/staging/rtl8188eu/include/wifi.h b/drivers/staging/rtl8188eu/include/wifi.h
index 791f287a546d..3998d5633860 100644
--- a/drivers/staging/rtl8188eu/include/wifi.h
+++ b/drivers/staging/rtl8188eu/include/wifi.h
@@ -324,13 +324,6 @@ static inline unsigned char *get_hdr_bssid(unsigned char *pframe)
 	return sa;
 }
 
-static inline int IsFrameTypeCtrl(unsigned char *pframe)
-{
-	if (WIFI_CTRL_TYPE == GetFrameType(pframe))
-		return true;
-	else
-		return false;
-}
 /*-----------------------------------------------------------------------------
 			Below is for the security related definition
 ------------------------------------------------------------------------------*/

Re: [PATCH 4/4] staging: rtl8188eu: include: placed constant on the right side of the test in comparisons

[B K Karthik]

On Sat, Jul 18, 2020 at 9:17 AM Joe Perches <joe@perches.com> wrote:
>
> On Sat, 2020-07-18 at 05:18 -0400, B K Karthik wrote:
> > placed constant on the right side of the test
> > to fix warnings issued by checkpatch
> []
> > diff --git a/drivers/staging/rtl8188eu/include/wifi.h b/drivers/staging/rtl8188eu/include/wifi.h
> []
> > @@ -326,7 +326,7 @@ static inline unsigned char *get_hdr_bssid(unsigned char *pframe)
> >
> >  static inline int IsFrameTypeCtrl(unsigned char *pframe)
> >  {
> > -     if (WIFI_CTRL_TYPE == GetFrameType(pframe))
> > +     if (GetFrameType(pframe) == WIFI_CTRL_TYPE)
> >               return true;
> >       else
> >               return false;
>
> Always try to improve code instead of merely shutting
> up checkpatch warnings.
>
> This function should likely be written:
>
> static inline bool IsFrameTypeCtrl(unsigned char *pframe)
> {
>         return GetFrameType(pframe) == WIFI_CTRL_TYPE;
> }
>
> and given it's used only once, it might be expanded
> in that place and removed altogether.
>
> Something like:
>
> (and the memcmp below could be ether_addr_equal instead
>  but I'm too lazy to find out if the addresses are both
>  guaranteed to be __aligned(2) which is likely)

Alright, I will try to do that and send a v2 asap :)
>
> ---
>  drivers/staging/rtl8188eu/hal/rtl8188e_rxdesc.c | 8 +++++---
>  drivers/staging/rtl8188eu/include/wifi.h        | 7 -------
>  2 files changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_rxdesc.c b/drivers/staging/rtl8188eu/hal/rtl8188e_rxdesc.c
> index 7d0135fde795..a2994f9ecbde 100644
> --- a/drivers/staging/rtl8188eu/hal/rtl8188e_rxdesc.c
> +++ b/drivers/staging/rtl8188eu/hal/rtl8188e_rxdesc.c
> @@ -144,10 +144,12 @@ void update_recvframe_phyinfo_88e(struct recv_frame *precvframe,
>
>         wlanhdr = precvframe->pkt->data;
>
> -       pkt_info.bPacketMatchBSSID = ((!IsFrameTypeCtrl(wlanhdr)) &&
> -               !pattrib->icv_err && !pattrib->crc_err &&
> +       pkt_info.bPacketMatchBSSID =
> +               GetFrameType(wlanhdr) != WIFI_CTRL_TYPE &&
> +               !pattrib->icv_err &&
> +               !pattrib->crc_err &&
>                 !memcmp(get_hdr_bssid(wlanhdr),
> -                get_bssid(&padapter->mlmepriv), ETH_ALEN));
> +                       get_bssid(&padapter->mlmepriv), ETH_ALEN);
>
>         pkt_info.bPacketToSelf = pkt_info.bPacketMatchBSSID &&
>                                  (!memcmp(get_da(wlanhdr),
> diff --git a/drivers/staging/rtl8188eu/include/wifi.h b/drivers/staging/rtl8188eu/include/wifi.h
> index 791f287a546d..3998d5633860 100644
> --- a/drivers/staging/rtl8188eu/include/wifi.h
> +++ b/drivers/staging/rtl8188eu/include/wifi.h
> @@ -324,13 +324,6 @@ static inline unsigned char *get_hdr_bssid(unsigned char *pframe)
>         return sa;
>  }
>
> -static inline int IsFrameTypeCtrl(unsigned char *pframe)
> -{
> -       if (WIFI_CTRL_TYPE == GetFrameType(pframe))
> -               return true;
> -       else
> -               return false;
> -}
>  /*-----------------------------------------------------------------------------
>                         Below is for the security related definition
>  ------------------------------------------------------------------------------*/

thank you for pointing me.

karthik