Skip to contentSkip to conversation

staging: r8188eu: Fix clang warnings

PATCH Superseded Nathan Chancellor 3 participants 15 messages Aug 3, 2021 – Aug 5, 2021 Staging

lore
Share & tools
lore
#0 lore raw

cover staging: r8188eu: Fix clang warnings

This series cleans up all of the clang warnings that I noticed with x86_64 allmodconfig on the current staging-next. This has been build tested with both clang and gcc with x86_64 allmodconfig.

Nathan Chancellor (4):
staging: r8188eu: Remove _rtw_spinlock_free()
staging: r8188eu: Remove unnecessary parentheses
staging: r8188eu: Remove self assignment in get_rx_power_val_by_reg()
staging: r8188eu: Remove pointless NULL check in
rtw_check_join_candidate()

 drivers/staging/r8188eu/core/rtw_cmd.c        |  2 -
 drivers/staging/r8188eu/core/rtw_mlme.c       | 18 +----
 drivers/staging/r8188eu/core/rtw_pwrctrl.c    |  2 +-
 drivers/staging/r8188eu/core/rtw_recv.c       | 11 ---
 drivers/staging/r8188eu/core/rtw_security.c   |  4 +-
 drivers/staging/r8188eu/core/rtw_sta_mgt.c    | 77 -------------------
 drivers/staging/r8188eu/core/rtw_wlan_util.c  |  2 +-
 drivers/staging/r8188eu/core/rtw_xmit.c       | 19 -----
 drivers/staging/r8188eu/hal/odm.c             |  2 +-
 drivers/staging/r8188eu/hal/rtl8188e_rf6052.c |  2 -
 drivers/staging/r8188eu/hal/usb_halinit.c     |  2 +-
 .../staging/r8188eu/include/osdep_service.h   |  1 -
 drivers/staging/r8188eu/os_dep/os_intfs.c     |  2 -
 .../staging/r8188eu/os_dep/osdep_service.c    |  4 -
 14 files changed, 9 insertions(+), 139 deletions(-)

base-commit: 11e14fc3e49481b6322bbd976bb7dd2f0d7ef3d2

Signature · 1 line
2.33.0.rc0
#1 lore raw

PATCH 1/4 staging: r8188eu: Remove _rtw_spinlock_free()

Clang warns:

drivers/staging/r8188eu/core/rtw_sta_mgt.c:156:13: warning: comparison of address of 'psta->lock' not equal to a null pointer is always true [-Wtautological-pointer-compare]

        if (&psta->lock != NULL)
             ~~~~~~^~~~    ~~~~

1 warning generated.

rtw_mfree_stainfo() is not called anywhere so this entire function can be removed. Upon further inspection, _rtw_spinlock_free() is an empty function so completely remove its use from the driver. Functions that only call _rtw_spinlock_free() are eliminated as a byproduct.

rtw_mfree_all_stainfo() was eliminated because it would be the only function left in rtw_mfree_sta_priv_lock() but it only iterates through free_sta_queue, not doing anything else.

---
 drivers/staging/r8188eu/core/rtw_cmd.c        |  2 -
 drivers/staging/r8188eu/core/rtw_mlme.c       | 16 +---
 drivers/staging/r8188eu/core/rtw_recv.c       | 11 ---
 drivers/staging/r8188eu/core/rtw_sta_mgt.c    | 77 -------------------
 drivers/staging/r8188eu/core/rtw_xmit.c       | 19 -----
 .../staging/r8188eu/include/osdep_service.h   |  1 -
 drivers/staging/r8188eu/os_dep/os_intfs.c     |  2 -
 .../staging/r8188eu/os_dep/osdep_service.c    |  4 -
 8 files changed, 2 insertions(+), 130 deletions(-)
drivers/staging/r8188eu/core/rtw_cmd.c +0 -2
index 4e17972ee66e..cb7d730286fe 100644
@@ -96,8 +96,6 @@ void _rtw_free_cmd_priv (struct	cmd_priv *pcmdpriv)
 {
 
 	if (pcmdpriv) {
-		_rtw_spinlock_free(&(pcmdpriv->cmd_queue.lock));
-
 		if (pcmdpriv->cmd_allocated_buf)
 			kfree(pcmdpriv->cmd_allocated_buf);
 
drivers/staging/r8188eu/core/rtw_mlme.c +2 -14
index a6d62074289f..fa4df4a3a2df 100644
@@ -85,13 +85,6 @@ int	_rtw_init_mlme_priv (struct adapter *padapter)
 	return res;
 }
 
-static void rtw_mfree_mlme_priv_lock (struct mlme_priv *pmlmepriv)
-{
-	_rtw_spinlock_free(&pmlmepriv->lock);
-	_rtw_spinlock_free(&(pmlmepriv->free_bss_pool.lock));
-	_rtw_spinlock_free(&(pmlmepriv->scanned_queue.lock));
-}
-
 #if defined (CONFIG_88EU_AP_MODE)
 static void rtw_free_mlme_ie_data(u8 **ppie, u32 *plen)
 {
@@ -126,13 +119,8 @@ void _rtw_free_mlme_priv (struct mlme_priv *pmlmepriv)
 
 	rtw_free_mlme_priv_ie_data(pmlmepriv);
 
-	if (pmlmepriv) {
-		rtw_mfree_mlme_priv_lock (pmlmepriv);
-
-		if (pmlmepriv->free_bss_buf) {
-			rtw_vmfree(pmlmepriv->free_bss_buf, MAX_BSS_CNT * sizeof(struct wlan_network));
-		}
-	}
+	if (pmlmepriv && pmlmepriv->free_bss_buf)
+		rtw_vmfree(pmlmepriv->free_bss_buf, MAX_BSS_CNT * sizeof(struct wlan_network));
 
 }
 
drivers/staging/r8188eu/core/rtw_recv.c +0 -11
index 610cc699caf9..1889a7935c97 100644
@@ -96,23 +96,12 @@ int _rtw_init_recv_priv(struct recv_priv *precvpriv, struct adapter *padapter)
 	return res;
 }
 
