staging: r8188eu: Fix clang warnings
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
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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
PATCH 2/4 staging: r8188eu: Remove unnecessary parentheses
In reply to Nathan ChancellorClang 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
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 assignmentif ((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.
Signed-off-by Nathan Chancellor <[email protected]>--- 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@@ -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@@ -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@@ -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
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
-0goto dm_CheckEdcaTurbo_EXIT;if (pmlmeinfo->assoc_AP_vendor >=HT_IOT_PEER_MAX )drivers/staging/r8188eu/hal/usb_halinit.c +1
-2@@ -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
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 assignmentif ((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.
Signed-off-by Nathan Chancellor <[email protected]>--- 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@@ -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@@ -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@@ -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@@ -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:
Reviewed-by Nick Desaulniers <[email protected]>
Thanks for the review!
Cheers,
Nathan
Quoted 19 lines
Quoted 19 lines
drivers/staging/r8188eu/hal/odm.c +0
-0goto dm_CheckEdcaTurbo_EXIT;if (pmlmeinfo->assoc_AP_vendor >=HT_IOT_PEER_MAX )drivers/staging/r8188eu/hal/usb_halinit.c +1
-2@@ -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
PATCH 3/4 staging: r8188eu: Remove self assignment in get_rx_power_val_by_reg()
In reply to Nathan ChancellorClang 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
@@ -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
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.
Signed-off-by Nathan Chancellor <[email protected]>--- drivers/staging/r8188eu/hal/rtl8188e_rf6052.c | 2 -- 1 file changed, 2 deletions(-)drivers/staging/r8188eu/hal/rtl8188e_rf6052.c +0
-0
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
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.
Signed-off-by Nathan Chancellor <[email protected]>--- 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@@ -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 indrivers/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
Reviewed-by Nick Desaulniers <[email protected]>
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
PATCH 4/4 staging: r8188eu: Remove pointless NULL check in rtw_check_join_candidate()
In reply to Nathan ChancellorClang 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
@@ -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
Greg Kroah-Hartman
maintainerQuoted 3 lines · Nathan Chancellor
This series cleans up all of the clang warnings that I noticed with
x86_64allmodconfig on the current staging-next. This has been build tested with both clang and gcc withx86_64allmodconfig.
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
v2 cover staging: r8188eu: Fix clang warnings
In reply to Nathan ChancellorThis 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
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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
PATCH v2 2/3 staging: r8188eu: Remove self assignment in get_rx_power_val_by_reg()
In reply to Nathan ChancellorClang 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
@@ -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
PATCH v2 3/3 staging: r8188eu: Remove pointless NULL check in rtw_check_join_candidate()
In reply to Nathan ChancellorClang 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
@@ -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
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.
Signed-off-by Nathan Chancellor <[email protected]>--- drivers/staging/r8188eu/core/rtw_mlme.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/staging/r8188eu/core/rtw_mlme.c +0
-0
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
-1if (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