[PATCH 0/4] staging: r8188eu: Fix clang warnings

[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.

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
--
2.33.0.rc0

[Nathan Chancellor]

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.

Signed-off-by: Nathan Chancellor <nat...@kernel.org>
---
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(-)

diff --git a/drivers/staging/r8188eu/core/rtw_cmd.c b/drivers/staging/r8188eu/core/rtw_cmd.c
index 4e17972ee66e..cb7d730286fe 100644
--- a/drivers/staging/r8188eu/core/rtw_cmd.c
+++ b/drivers/staging/r8188eu/core/rtw_cmd.c
@@ -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);

diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
index a6d62074289f..fa4df4a3a2df 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme.c
@@ -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));

}

diff --git a/drivers/staging/r8188eu/core/rtw_recv.c b/drivers/staging/r8188eu/core/rtw_recv.c
index 610cc699caf9..1889a7935c97 100644
--- a/drivers/staging/r8188eu/core/rtw_recv.c
+++ b/drivers/staging/r8188eu/core/rtw_recv.c
@@ -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) {
diff --git a/drivers/staging/r8188eu/core/rtw_sta_mgt.c b/drivers/staging/r8188eu/core/rtw_sta_mgt.c
index feaf39fddf7c..f51d2f78e292 100644
--- a/drivers/staging/r8188eu/core/rtw_sta_mgt.c
+++ b/drivers/staging/r8188eu/core/rtw_sta_mgt.c
@@ -127,81 +127,6 @@ inline struct sta_info *rtw_get_stainfo_by_offset(struct sta_priv *stapriv, int
return (struct sta_info *)(stapriv->pstainfo_buf + offset * sizeof(struct sta_info));
}

-void _rtw_free_sta_xmit_priv_lock(struct sta_xmit_priv *psta_xmitpriv);
-void _rtw_free_sta_xmit_priv_lock(struct sta_xmit_priv *psta_xmitpriv)
-{
-
- _rtw_spinlock_free(&psta_xmitpriv->lock);
-
- _rtw_spinlock_free(&(psta_xmitpriv->be_q.sta_pending.lock));
- _rtw_spinlock_free(&(psta_xmitpriv->bk_q.sta_pending.lock));
- _rtw_spinlock_free(&(psta_xmitpriv->vi_q.sta_pending.lock));
- _rtw_spinlock_free(&(psta_xmitpriv->vo_q.sta_pending.lock));
-
-}
-
-static void _rtw_free_sta_recv_priv_lock(struct sta_recv_priv *psta_recvpriv)
-{
-
- _rtw_spinlock_free(&psta_recvpriv->lock);
-
- _rtw_spinlock_free(&(psta_recvpriv->defrag_q.lock));
-
-}
-
-void rtw_mfree_stainfo(struct sta_info *psta);
-void rtw_mfree_stainfo(struct sta_info *psta)
-{
-
- if (&psta->lock != NULL)
- _rtw_spinlock_free(&psta->lock);
-
- _rtw_free_sta_xmit_priv_lock(&psta->sta_xmitpriv);
- _rtw_free_sta_recv_priv_lock(&psta->sta_recvpriv);
-
-}
-
-/* this function is used to free the memory of lock || sema for all stainfos */
-void rtw_mfree_all_stainfo(struct sta_priv *pstapriv);
-void rtw_mfree_all_stainfo(struct sta_priv *pstapriv)
-{
- struct list_head *plist, *phead;
- struct sta_info *psta = NULL;
-
- spin_lock_bh(&pstapriv->sta_hash_lock);
-
- phead = get_list_head(&pstapriv->free_sta_queue);
- plist = phead->next;
-
- while (phead != plist) {
- psta = container_of(plist, struct sta_info, list);
- plist = plist->next;
- }
-
- spin_unlock_bh(&pstapriv->sta_hash_lock);
-}
-
-static void rtw_mfree_sta_priv_lock(struct sta_priv *pstapriv)
-{
-#ifdef CONFIG_88EU_AP_MODE
- struct wlan_acl_pool *pacl_list = &pstapriv->acl_list;
-#endif
-
- rtw_mfree_all_stainfo(pstapriv); /* be done before free sta_hash_lock */
-
- _rtw_spinlock_free(&pstapriv->free_sta_queue.lock);
-
- _rtw_spinlock_free(&pstapriv->sta_hash_lock);
- _rtw_spinlock_free(&pstapriv->wakeup_q.lock);
- _rtw_spinlock_free(&pstapriv->sleep_q.lock);
-
-#ifdef CONFIG_88EU_AP_MODE
- _rtw_spinlock_free(&pstapriv->asoc_list_lock);
- _rtw_spinlock_free(&pstapriv->auth_list_lock);
- _rtw_spinlock_free(&pacl_list->acl_node_q.lock);
-#endif
-}
-
u32 _rtw_free_sta_priv(struct sta_priv *pstapriv)
{
struct list_head *phead, *plist;
@@ -230,8 +155,6 @@ u32 _rtw_free_sta_priv(struct sta_priv *pstapriv)
spin_unlock_bh(&pstapriv->sta_hash_lock);
/*===============================*/

- rtw_mfree_sta_priv_lock(pstapriv);
-
if (pstapriv->pallocated_stainfo_buf)
rtw_vmfree(pstapriv->pallocated_stainfo_buf, sizeof(struct sta_info)*NUM_STA+4);
}
diff --git a/drivers/staging/r8188eu/core/rtw_xmit.c b/drivers/staging/r8188eu/core/rtw_xmit.c
index 4a5393184737..499bb8ce0290 100644
--- a/drivers/staging/r8188eu/core/rtw_xmit.c
+++ b/drivers/staging/r8188eu/core/rtw_xmit.c
@@ -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));
diff --git a/drivers/staging/r8188eu/include/osdep_service.h b/drivers/staging/r8188eu/include/osdep_service.h
index ac0bd5cd0f82..df7f293981e7 100644
--- a/drivers/staging/r8188eu/include/osdep_service.h
+++ b/drivers/staging/r8188eu/include/osdep_service.h
@@ -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);