-static void rtw_mfree_recv_priv_lock(struct recv_priv *precvpriv)
-{
-	_rtw_spinlock_free(&precvpriv->lock);
-	_rtw_spinlock_free(&precvpriv->free_recv_queue.lock);
-	_rtw_spinlock_free(&precvpriv->recv_pending_queue.lock);
-
-	_rtw_spinlock_free(&precvpriv->free_recv_buf_queue.lock);
-}
-
 void _rtw_free_recv_priv (struct recv_priv *precvpriv)
 {
 	struct adapter	*padapter = precvpriv->adapter;
 
 	rtw_free_uc_swdec_pending_queue(padapter);
 
-	rtw_mfree_recv_priv_lock(precvpriv);
-
 	rtw_os_recv_resource_free(precvpriv);
 
 	if (precvpriv->pallocated_frame_buf) {
drivers/staging/r8188eu/core/rtw_sta_mgt.c +0 -77

89 lines not on this page · raw message

drivers/staging/r8188eu/core/rtw_xmit.c +0 -19
index 4a5393184737..499bb8ce0290 100644
@@ -210,21 +210,6 @@ s32	_rtw_init_xmit_priv(struct xmit_priv *pxmitpriv, struct adapter *padapter)
 	return res;
 }
 
-static void  rtw_mfree_xmit_priv_lock (struct xmit_priv *pxmitpriv)
-{
-	_rtw_spinlock_free(&pxmitpriv->lock);
-
-	_rtw_spinlock_free(&pxmitpriv->be_pending.lock);
-	_rtw_spinlock_free(&pxmitpriv->bk_pending.lock);
-	_rtw_spinlock_free(&pxmitpriv->vi_pending.lock);
-	_rtw_spinlock_free(&pxmitpriv->vo_pending.lock);
-	_rtw_spinlock_free(&pxmitpriv->bm_pending.lock);
-
-	_rtw_spinlock_free(&pxmitpriv->free_xmit_queue.lock);
-	_rtw_spinlock_free(&pxmitpriv->free_xmitbuf_queue.lock);
-	_rtw_spinlock_free(&pxmitpriv->pending_xmitbuf_queue.lock);
-}
-
 void _rtw_free_xmit_priv (struct xmit_priv *pxmitpriv)
 {
 	int i;
@@ -236,8 +221,6 @@ void _rtw_free_xmit_priv (struct xmit_priv *pxmitpriv)
 
 	rtw_hal_free_xmit_priv(padapter);
 
-	rtw_mfree_xmit_priv_lock(pxmitpriv);
-
 	if (pxmitpriv->pxmit_frame_buf == NULL)
 		return;
 
@@ -259,8 +242,6 @@ void _rtw_free_xmit_priv (struct xmit_priv *pxmitpriv)
 		rtw_vmfree(pxmitpriv->pallocated_xmitbuf, NR_XMITBUFF * sizeof(struct xmit_buf) + 4);
 
 	/*  free xmit extension buff */
-	_rtw_spinlock_free(&pxmitpriv->free_xmit_extbuf_queue.lock);
-
 	pxmitbuf = (struct xmit_buf *)pxmitpriv->pxmit_extbuf;
 	for (i = 0; i < num_xmit_extbuf; i++) {
 		rtw_os_xmit_resource_free(padapter, pxmitbuf, (max_xmit_extbuf_size + XMITBUF_ALIGN_SZ));
drivers/staging/r8188eu/include/osdep_service.h +0 -1
index ac0bd5cd0f82..df7f293981e7 100644
@@ -230,7 +230,6 @@ void *rtw_malloc2d(int h, int w, int size);
 u32  _rtw_down_sema(struct semaphore *sema);
 void _rtw_mutex_init(struct mutex *pmutex);
 void _rtw_mutex_free(struct mutex *pmutex);
-void _rtw_spinlock_free(spinlock_t *plock);
 
 void _rtw_init_queue(struct __queue *pqueue);
 
drivers/staging/r8188eu/os_dep/os_intfs.c +0 -2
index 5f1ec883d6ed..935e35c82666 100644
@@ -979,8 +979,6 @@ u8 rtw_free_drv_sw(struct adapter *padapter)
 	}
 	#endif
 
-	_rtw_spinlock_free(&padapter->br_ext_lock);
-
 	free_mlme_ext_priv(&padapter->mlmeextpriv);
 
 	rtw_free_cmd_priv(&padapter->cmdpriv);
drivers/staging/r8188eu/os_dep/osdep_service.c +0 -4
index 71d4fa25db86..a23f5e448bf4 100644
@@ -118,10 +118,6 @@ void	_rtw_mutex_free(struct mutex *pmutex)
 	mutex_destroy(pmutex);
 }
 
-void	_rtw_spinlock_free(spinlock_t *plock)
-{
-}
-
 void	_rtw_init_queue(struct __queue *pqueue)
 {
 	INIT_LIST_HEAD(&(pqueue->queue));
Signature · 1 line
2.33.0.rc0
#2 lore raw

PATCH 2/4 staging: r8188eu: Remove unnecessary parentheses

In reply to Nathan Chancellor

Clang warns several times across the driver along the lines of:

drivers/staging/r8188eu/core/rtw_pwrctrl.c:222:21: warning: equality comparison with extraneous parentheses [-Wparentheses-equality]

        if ((pwrpriv->rpwm == pslv)) {
             ~~~~~~~~~~~~~~^~~~~~~

drivers/staging/r8188eu/core/rtw_pwrctrl.c:222:21: note: remove extraneous parentheses around the comparison to silence this warning
if ((pwrpriv->rpwm == pslv)) {
~ ^ ~
drivers/staging/r8188eu/core/rtw_pwrctrl.c:222:21: note: use '=' to turn this equality comparison into an assignment

        if ((pwrpriv->rpwm == pslv)) {
                           ^~
                           =

1 warning generated.

The compilers have agreed that single parentheses are used for equality and double parentheses are used for assignment within control flow statements such as if and while so remove them in these places to fix the warning.

---
 drivers/staging/r8188eu/core/rtw_pwrctrl.c   | 2 +-
 drivers/staging/r8188eu/core/rtw_security.c  | 4 ++--
 drivers/staging/r8188eu/core/rtw_wlan_util.c | 2 +-
 drivers/staging/r8188eu/hal/odm.c            | 2 +-
 drivers/staging/r8188eu/hal/usb_halinit.c    | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)
drivers/staging/r8188eu/core/rtw_pwrctrl.c +1 -1
index d67eeb045002..598c32d7eaa5 100644
@@ -219,7 +219,7 @@ void rtw_set_rpwm(struct adapter *padapter, u8 pslv)
 			pslv = PS_STATE_S3;
 	}
 
-	if ((pwrpriv->rpwm == pslv)) {
+	if (pwrpriv->rpwm == pslv) {
 		RT_TRACE(_module_rtl871x_pwrctrl_c_, _drv_err_,
 			 ("%s: Already set rpwm[0x%02X], new=0x%02X!\n", __func__, pwrpriv->rpwm, pslv));
 		return;
drivers/staging/r8188eu/core/rtw_security.c +2 -2
index 2c1b9a6dcdf2..45fd8b1aeb59 100644
@@ -1211,7 +1211,7 @@ u32	rtw_aes_encrypt(struct adapter *padapter, u8 *pxmitframe)
 	pframe = ((struct xmit_frame *)pxmitframe)->buf_addr + hw_hdr_offset;
 
 	/* 4 start to encrypt each fragment */
-	if ((pattrib->encrypt == _AES_)) {
+	if (pattrib->encrypt == _AES_) {
 		if (pattrib->psta)
 			stainfo = pattrib->psta;
 		else
@@ -1454,7 +1454,7 @@ u32	rtw_aes_decrypt(struct adapter *padapter, u8 *precvframe)
 
 	pframe = (unsigned char *)((struct recv_frame *)precvframe)->rx_data;
 	/* 4 start to encrypt each fragment */
-	if ((prxattrib->encrypt == _AES_)) {
+	if (prxattrib->encrypt == _AES_) {
 		stainfo = rtw_get_stainfo(&padapter->stapriv, &prxattrib->ta[0]);
 		if (stainfo != NULL) {
 			RT_TRACE(_module_rtl871x_security_c_, _drv_err_, ("rtw_aes_decrypt: stainfo!= NULL!!!\n"));
drivers/staging/r8188eu/core/rtw_wlan_util.c +1 -1
index 15edccef9f1d..4a8e52484cfd 100644
@@ -1306,7 +1306,7 @@ int support_short_GI(struct adapter *padapter, struct HT_caps_element *pHT_caps)
 	if (!(pmlmeinfo->HT_enable))
 		return _FAIL;
 
-	if ((pmlmeinfo->assoc_AP_vendor == HT_IOT_PEER_RALINK))
+	if (pmlmeinfo->assoc_AP_vendor == HT_IOT_PEER_RALINK)
 		return _FAIL;
 
 	bit_offset = (pmlmeext->cur_bwmode & HT_CHANNEL_WIDTH_40) ? 6 : 5;
drivers/staging/r8188eu/hal/odm.c +1 -1
index 0bc836311036..65a117408d50 100644
@@ -1631,7 +1631,7 @@ void odm_EdcaTurboCheckCE(struct odm_dm_struct *pDM_Odm)
 	struct mlme_ext_priv	*pmlmeext = &(Adapter->mlmeextpriv);
 	struct mlme_ext_info	*pmlmeinfo = &(pmlmeext->mlmext_info);
 
-	if ((pregpriv->wifi_spec == 1))/*  (pmlmeinfo->HT_enable == 0)) */
+	if (pregpriv->wifi_spec == 1)
 		goto dm_CheckEdcaTurbo_EXIT;
 
 	if (pmlmeinfo->assoc_AP_vendor >=  HT_IOT_PEER_MAX)
drivers/staging/r8188eu/hal/usb_halinit.c +1 -1
index d985894c0f30..ec7badfd72aa 100644
@@ -1300,7 +1300,7 @@ static void hw_var_set_opmode(struct adapter *Adapter, u8 variable, u8 *val)
 		StopTxBeacon(Adapter);
 
 		rtw_write8(Adapter, REG_BCN_CTRL, 0x19);/* disable atim wnd */
-	} else if ((mode == _HW_STATE_ADHOC_)) {
+	} else if (mode == _HW_STATE_ADHOC_) {
 		ResumeTxBeacon(Adapter);
 		rtw_write8(Adapter, REG_BCN_CTRL, 0x1a);
 	} else if (mode == _HW_STATE_AP_) {
Signature · 1 line
2.33.0.rc0
Reviewed-by on v1 2/4
#3 lore raw

Re: [PATCH 2/4] staging: r8188eu: Remove unnecessary parentheses

Quoted 85 lines · Nathan Chancellor

Clang warns several times across the driver along the lines of:

drivers/staging/r8188eu/core/rtw_pwrctrl.c:222:21: warning: equality comparison with extraneous parentheses [-Wparentheses-equality]

        if ((pwrpriv->rpwm == pslv)) {
             ~~~~~~~~~~~~~~^~~~~~~

drivers/staging/r8188eu/core/rtw_pwrctrl.c:222:21: note: remove extraneous parentheses around the comparison to silence this warning
if ((pwrpriv->rpwm == pslv)) {
~ ^ ~
drivers/staging/r8188eu/core/rtw_pwrctrl.c:222:21: note: use '=' to turn this equality comparison into an assignment

        if ((pwrpriv->rpwm == pslv)) {
                           ^~
                           =

1 warning generated.

The compilers have agreed that single parentheses are used for equality and double parentheses are used for assignment within control flow statements such as if and while so remove them in these places to fix the warning.

---
 drivers/staging/r8188eu/core/rtw_pwrctrl.c   | 2 +-
 drivers/staging/r8188eu/core/rtw_security.c  | 4 ++--
 drivers/staging/r8188eu/core/rtw_wlan_util.c | 2 +-
 drivers/staging/r8188eu/hal/odm.c            | 2 +-
 drivers/staging/r8188eu/hal/usb_halinit.c    | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)
drivers/staging/r8188eu/core/rtw_pwrctrl.c +1 -1
index d67eeb045002..598c32d7eaa5 100644
@@ -219,7 +219,7 @@ void rtw_set_rpwm(struct adapter *padapter, u8 pslv)
                        pslv = PS_STATE_S3;
        }
 
-       if ((pwrpriv->rpwm == pslv)) {
+       if (pwrpriv->rpwm == pslv) {
                RT_TRACE(_module_rtl871x_pwrctrl_c_, _drv_err_,
                         ("%s: Already set rpwm[0x%02X], new=0x%02X!\n", __func__, pwrpriv->rpwm, pslv));
                return;
drivers/staging/r8188eu/core/rtw_security.c +2 -2
index 2c1b9a6dcdf2..45fd8b1aeb59 100644
@@ -1211,7 +1211,7 @@ u32       rtw_aes_encrypt(struct adapter *padapter, u8 *pxmitframe)
        pframe = ((struct xmit_frame *)pxmitframe)->buf_addr + hw_hdr_offset;
 
        /* 4 start to encrypt each fragment */
-       if ((pattrib->encrypt == _AES_)) {
+       if (pattrib->encrypt == _AES_) {
                if (pattrib->psta)
                        stainfo = pattrib->psta;
                else
@@ -1454,7 +1454,7 @@ u32       rtw_aes_decrypt(struct adapter *padapter, u8 *precvframe)
 
        pframe = (unsigned char *)((struct recv_frame *)precvframe)->rx_data;
        /* 4 start to encrypt each fragment */
-       if ((prxattrib->encrypt == _AES_)) {
+       if (prxattrib->encrypt == _AES_) {
                stainfo = rtw_get_stainfo(&padapter->stapriv, &prxattrib->ta[0]);
                if (stainfo != NULL) {
                        RT_TRACE(_module_rtl871x_security_c_, _drv_err_, ("rtw_aes_decrypt: stainfo!= NULL!!!\n"));
drivers/staging/r8188eu/core/rtw_wlan_util.c +1 -1
index 15edccef9f1d..4a8e52484cfd 100644
@@ -1306,7 +1306,7 @@ int support_short_GI(struct adapter *padapter, struct HT_caps_element *pHT_caps)
        if (!(pmlmeinfo->HT_enable))
                return _FAIL;
 
-       if ((pmlmeinfo->assoc_AP_vendor == HT_IOT_PEER_RALINK))
+       if (pmlmeinfo->assoc_AP_vendor == HT_IOT_PEER_RALINK)
                return _FAIL;
 
        bit_offset = (pmlmeext->cur_bwmode & HT_CHANNEL_WIDTH_40) ? 6 : 5;
drivers/staging/r8188eu/hal/odm.c +0 -0
index 0bc836311036..65a117408d50 100644

drivers/staging/r8188eu/hal/odm.c void odm_EdcaTurboCheckCE(struct odm_dm_struct *pDM_Odm) · line 1634

Quoted 5 lines
drivers/staging/r8188eu/hal/odm.c +1 -1
@@ -1631,7 +1631,7 @@ void odm_EdcaTurboCheckCE(struct odm_dm_struct *pDM_Odm)
        struct mlme_ext_priv    *pmlmeext = &(Adapter->mlmeextpriv);
        struct mlme_ext_info    *pmlmeinfo = &(pmlmeext->mlmext_info);
 
-       if ((pregpriv->wifi_spec == 1))/*  (pmlmeinfo->HT_enable == 0)) */
+       if (pregpriv->wifi_spec == 1)

^ was the comment you removed important?

If not:

Quoted 23 lines
drivers/staging/r8188eu/hal/odm.c +0 -0
                goto dm_CheckEdcaTurbo_EXIT;
 
        if (pmlmeinfo->assoc_AP_vendor >=  HT_IOT_PEER_MAX)
drivers/staging/r8188eu/hal/usb_halinit.c +1 -2
index d985894c0f30..ec7badfd72aa 100644
@@ -1300,7 +1300,7 @@ static void hw_var_set_opmode(struct adapter *Adapter, u8 variable, u8 *val)
                StopTxBeacon(Adapter);
 
                rtw_write8(Adapter, REG_BCN_CTRL, 0x19);/* disable atim wnd */
-       } else if ((mode == _HW_STATE_ADHOC_)) {
+       } else if (mode == _HW_STATE_ADHOC_) {
                ResumeTxBeacon(Adapter);
                rtw_write8(Adapter, REG_BCN_CTRL, 0x1a);
        } else if (mode == _HW_STATE_AP_) {
--

2.33.0.rc0

--
You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20210803223609.1627280-3-nathan%40kernel.org.

Signature · 2 lines
Thanks,
~Nick Desaulniers
#4 lore raw

Re: [PATCH 2/4] staging: r8188eu: Remove unnecessary parentheses

Quoted 89 lines · Nick Desaulniers
Quoted 88 lines · Nathan Chancellor

Clang warns several times across the driver along the lines of:

drivers/staging/r8188eu/core/rtw_pwrctrl.c:222:21: warning: equality comparison with extraneous parentheses [-Wparentheses-equality]

         if ((pwrpriv->rpwm == pslv)) {
              ~~~~~~~~~~~~~~^~~~~~~

drivers/staging/r8188eu/core/rtw_pwrctrl.c:222:21: note: remove extraneous parentheses around the comparison to silence this warning
if ((pwrpriv->rpwm == pslv)) {
~ ^ ~
drivers/staging/r8188eu/core/rtw_pwrctrl.c:222:21: note: use '=' to turn this equality comparison into an assignment

         if ((pwrpriv->rpwm == pslv)) {
                            ^~
                            =

1 warning generated.

The compilers have agreed that single parentheses are used for equality and double parentheses are used for assignment within control flow statements such as if and while so remove them in these places to fix the warning.

---
  drivers/staging/r8188eu/core/rtw_pwrctrl.c   | 2 +-
  drivers/staging/r8188eu/core/rtw_security.c  | 4 ++--
  drivers/staging/r8188eu/core/rtw_wlan_util.c | 2 +-
  drivers/staging/r8188eu/hal/odm.c            | 2 +-
  drivers/staging/r8188eu/hal/usb_halinit.c    | 2 +-
  5 files changed, 6 insertions(+), 6 deletions(-)
drivers/staging/r8188eu/core/rtw_pwrctrl.c +1 -1
index d67eeb045002..598c32d7eaa5 100644
@@ -219,7 +219,7 @@ void rtw_set_rpwm(struct adapter *padapter, u8 pslv)
                         pslv = PS_STATE_S3;
         }
 
-       if ((pwrpriv->rpwm == pslv)) {
+       if (pwrpriv->rpwm == pslv) {
                 RT_TRACE(_module_rtl871x_pwrctrl_c_, _drv_err_,
                          ("%s: Already set rpwm[0x%02X], new=0x%02X!\n", __func__, pwrpriv->rpwm, pslv));
                 return;
drivers/staging/r8188eu/core/rtw_security.c +2 -2
index 2c1b9a6dcdf2..45fd8b1aeb59 100644
@@ -1211,7 +1211,7 @@ u32       rtw_aes_encrypt(struct adapter *padapter, u8 *pxmitframe)
         pframe = ((struct xmit_frame *)pxmitframe)->buf_addr + hw_hdr_offset;
 
         /* 4 start to encrypt each fragment */
-       if ((pattrib->encrypt == _AES_)) {
+       if (pattrib->encrypt == _AES_) {
                 if (pattrib->psta)
                         stainfo = pattrib->psta;
                 else
@@ -1454,7 +1454,7 @@ u32       rtw_aes_decrypt(struct adapter *padapter, u8 *precvframe)
 
         pframe = (unsigned char *)((struct recv_frame *)precvframe)->rx_data;
         /* 4 start to encrypt each fragment */
-       if ((prxattrib->encrypt == _AES_)) {
+       if (prxattrib->encrypt == _AES_) {
                 stainfo = rtw_get_stainfo(&padapter->stapriv, &prxattrib->ta[0]);
                 if (stainfo != NULL) {
                         RT_TRACE(_module_rtl871x_security_c_, _drv_err_, ("rtw_aes_decrypt: stainfo!= NULL!!!\n"));
drivers/staging/r8188eu/core/rtw_wlan_util.c +1 -1
index 15edccef9f1d..4a8e52484cfd 100644
@@ -1306,7 +1306,7 @@ int support_short_GI(struct adapter *padapter, struct HT_caps_element *pHT_caps)
         if (!(pmlmeinfo->HT_enable))
                 return _FAIL;
 
-       if ((pmlmeinfo->assoc_AP_vendor == HT_IOT_PEER_RALINK))
+       if (pmlmeinfo->assoc_AP_vendor == HT_IOT_PEER_RALINK)
                 return _FAIL;
 
         bit_offset = (pmlmeext->cur_bwmode & HT_CHANNEL_WIDTH_40) ? 6 : 5;
drivers/staging/r8188eu/hal/odm.c +0 -0
index 0bc836311036..65a117408d50 100644
@@ -1631,7 +1631,7 @@ void odm_EdcaTurboCheckCE(struct odm_dm_struct *pDM_Odm)
         struct mlme_ext_priv    *pmlmeext = &(Adapter->mlmeextpriv);
         struct mlme_ext_info    *pmlmeinfo = &(pmlmeext->mlmext_info);
 
Quoted 4 lines

drivers/staging/r8188eu/hal/odm.c void odm_EdcaTurboCheckCE(struct odm_dm_struct *pDM_Odm) · line 1634

Quoted 2 lines
drivers/staging/r8188eu/hal/odm.c +1 -1
-       if ((pregpriv->wifi_spec == 1))/*  (pmlmeinfo->HT_enable == 0)) */
+       if (pregpriv->wifi_spec == 1)

^ was the comment you removed important?

I doubt it, it looks like commented out code, which presumably means it did not work. I have cleaned up dead comments like this at the same time as cleaning up warnings in the past without any issues.

Quoted 2 lines

If not:

Thanks for the review!

Cheers,
Nathan

Quoted 19 lines
Quoted 19 lines
drivers/staging/r8188eu/hal/odm.c +0 -0
                 goto dm_CheckEdcaTurbo_EXIT;
 
         if (pmlmeinfo->assoc_AP_vendor >=  HT_IOT_PEER_MAX)
drivers/staging/r8188eu/hal/usb_halinit.c +1 -2
index d985894c0f30..ec7badfd72aa 100644
@@ -1300,7 +1300,7 @@ static void hw_var_set_opmode(struct adapter *Adapter, u8 variable, u8 *val)
                 StopTxBeacon(Adapter);
 
                 rtw_write8(Adapter, REG_BCN_CTRL, 0x19);/* disable atim wnd */
-       } else if ((mode == _HW_STATE_ADHOC_)) {
+       } else if (mode == _HW_STATE_ADHOC_) {
                 ResumeTxBeacon(Adapter);
                 rtw_write8(Adapter, REG_BCN_CTRL, 0x1a);
         } else if (mode == _HW_STATE_AP_) {
--

2.33.0.rc0

#5 lore raw

PATCH 3/4 staging: r8188eu: Remove self assignment in get_rx_power_val_by_reg()

In reply to Nathan Chancellor

Clang warns:

drivers/staging/r8188eu/hal/rtl8188e_rf6052.c:344:13: warning:
explicitly assigning value of variable of type 'u32' (aka 'unsigned int') to itself [-Wself-assign]

                        writeVal = writeVal;
                        ~~~~~~~~ ^ ~~~~~~~~

1 warning generated.

Remove this branch as it is useless.

---
 drivers/staging/r8188eu/hal/rtl8188e_rf6052.c | 2 --
 1 file changed, 2 deletions(-)
drivers/staging/r8188eu/hal/rtl8188e_rf6052.c +0 -2
index 335b120ce603..77889dc05858 100644
@@ -340,8 +340,6 @@ static void get_rx_power_val_by_reg(struct adapter *Adapter, u8 Channel,
 		/*  This mechanism is only applied when Driver-Highpower-Mechanism is OFF. */
 		if (pdmpriv->DynamicTxHighPowerLvl == TxHighPwrLevel_BT1)
 			writeVal = writeVal - 0x06060606;
-		else if (pdmpriv->DynamicTxHighPowerLvl == TxHighPwrLevel_BT2)
-			writeVal = writeVal;
 		*(pOutWriteVal+rf) = writeVal;
 	}
 }
Signature · 1 line
2.33.0.rc0
Reviewed-by on v1 3/4
#6 lore raw

Re: [PATCH 3/4] staging: r8188eu: Remove self assignment in get_rx_power_val_by_reg()

Quoted 22 lines · Nathan Chancellor

Clang warns:

drivers/staging/r8188eu/hal/rtl8188e_rf6052.c:344:13: warning:
explicitly assigning value of variable of type 'u32' (aka 'unsigned int') to itself [-Wself-assign]

                        writeVal = writeVal;
                        ~~~~~~~~ ^ ~~~~~~~~

1 warning generated.

Remove this branch as it is useless.

---
 drivers/staging/r8188eu/hal/rtl8188e_rf6052.c | 2 --
 1 file changed, 2 deletions(-)
drivers/staging/r8188eu/hal/rtl8188e_rf6052.c +0 -0
index 335b120ce603..77889dc05858 100644

drivers/staging/r8188eu/hal/rtl8188e_rf6052.c static void get_rx_power_val_by_reg(struct adapter *Adapter, u8 Channel, · line 344

Quoted 5 lines
drivers/staging/r8188eu/hal/rtl8188e_rf6052.c +0 -2
@@ -340,8 +340,6 @@ static void get_rx_power_val_by_reg(struct adapter *Adapter, u8 Channel,
                /*  This mechanism is only applied when Driver-Highpower-Mechanism is OFF. */
                if (pdmpriv->DynamicTxHighPowerLvl == TxHighPwrLevel_BT1)
                        writeVal = writeVal - 0x06060606;
-               else if (pdmpriv->DynamicTxHighPowerLvl == TxHighPwrLevel_BT2)
-                       writeVal = writeVal;

Hopefully the author didn't mean to subtract another magic constant like they did above. This also eliminates the last use of TxHighPwrLevel_BT2, but it's probably ok to keep the define in drivers/staging/r8188eu/include/odm.h.

Quoted 5 lines
drivers/staging/r8188eu/hal/rtl8188e_rf6052.c +0 -1
                *(pOutWriteVal+rf) = writeVal;
        }
 }
--

2.33.0.rc0

Signature · 2 lines
Thanks,
~Nick Desaulniers
#7 lore raw

Re: [PATCH 3/4] staging: r8188eu: Remove self assignment in get_rx_power_val_by_reg()

Quoted 18 lines · Nick Desaulniers' via Clang Built Linux
Quoted 17 lines · Nathan Chancellor

Clang warns:

drivers/staging/r8188eu/hal/rtl8188e_rf6052.c:344:13: warning:
explicitly assigning value of variable of type 'u32' (aka 'unsigned int') to itself [-Wself-assign]

                         writeVal = writeVal;
                         ~~~~~~~~ ^ ~~~~~~~~

1 warning generated.

Remove this branch as it is useless.

---
  drivers/staging/r8188eu/hal/rtl8188e_rf6052.c | 2 --
  1 file changed, 2 deletions(-)
Quoted 15 lines

drivers/staging/r8188eu/hal/rtl8188e_rf6052.c static void get_rx_power_val_by_reg(struct adapter *Adapter, u8 Channel, · line 344

Quoted 10 lines
drivers/staging/r8188eu/hal/rtl8188e_rf6052.c +0 -2
index 335b120ce603..77889dc05858 100644
@@ -340,8 +340,6 @@ static void get_rx_power_val_by_reg(struct adapter *Adapter, u8 Channel,
                 /*  This mechanism is only applied when Driver-Highpower-Mechanism is OFF. */
                 if (pdmpriv->DynamicTxHighPowerLvl == TxHighPwrLevel_BT1)
                         writeVal = writeVal - 0x06060606;
-               else if (pdmpriv->DynamicTxHighPowerLvl == TxHighPwrLevel_BT2)
-                       writeVal = writeVal;

Hopefully the author didn't mean to subtract another magic constant like they did above. This also eliminates the last use of TxHighPwrLevel_BT2, but it's probably ok to keep the define in drivers/staging/r8188eu/include/odm.h.

For what it's worth, it seems that the DynamicTxHighPowerLvl member is only ever assigned TxHighPwrLevel_Normal so the first if statement is probably dead code too but I wanted to clean up the clang warnings, not rewrite the driver, since I could do that for hours upon hours :)

Quoted 1 line

Thanks for the review!

Cheers,
Nathan

Quoted 7 lines
Quoted 5 lines
drivers/staging/r8188eu/hal/rtl8188e_rf6052.c +0 -1
                 *(pOutWriteVal+rf) = writeVal;
         }
  }
--

2.33.0.rc0

#8 lore raw

PATCH 4/4 staging: r8188eu: Remove pointless NULL check in rtw_check_join_candidate()

In reply to Nathan Chancellor

Clang warns:

drivers/staging/r8188eu/core/rtw_mlme.c:1629:28: warning: address of array 'pmlmepriv->assoc_ssid.Ssid' will always evaluate to 'true' [-Wpointer-bool-conversion]

        if (pmlmepriv->assoc_ssid.Ssid && pmlmepriv->assoc_ssid.SsidLength) {
            ~~~~~~~~~~~~~~~~~~~~~~^~~~ ~~

1 warning generated.

Ssid is an array not at the beginning of a struct so its address cannot be NULL so remove the check.

---
 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 fa4df4a3a2df..16d8f7317897 100644
@@ -1626,7 +1626,7 @@ static int rtw_check_join_candidate(struct mlme_priv *pmlmepriv
 	}
 
 	/* check ssid, if needed */
-	if (pmlmepriv->assoc_ssid.Ssid && pmlmepriv->assoc_ssid.SsidLength) {
+	if (pmlmepriv->assoc_ssid.SsidLength) {
 		if (competitor->network.Ssid.SsidLength != pmlmepriv->assoc_ssid.SsidLength ||
 		    memcmp(competitor->network.Ssid.Ssid, pmlmepriv->assoc_ssid.Ssid, pmlmepriv->assoc_ssid.SsidLength))
 			goto exit;
Signature · 1 line
2.33.0.rc0
#9 lore raw
In reply to Nathan Chancellor
Quoted 3 lines · Nathan Chancellor

This series cleans up all of the clang warnings that I noticed with x86_64 allmodconfig on the current staging-next. This has been build tested with both clang and gcc with x86_64 allmodconfig.

Can you rebase on my staging-testing branch and resend this series? There is a lot of churn in this driver right now, and your series does not apply anymore.

thanks,

greg k-h

#10 lore raw

v2 cover staging: r8188eu: Fix clang warnings

In reply to Nathan Chancellor

This series cleans up all of the clang warnings that I noticed with x86_64 allmodconfig on the current staging-testing. This has been build tested with both clang and gcc with x86_64 allmodconfig.

v1 -> v2:

* Rebase on staging-testing and fix conflict in patch 2.

* Drop patch 1 as it has already been fixed with commit 1c10f2b95cc1 ("staging: r8188eu: Remove all calls to _rtw_spinlock_free()") and follow-ups.

* Pick up Nick's reviewed-by tag for patches 1 and 2.

Nathan Chancellor (3):
staging: r8188eu: Remove unnecessary parentheses
staging: r8188eu: Remove self assignment in get_rx_power_val_by_reg()
staging: r8188eu: Remove pointless NULL check in
rtw_check_join_candidate()

 drivers/staging/r8188eu/core/rtw_mlme.c       | 2 +-
 drivers/staging/r8188eu/core/rtw_pwrctrl.c    | 2 +-
 drivers/staging/r8188eu/core/rtw_security.c   | 4 ++--
 drivers/staging/r8188eu/core/rtw_wlan_util.c  | 2 +-
 drivers/staging/r8188eu/hal/odm.c             | 2 +-
 drivers/staging/r8188eu/hal/rtl8188e_rf6052.c | 2 --
 drivers/staging/r8188eu/hal/usb_halinit.c     | 2 +-
 7 files changed, 7 insertions(+), 9 deletions(-)

base-commit: d48401b8609ff19db0f461759ac6b5210cd81288

Signature · 1 line
2.33.0.rc0
#11 lore raw

PATCH v2 1/3 staging: r8188eu: Remove unnecessary parentheses

Clang warns several times across the driver along the lines of:

drivers/staging/r8188eu/core/rtw_pwrctrl.c:222:21: warning: equality comparison with extraneous parentheses [-Wparentheses-equality]

        if ((pwrpriv->rpwm == pslv)) {
             ~~~~~~~~~~~~~~^~~~~~~

drivers/staging/r8188eu/core/rtw_pwrctrl.c:222:21: note: remove extraneous parentheses around the comparison to silence this warning
if ((pwrpriv->rpwm == pslv)) {
~ ^ ~
drivers/staging/r8188eu/core/rtw_pwrctrl.c:222:21: note: use '=' to turn this equality comparison into an assignment

        if ((pwrpriv->rpwm == pslv)) {
                           ^~
                           =

1 warning generated.

The compilers have agreed that single parentheses are used for equality and double parentheses are used for assignment within control flow statements such as if and while so remove them in these places to fix the warning.

---
 drivers/staging/r8188eu/core/rtw_pwrctrl.c   | 2 +-
 drivers/staging/r8188eu/core/rtw_security.c  | 4 ++--
 drivers/staging/r8188eu/core/rtw_wlan_util.c | 2 +-
 drivers/staging/r8188eu/hal/odm.c            | 2 +-
 drivers/staging/r8188eu/hal/usb_halinit.c    | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)
drivers/staging/r8188eu/core/rtw_pwrctrl.c +1 -1
index 2c029f85d63d..967c9623e7d1 100644
@@ -218,7 +218,7 @@ void rtw_set_rpwm(struct adapter *padapter, u8 pslv)
 			pslv = PS_STATE_S3;
 	}
 
-	if ((pwrpriv->rpwm == pslv))
+	if (pwrpriv->rpwm == pslv)
 		return;
 
 	if ((padapter->bSurpriseRemoved) ||
drivers/staging/r8188eu/core/rtw_security.c +2 -2
index df8107a0f5f2..08a5a1975d11 100644
@@ -1209,7 +1209,7 @@ u32	rtw_aes_encrypt(struct adapter *padapter, u8 *pxmitframe)
 	pframe = ((struct xmit_frame *)pxmitframe)->buf_addr + hw_hdr_offset;
 
 	/* 4 start to encrypt each fragment */
-	if ((pattrib->encrypt == _AES_)) {
+	if (pattrib->encrypt == _AES_) {
 		if (pattrib->psta)
 			stainfo = pattrib->psta;
 		else
@@ -1452,7 +1452,7 @@ u32	rtw_aes_decrypt(struct adapter *padapter, u8 *precvframe)
 
 	pframe = (unsigned char *)((struct recv_frame *)precvframe)->rx_data;
 	/* 4 start to encrypt each fragment */
-	if ((prxattrib->encrypt == _AES_)) {
+	if (prxattrib->encrypt == _AES_) {
 		stainfo = rtw_get_stainfo(&padapter->stapriv, &prxattrib->ta[0]);
 		if (stainfo) {
 			RT_TRACE(_module_rtl871x_security_c_, _drv_err_, ("rtw_aes_decrypt: stainfo!= NULL!!!\n"));
drivers/staging/r8188eu/core/rtw_wlan_util.c +1 -1
index 5a9a824dcbfd..21a3d0868214 100644
@@ -1283,7 +1283,7 @@ int support_short_GI(struct adapter *padapter, struct HT_caps_element *pHT_caps)
 	if (!(pmlmeinfo->HT_enable))
 		return _FAIL;
 
-	if ((pmlmeinfo->assoc_AP_vendor == HT_IOT_PEER_RALINK))
+	if (pmlmeinfo->assoc_AP_vendor == HT_IOT_PEER_RALINK)
 		return _FAIL;
 
 	bit_offset = (pmlmeext->cur_bwmode & HT_CHANNEL_WIDTH_40) ? 6 : 5;
drivers/staging/r8188eu/hal/odm.c +1 -1
index 08ec1e18b3f0..0deeb21c8006 100644
@@ -1630,7 +1630,7 @@ void odm_EdcaTurboCheckCE(struct odm_dm_struct *pDM_Odm)
 	struct mlme_ext_priv	*pmlmeext = &(Adapter->mlmeextpriv);
 	struct mlme_ext_info	*pmlmeinfo = &(pmlmeext->mlmext_info);
 
-	if ((pregpriv->wifi_spec == 1))/*  (pmlmeinfo->HT_enable == 0)) */
+	if (pregpriv->wifi_spec == 1)
 		goto dm_CheckEdcaTurbo_EXIT;
 
 	if (pmlmeinfo->assoc_AP_vendor >=  HT_IOT_PEER_MAX)
drivers/staging/r8188eu/hal/usb_halinit.c +1 -1
index b214b7b1e9de..c2d9c0359e64 100644
@@ -1277,7 +1277,7 @@ static void hw_var_set_opmode(struct adapter *Adapter, u8 variable, u8 *val)
 		StopTxBeacon(Adapter);
 
 		rtw_write8(Adapter, REG_BCN_CTRL, 0x19);/* disable atim wnd */
-	} else if ((mode == _HW_STATE_ADHOC_)) {
+	} else if (mode == _HW_STATE_ADHOC_) {
 		ResumeTxBeacon(Adapter);
 		rtw_write8(Adapter, REG_BCN_CTRL, 0x1a);
 	} else if (mode == _HW_STATE_AP_) {
Signature · 1 line
2.33.0.rc0
#12 lore raw

PATCH v2 2/3 staging: r8188eu: Remove self assignment in get_rx_power_val_by_reg()

In reply to Nathan Chancellor

Clang warns:

drivers/staging/r8188eu/hal/rtl8188e_rf6052.c:344:13: warning:
explicitly assigning value of variable of type 'u32' (aka 'unsigned int') to itself [-Wself-assign]

                        writeVal = writeVal;
                        ~~~~~~~~ ^ ~~~~~~~~

1 warning generated.

Remove this branch as it is useless.

---
 drivers/staging/r8188eu/hal/rtl8188e_rf6052.c | 2 --
 1 file changed, 2 deletions(-)
drivers/staging/r8188eu/hal/rtl8188e_rf6052.c +0 -2
index 335b120ce603..77889dc05858 100644
@@ -340,8 +340,6 @@ static void get_rx_power_val_by_reg(struct adapter *Adapter, u8 Channel,
 		/*  This mechanism is only applied when Driver-Highpower-Mechanism is OFF. */
 		if (pdmpriv->DynamicTxHighPowerLvl == TxHighPwrLevel_BT1)
 			writeVal = writeVal - 0x06060606;
-		else if (pdmpriv->DynamicTxHighPowerLvl == TxHighPwrLevel_BT2)
-			writeVal = writeVal;
 		*(pOutWriteVal+rf) = writeVal;
 	}
 }
Signature · 1 line
2.33.0.rc0
#13 lore raw

PATCH v2 3/3 staging: r8188eu: Remove pointless NULL check in rtw_check_join_candidate()

In reply to Nathan Chancellor

Clang warns:

drivers/staging/r8188eu/core/rtw_mlme.c:1629:28: warning: address of array 'pmlmepriv->assoc_ssid.Ssid' will always evaluate to 'true' [-Wpointer-bool-conversion]

        if (pmlmepriv->assoc_ssid.Ssid && pmlmepriv->assoc_ssid.SsidLength) {
            ~~~~~~~~~~~~~~~~~~~~~~^~~~ ~~

1 warning generated.

Ssid is an array not at the beginning of a struct so its address cannot be NULL so remove the check.

---
 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 e3d5a721d25c..95b952871e67 100644
@@ -1622,7 +1622,7 @@ static int rtw_check_join_candidate(struct mlme_priv *pmlmepriv
 	}
 
 	/* check ssid, if needed */
-	if (pmlmepriv->assoc_ssid.Ssid && pmlmepriv->assoc_ssid.SsidLength) {
+	if (pmlmepriv->assoc_ssid.SsidLength) {
 		if (competitor->network.Ssid.SsidLength != pmlmepriv->assoc_ssid.SsidLength ||
 		    memcmp(competitor->network.Ssid.Ssid, pmlmepriv->assoc_ssid.Ssid, pmlmepriv->assoc_ssid.SsidLength))
 			goto exit;
Signature · 1 line
2.33.0.rc0
Reviewed-by on v2 3/3
#14 lore raw

Re: [PATCH v2 3/3] staging: r8188eu: Remove pointless NULL check in rtw_check_join_candidate()

Quoted 23 lines · Nathan Chancellor

Clang warns:

drivers/staging/r8188eu/core/rtw_mlme.c:1629:28: warning: address of array 'pmlmepriv->assoc_ssid.Ssid' will always evaluate to 'true' [-Wpointer-bool-conversion]

        if (pmlmepriv->assoc_ssid.Ssid && pmlmepriv->assoc_ssid.SsidLength) {
            ~~~~~~~~~~~~~~~~~~~~~~^~~~ ~~

1 warning generated.

Ssid is an array not at the beginning of a struct so its address cannot be NULL so remove the check.

---
 drivers/staging/r8188eu/core/rtw_mlme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/r8188eu/core/rtw_mlme.c +0 -0
index e3d5a721d25c..95b952871e67 100644

drivers/staging/r8188eu/core/rtw_mlme.c static int rtw_check_join_candidate(struct mlme_priv *pmlmepriv · line 1625

Quoted 5 lines
drivers/staging/r8188eu/core/rtw_mlme.c +1 -1
@@ -1622,7 +1622,7 @@ static int rtw_check_join_candidate(struct mlme_priv *pmlmepriv
        }
 
        /* check ssid, if needed */
-       if (pmlmepriv->assoc_ssid.Ssid && pmlmepriv->assoc_ssid.SsidLength) {
+       if (pmlmepriv->assoc_ssid.SsidLength) {

Perhaps they meant to check pmlmepriv->assoc_ssid.Ssid[0] but the length should probably reflect that anyways.

Quoted 6 lines
drivers/staging/r8188eu/core/rtw_mlme.c +0 -1
                if (competitor->network.Ssid.SsidLength != pmlmepriv->assoc_ssid.SsidLength ||
                    memcmp(competitor->network.Ssid.Ssid, pmlmepriv->assoc_ssid.Ssid, pmlmepriv->assoc_ssid.SsidLength))
                        goto exit;
--

2.33.0.rc0

Signature · 2 lines
Thanks,
~Nick Desaulniers