2019-05-15 17:47:08

by Hariprasad Kelam

[permalink] [raw]
Subject: [PATCH] staging: rtl8723bs: core: rtw_recv: fix warning Comparison to NULL

fix below warning reported by checkpatch

CHECK: Comparison to NULL could be written
"!precvpriv->pallocated_frame_buf"
CHECK: Comparison to NULL could be written "padapter"

Signed-off-by: Hariprasad Kelam <[email protected]>
---
drivers/staging/rtl8723bs/core/rtw_recv.c | 48 +++++++++++++++----------------
1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c b/drivers/staging/rtl8723bs/core/rtw_recv.c
index b543e97..b01dae5 100644
--- a/drivers/staging/rtl8723bs/core/rtw_recv.c
+++ b/drivers/staging/rtl8723bs/core/rtw_recv.c
@@ -50,7 +50,7 @@ sint _rtw_init_recv_priv(struct recv_priv *precvpriv, struct adapter *padapter)

precvpriv->pallocated_frame_buf = vzalloc(NR_RECVFRAME * sizeof(union recv_frame) + RXFRAME_ALIGN_SZ);

- if (precvpriv->pallocated_frame_buf == NULL) {
+ if (!precvpriv->pallocated_frame_buf) {
res = _FAIL;
goto exit;
}
@@ -122,7 +122,7 @@ union recv_frame *_rtw_alloc_recvframe(struct __queue *pfree_recv_queue)

list_del_init(&precvframe->u.hdr.list);
padapter = precvframe->u.hdr.adapter;
- if (padapter != NULL) {
+ if (padapter) {
precvpriv = &padapter->recvpriv;
if (pfree_recv_queue == &precvpriv->free_recv_queue)
precvpriv->free_recvframe_cnt--;
@@ -160,7 +160,7 @@ int rtw_free_recvframe(union recv_frame *precvframe, struct __queue *pfree_recv_

list_add_tail(&(precvframe->u.hdr.list), get_list_head(pfree_recv_queue));

- if (padapter != NULL) {
+ if (padapter) {
if (pfree_recv_queue == &precvpriv->free_recv_queue)
precvpriv->free_recvframe_cnt++;
}
@@ -183,7 +183,7 @@ sint _rtw_enqueue_recvframe(union recv_frame *precvframe, struct __queue *queue)

list_add_tail(&(precvframe->u.hdr.list), get_list_head(queue));

- if (padapter != NULL)
+ if (padapter)
if (queue == &precvpriv->free_recv_queue)
precvpriv->free_recvframe_cnt++;

@@ -334,7 +334,7 @@ sint recvframe_chkmic(struct adapter *adapter, union recv_frame *precvframe)
prxattrib->ra[0], prxattrib->ra[1], prxattrib->ra[2], prxattrib->ra[3], prxattrib->ra[4], prxattrib->ra[5]));

/* calculate mic code */
- if (stainfo != NULL) {
+ if (stainfo) {
if (IS_MCAST(prxattrib->ra)) {
/* mickey =&psecuritypriv->dot118021XGrprxmickey.skey[0]; */
/* iv = precvframe->u.hdr.rx_data+prxattrib->hdrlen; */
@@ -570,7 +570,7 @@ union recv_frame *portctrl(struct adapter *adapter, union recv_frame *precv_fram
RT_TRACE(_module_rtl871x_recv_c_, _drv_info_, ("########portctrl:adapter->securitypriv.dot11AuthAlgrthm =%d\n", adapter->securitypriv.dot11AuthAlgrthm));

if (auth_alg == 2) {
- if ((psta != NULL) && (psta->ieee8021x_blocked)) {
+ if ((psta) && (psta->ieee8021x_blocked)) {
__be16 be_tmp;

/* blocked */
@@ -859,7 +859,7 @@ sint sta2sta_data_frame(
else
*psta = rtw_get_stainfo(pstapriv, sta_addr); /* get ap_info */

- if (*psta == NULL) {
+ if (!*psta) {
RT_TRACE(_module_rtl871x_recv_c_, _drv_err_, ("can't get psta under sta2sta_data_frame ; drop pkt\n"));
ret = _FAIL;
goto exit;
@@ -942,7 +942,7 @@ sint ap2sta_data_frame(
else
*psta = rtw_get_stainfo(pstapriv, pattrib->bssid); /* get ap_info */

- if (*psta == NULL) {
+ if (!*psta) {
RT_TRACE(_module_rtl871x_recv_c_, _drv_err_, ("ap2sta: can't get psta under STATION_MODE ; drop pkt\n"));
#ifdef DBG_RX_DROP_FRAME
DBG_871X("DBG_RX_DROP_FRAME %s can't get psta under STATION_MODE ; drop pkt\n", __func__);
@@ -974,7 +974,7 @@ sint ap2sta_data_frame(


*psta = rtw_get_stainfo(pstapriv, pattrib->bssid); /* get sta_info */
- if (*psta == NULL) {
+ if (!*psta) {
RT_TRACE(_module_rtl871x_recv_c_, _drv_err_, ("can't get psta under MP_MODE ; drop pkt\n"));
#ifdef DBG_RX_DROP_FRAME
DBG_871X("DBG_RX_DROP_FRAME %s can't get psta under WIFI_MP_STATE ; drop pkt\n", __func__);
@@ -991,7 +991,7 @@ sint ap2sta_data_frame(
} else {
if (!memcmp(myhwaddr, pattrib->dst, ETH_ALEN) && (!bmcast)) {
*psta = rtw_get_stainfo(pstapriv, pattrib->bssid); /* get sta_info */
- if (*psta == NULL) {
+ if (!*psta) {

/* for AP multicast issue , modify by yiwei */
static unsigned long send_issue_deauth_time;
@@ -1042,7 +1042,7 @@ sint sta2ap_data_frame(
}

*psta = rtw_get_stainfo(pstapriv, pattrib->src);
- if (*psta == NULL) {
+ if (!*psta == NULL) {
RT_TRACE(_module_rtl871x_recv_c_, _drv_err_, ("can't get psta under AP_MODE; drop pkt\n"));
DBG_871X("issue_deauth to sta =" MAC_FMT " for the reason(7)\n", MAC_ARG(pattrib->src));

@@ -1099,7 +1099,7 @@ sint validate_recv_ctrl_frame(struct adapter *padapter, union recv_frame *precv_
return _FAIL;

psta = rtw_get_stainfo(pstapriv, GetAddr2Ptr(pframe));
- if (psta == NULL)
+ if (!psta)
return _FAIL;

/* for rx pkt statistics */
@@ -1226,7 +1226,7 @@ sint validate_recv_mgnt_frame(struct adapter *padapter, union recv_frame *precv_
RT_TRACE(_module_rtl871x_recv_c_, _drv_info_, ("+validate_recv_mgnt_frame\n"));

precv_frame = recvframe_chk_defrag(padapter, precv_frame);
- if (precv_frame == NULL) {
+ if (!precv_frame) {
RT_TRACE(_module_rtl871x_recv_c_, _drv_notice_, ("%s: fragment packet\n", __func__));
return _SUCCESS;
}
@@ -1274,7 +1274,7 @@ sint validate_recv_data_frame(struct adapter *adapter, union recv_frame *precv_f
psa = get_sa(ptr);
pbssid = get_hdr_bssid(ptr);

- if (pbssid == NULL) {
+ if (!pbssid) {
#ifdef DBG_RX_DROP_FRAME
DBG_871X("DBG_RX_DROP_FRAME %s pbssid == NULL\n", __func__);
#endif
@@ -1329,7 +1329,7 @@ sint validate_recv_data_frame(struct adapter *adapter, union recv_frame *precv_f
}


- if (psta == NULL) {
+ if (!psta) {
RT_TRACE(_module_rtl871x_recv_c_, _drv_err_, (" after to_fr_ds_chk; psta == NULL\n"));
#ifdef DBG_RX_DROP_FRAME
DBG_871X("DBG_RX_DROP_FRAME %s psta == NULL\n", __func__);
@@ -1426,7 +1426,7 @@ static sint validate_80211w_mgmt(struct adapter *adapter, union recv_frame *prec
/* actual management data frame body */
data_len = pattrib->pkt_len - pattrib->hdrlen - pattrib->iv_len - pattrib->icv_len;
mgmt_DATA = rtw_zmalloc(data_len);
- if (mgmt_DATA == NULL) {
+ if (!mgmt_DATA) {
DBG_871X("%s mgmt allocate fail !!!!!!!!!\n", __func__);
goto validate_80211w_fail;
}
@@ -1812,7 +1812,7 @@ union recv_frame *recvframe_chk_defrag(struct adapter *padapter, union recv_fram

psta_addr = pfhdr->attrib.ta;
psta = rtw_get_stainfo(pstapriv, psta_addr);
- if (psta == NULL) {
+ if (!psta) {
u8 type = GetFrameType(pfhdr->rx_data);
if (type != WIFI_DATA_TYPE) {
psta = rtw_get_bcmc_stainfo(padapter);
@@ -1828,7 +1828,7 @@ union recv_frame *recvframe_chk_defrag(struct adapter *padapter, union recv_fram
if (ismfrag == 1) {
/* 0~(n-1) fragment frame */
/* enqueue to defraf_g */
- if (pdefrag_q != NULL) {
+ if (pdefrag_q) {
if (fragnum == 0)
/* the first fragment */
if (!list_empty(&pdefrag_q->queue))
@@ -1859,7 +1859,7 @@ union recv_frame *recvframe_chk_defrag(struct adapter *padapter, union recv_fram
if ((ismfrag == 0) && (fragnum != 0)) {
/* the last fragment frame */
/* enqueue the last fragment */
- if (pdefrag_q != NULL) {
+ if (pdefrag_q) {
/* spin_lock(&pdefrag_q->lock); */
phead = get_list_head(pdefrag_q);
list_add_tail(&pfhdr->list, phead);
@@ -1880,7 +1880,7 @@ union recv_frame *recvframe_chk_defrag(struct adapter *padapter, union recv_fram
}


- if ((prtnframe != NULL) && (prtnframe->u.hdr.attrib.privacy)) {
+ if ((prtnframe) && (prtnframe->u.hdr.attrib.privacy)) {
/* after defrag we must check tkip mic code */
if (recvframe_chkmic(padapter, prtnframe) == _FAIL) {
RT_TRACE(_module_rtl871x_recv_c_, _drv_err_, ("recvframe_chkmic(padapter, prtnframe) == _FAIL\n"));
@@ -1924,7 +1924,7 @@ static int amsdu_to_msdu(struct adapter *padapter, union recv_frame *prframe)
}

sub_pkt = rtw_os_alloc_msdu_pkt(prframe, nSubframe_Length, pdata);
- if (sub_pkt == NULL) {
+ if (!sub_pkt) {
DBG_871X("%s(): allocate sub packet fail !!!\n", __func__);
break;
}
@@ -2453,7 +2453,7 @@ static int recv_func_posthandle(struct adapter *padapter, union recv_frame *prfr
DBG_COUNTER(padapter->rx_logs.core_rx_post);

prframe = decryptor(padapter, prframe);
- if (prframe == NULL) {
+ if (!prframe) {
RT_TRACE(_module_rtl871x_recv_c_, _drv_err_, ("decryptor: drop pkt\n"));
#ifdef DBG_RX_DROP_FRAME
DBG_871X("DBG_RX_DROP_FRAME %s decryptor: drop pkt\n", __func__);
@@ -2464,7 +2464,7 @@ static int recv_func_posthandle(struct adapter *padapter, union recv_frame *prfr
}

prframe = recvframe_chk_defrag(padapter, prframe);
- if (prframe == NULL) {
+ if (!prframe) {
RT_TRACE(_module_rtl871x_recv_c_, _drv_err_, ("recvframe_chk_defrag: drop pkt\n"));
#ifdef DBG_RX_DROP_FRAME
DBG_871X("DBG_RX_DROP_FRAME %s recvframe_chk_defrag: drop pkt\n", __func__);
@@ -2474,7 +2474,7 @@ static int recv_func_posthandle(struct adapter *padapter, union recv_frame *prfr
}

prframe = portctrl(padapter, prframe);
- if (prframe == NULL) {
+ if (!prframe) {
RT_TRACE(_module_rtl871x_recv_c_, _drv_err_, ("portctrl: drop pkt\n"));
#ifdef DBG_RX_DROP_FRAME
DBG_871X("DBG_RX_DROP_FRAME %s portctrl: drop pkt\n", __func__);
--
2.7.4


2019-05-16 08:03:25

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] staging: rtl8723bs: core: rtw_recv: fix warning Comparison to NULL

On Wed, May 15, 2019 at 11:15:36PM +0530, Hariprasad Kelam wrote:
> @@ -1042,7 +1042,7 @@ sint sta2ap_data_frame(
> }
>
> *psta = rtw_get_stainfo(pstapriv, pattrib->src);
> - if (*psta == NULL) {
> + if (!*psta == NULL) {
^^^^^^^^^^^^^^
It's surprising that this didn't cause some kind of warning somewhere...

> RT_TRACE(_module_rtl871x_recv_c_, _drv_err_, ("can't get psta under AP_MODE; drop pkt\n"));
> DBG_871X("issue_deauth to sta =" MAC_FMT " for the reason(7)\n", MAC_ARG(pattrib->src));
>

regards,
dan carpenter

2019-05-16 19:06:00

by Hariprasad Kelam

[permalink] [raw]
Subject: Re: [PATCH] staging: rtl8723bs: core: rtw_recv: fix warning Comparison to NULL

On Thu, May 16, 2019 at 11:00:56AM +0300, Dan Carpenter wrote:
> On Wed, May 15, 2019 at 11:15:36PM +0530, Hariprasad Kelam wrote:
> > @@ -1042,7 +1042,7 @@ sint sta2ap_data_frame(
> > }
> >
> > *psta = rtw_get_stainfo(pstapriv, pattrib->src);
> > - if (*psta == NULL) {
> > + if (!*psta == NULL) {
> ^^^^^^^^^^^^^^
> It's surprising that this didn't cause some kind of warning somewhere...

Thanks for pointing out this error. Here my intention is to write
if(!*psta)
but somehow i missed it .

Will resend this patch after correcting the same.Like below

> - if (*psta == NULL) {
> > + if (!*psta) {


Do let me know if the above propose change is fine or not.
>
> > RT_TRACE(_module_rtl871x_recv_c_, _drv_err_, ("can't get psta under AP_MODE; drop pkt\n"));
> > DBG_871X("issue_deauth to sta =" MAC_FMT " for the reason(7)\n", MAC_ARG(pattrib->src));
> >
>
> regards,
> dan carpenter

2019-05-16 19:26:01

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] staging: rtl8723bs: core: rtw_recv: fix warning Comparison to NULL

On Thu, 2019-05-16 at 23:55 +0530, Hariprasad Kelam wrote:
> On Thu, May 16, 2019 at 11:00:56AM +0300, Dan Carpenter wrote:
> > On Wed, May 15, 2019 at 11:15:36PM +0530, Hariprasad Kelam wrote:
> > > @@ -1042,7 +1042,7 @@ sint sta2ap_data_frame(
> > > }
> > >
> > > *psta = rtw_get_stainfo(pstapriv, pattrib->src);
> > > - if (*psta == NULL) {
> > > + if (!*psta == NULL) {
> > ^^^^^^^^^^^^^^
> > It's surprising that this didn't cause some kind of warning somewhere...
>
> Thanks for pointing out this error. Here my intention is to write
> if(!*psta)
> but somehow i missed it .
>
> Will resend this patch after correcting the same.Like below
>
> > - if (*psta == NULL) {
> > > + if (!*psta) {

You could run the coccinelle spatch file for bool
comparisons on the files instead. It's much less
error prone.

$ spatch --sp-file scripts/coccinelle/misc/boolconv.cocci --in-place drivers/staging/rtl8723bs/

Or you could use a patch to checkpatch like below and then use

$ git ls-files drivers/staging/rtl8723bs | \
xargs ./scripts/checkpatch.pl -f --fix-inplace --types=bool_comparison

but that definitely would not be as good as the coccinelle
tool use as coccinelle is much better at parsing expressions.

---
scripts/checkpatch.pl | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1c421ac42b07..fe83aa0b1f97 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6407,11 +6407,13 @@ sub process {
$op = "";
}

- CHK("BOOL_COMPARISON",
- "Using comparison to $otype is error prone\n" . $herecurr);
-
+ if (CHK("BOOL_COMPARISON",
+ "Using comparison to $otype is error prone\n" . $herecurr) &&
+ $fix) {
+ $fixed[$fixlinenr] =~ s/\b(?:true|false|$Lval)\s*(?:==|\!=)\s*(?:true|false|$Lval)\b/${op}${arg}/;
+ }
## maybe suggesting a correct construct would better
-## "Using comparison to $otype is error prone. Perhaps use '${lead}${op}${arg}${trail}'\n" . $herecurr);
+## "Using comparison to $otype is error prone. Perhaps use '${lead}${op}${arg}${trail}'\n" . $herecurr);

}
}