diff --git a/drivers/staging/r8188eu/os_dep/os_intfs.c b/drivers/staging/r8188eu/os_dep/os_intfs.c
index 5f1ec883d6ed..935e35c82666 100644
--- a/drivers/staging/r8188eu/os_dep/os_intfs.c
+++ b/drivers/staging/r8188eu/os_dep/os_intfs.c
@@ -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);
diff --git a/drivers/staging/r8188eu/os_dep/osdep_service.c b/drivers/staging/r8188eu/os_dep/osdep_service.c
index 71d4fa25db86..a23f5e448bf4 100644
--- a/drivers/staging/r8188eu/os_dep/osdep_service.c
+++ b/drivers/staging/r8188eu/os_dep/osdep_service.c
@@ -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));
--
2.33.0.rc0

[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.

Signed-off-by: Nathan Chancellor <nat...@kernel.org>
---

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

diff --git a/drivers/staging/r8188eu/core/rtw_pwrctrl.c b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
index d67eeb045002..598c32d7eaa5 100644
--- a/drivers/staging/r8188eu/core/rtw_pwrctrl.c
+++ b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
@@ -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;
diff --git a/drivers/staging/r8188eu/core/rtw_security.c b/drivers/staging/r8188eu/core/rtw_security.c
index 2c1b9a6dcdf2..45fd8b1aeb59 100644
--- a/drivers/staging/r8188eu/core/rtw_security.c
+++ b/drivers/staging/r8188eu/core/rtw_security.c
@@ -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"));
diff --git a/drivers/staging/r8188eu/core/rtw_wlan_util.c b/drivers/staging/r8188eu/core/rtw_wlan_util.c
index 15edccef9f1d..4a8e52484cfd 100644
--- a/drivers/staging/r8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/r8188eu/core/rtw_wlan_util.c
@@ -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;
diff --git a/drivers/staging/r8188eu/hal/odm.c b/drivers/staging/r8188eu/hal/odm.c
index 0bc836311036..65a117408d50 100644
--- a/drivers/staging/r8188eu/hal/odm.c
+++ b/drivers/staging/r8188eu/hal/odm.c
@@ -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)
diff --git a/drivers/staging/r8188eu/hal/usb_halinit.c b/drivers/staging/r8188eu/hal/usb_halinit.c
index d985894c0f30..ec7badfd72aa 100644
--- a/drivers/staging/r8188eu/hal/usb_halinit.c
+++ b/drivers/staging/r8188eu/hal/usb_halinit.c
@@ -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

[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 <nat...@kernel.org>
---

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

diff --git a/drivers/staging/r8188eu/hal/rtl8188e_rf6052.c b/drivers/staging/r8188eu/hal/rtl8188e_rf6052.c
index 335b120ce603..77889dc05858 100644
--- a/drivers/staging/r8188eu/hal/rtl8188e_rf6052.c
+++ b/drivers/staging/r8188eu/hal/rtl8188e_rf6052.c
@@ -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;
}
}
--
2.33.0.rc0

[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 <nat...@kernel.org>
---

drivers/staging/r8188eu/core/rtw_mlme.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
index fa4df4a3a2df..16d8f7317897 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme.c
@@ -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;
--
2.33.0.rc0

[Nick Desaulniers]

^ was the comment you removed important?

If not:
Reviewed-by: Nick Desaulniers <ndesau...@google.com>

> goto dm_CheckEdcaTurbo_EXIT;
>
> if (pmlmeinfo->assoc_AP_vendor >= HT_IOT_PEER_MAX)
> diff --git a/drivers/staging/r8188eu/hal/usb_halinit.c b/drivers/staging/r8188eu/hal/usb_halinit.c
> index d985894c0f30..ec7badfd72aa 100644
> --- a/drivers/staging/r8188eu/hal/usb_halinit.c
> +++ b/drivers/staging/r8188eu/hal/usb_halinit.c
> @@ -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 clang-built-li...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20210803223609.1627280-3-nathan%40kernel.org.



--
Thanks,
~Nick Desaulniers

[Nick Desaulniers]

On Tue, Aug 3, 2021 at 3:36 PM Nathan Chancellor <nat...@kernel.org> wrote:
>

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.

Reviewed-by: Nick Desaulniers <ndesau...@google.com>

> *(pOutWriteVal+rf) = writeVal;
> }
> }
> --
> 2.33.0.rc0

--
Thanks,
~Nick Desaulniers

[Nathan Chancellor]

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.

> If not:
> Reviewed-by: Nick Desaulniers <ndesau...@google.com>

Thanks for the review!

Cheers,
Nathan

[Nathan Chancellor]

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

> Reviewed-by: Nick Desaulniers <ndesau...@google.com>

Thanks for the review!

Cheers,
Nathan

[Greg Kroah-Hartman]

On Tue, Aug 03, 2021 at 03:36:05PM -0700, Nathan Chancellor wrote:
> 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

[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
--
2.33.0.rc0

[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.

Signed-off-by: Nathan Chancellor <nat...@kernel.org>

Reviewed-by: Nick Desaulniers <ndesau...@google.com>
---

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

diff --git a/drivers/staging/r8188eu/core/rtw_pwrctrl.c b/drivers/staging/r8188eu/core/rtw_pwrctrl.c

index 2c029f85d63d..967c9623e7d1 100644
--- a/drivers/staging/r8188eu/core/rtw_pwrctrl.c
+++ b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
@@ -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) ||
diff --git a/drivers/staging/r8188eu/core/rtw_security.c b/drivers/staging/r8188eu/core/rtw_security.c
index df8107a0f5f2..08a5a1975d11 100644
--- a/drivers/staging/r8188eu/core/rtw_security.c
+++ b/drivers/staging/r8188eu/core/rtw_security.c
@@ -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"));
diff --git a/drivers/staging/r8188eu/core/rtw_wlan_util.c b/drivers/staging/r8188eu/core/rtw_wlan_util.c

index 5a9a824dcbfd..21a3d0868214 100644
--- a/drivers/staging/r8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/r8188eu/core/rtw_wlan_util.c
@@ -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;
diff --git a/drivers/staging/r8188eu/hal/odm.c b/drivers/staging/r8188eu/hal/odm.c

index 08ec1e18b3f0..0deeb21c8006 100644
--- a/drivers/staging/r8188eu/hal/odm.c
+++ b/drivers/staging/r8188eu/hal/odm.c
@@ -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)
diff --git a/drivers/staging/r8188eu/hal/usb_halinit.c b/drivers/staging/r8188eu/hal/usb_halinit.c

index b214b7b1e9de..c2d9c0359e64 100644
--- a/drivers/staging/r8188eu/hal/usb_halinit.c
+++ b/drivers/staging/r8188eu/hal/usb_halinit.c
@@ -1277,7 +1277,7 @@ static void hw_var_set_opmode(struct adapter *Adapter, u8 variable, u8 *val)

[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 <nat...@kernel.org>
Reviewed-by: Nick Desaulniers <ndesau...@google.com>
---

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

diff --git a/drivers/staging/r8188eu/hal/rtl8188e_rf6052.c b/drivers/staging/r8188eu/hal/rtl8188e_rf6052.c
index 335b120ce603..77889dc05858 100644
--- a/drivers/staging/r8188eu/hal/rtl8188e_rf6052.c
+++ b/drivers/staging/r8188eu/hal/rtl8188e_rf6052.c
@@ -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;

[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 <nat...@kernel.org>
---

drivers/staging/r8188eu/core/rtw_mlme.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c

index e3d5a721d25c..95b952871e67 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme.c
@@ -1622,7 +1622,7 @@ static int rtw_check_join_candidate(struct mlme_priv *pmlmepriv

[Nick Desaulniers]

On Thu, Aug 5, 2021 at 11:58 AM Nathan Chancellor <nat...@kernel.org> wrote:
>
> 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 <nat...@kernel.org>
> ---
> drivers/staging/r8188eu/core/rtw_mlme.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
> index e3d5a721d25c..95b952871e67 100644
> --- a/drivers/staging/r8188eu/core/rtw_mlme.c
> +++ b/drivers/staging/r8188eu/core/rtw_mlme.c
> @@ -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.

Reviewed-by: Nick Desaulniers <ndesau...@google.com>

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

--
Thanks,
~Nick Desaulniers