Re: [PATCH 14/16] staging: rtl8723bs: remove all RT_TRACE logs in core/rtw_wlan_util.c

[Fabio Aiuto]

On Fri, Apr 02, 2021 at 03:37:57AM -0700, Joe Perches wrote:
> On Fri, 2021-04-02 at 12:01 +0200, Fabio Aiuto wrote:
> > remove all RT_TRACE logs
> > 
> > fix patch-related checkpatch issues
> > 
> > Signed-off-by: Fabio Aiuto <fabioaiuto83@xxxxxxxxx>
> > ---
> >  .../staging/rtl8723bs/core/rtw_wlan_util.c    | 26 +++++--------------
> >  1 file changed, 6 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
> []
> > @@ -1382,25 +1374,19 @@ int rtw_check_bcn_info(struct adapter *Adapter, u8 *pframe, u32 packet_len)
> >  	if (encryp_protocol == ENCRYP_PROTOCOL_WPA || encryp_protocol == ENCRYP_PROTOCOL_WPA2) {
> >  		pbuf = rtw_get_wpa_ie(&bssid->IEs[12], &wpa_ielen, bssid->IELength-12);
> >  		if (pbuf && (wpa_ielen > 0)) {
> > -			if (_SUCCESS == rtw_parse_wpa_ie(pbuf, wpa_ielen+2, &group_cipher, &pairwise_cipher, &is_8021x)) {
> > -				RT_TRACE(_module_rtl871x_mlme_c_, _drv_info_,
> > -						("%s pnetwork->pairwise_cipher: %d, group_cipher is %d, is_8021x is %d\n", __func__,
> > -						 pairwise_cipher, group_cipher, is_8021x));
> > -			}
> > +			if (rtw_parse_wpa_ie(pbuf, wpa_ielen + 2, &group_cipher,
> > +					     &pairwise_cipher, &is_8021x) == _SUCCESS)
> > +				;
> 
> 		This sort of if is a bit silly.
> 		Better would be to remove the test and just use:
> 
> 			rtw_parse_wpa_ie(pbuf, wpa_ielen + 2, &group_cipher,
> 					 &pairwise_cipher, &is_8021x);
> 
> >  		} else {
> >  			pbuf = rtw_get_wpa2_ie(&bssid->IEs[12], &wpa_ielen, bssid->IELength-12);
> >  
> > 
> >  			if (pbuf && (wpa_ielen > 0)) {
> > -				if (_SUCCESS == rtw_parse_wpa2_ie(pbuf, wpa_ielen+2, &group_cipher, &pairwise_cipher, &is_8021x)) {
> > -					RT_TRACE(_module_rtl871x_mlme_c_, _drv_info_,
> > -							("%s pnetwork->pairwise_cipher: %d, pnetwork->group_cipher is %d, is_802x is %d\n",
> > -							 __func__, pairwise_cipher, group_cipher, is_8021x));
> > -				}
> > +				if (rtw_parse_wpa2_ie(pbuf, wpa_ielen + 2, &group_cipher,
> > +						      &pairwise_cipher, &is_8021x) == _SUCCESS)
> > +					;
> 
> 				rtw_parse_wpa2_ie(pbuf, wpa_ielen + 2, &group_cipher,
> 						  &pairwise_cipher, &is_8021x);
> 
> etc...
> 
> Lastly, another suggestion would be to just submit a single patch
> removing _ALL_ the RT_TRACE uses not intermixing various other cleanups
> with the series and then do those other cleanups.
> 
> Using a coccinelle script like:
> 
> $ cat RT_TRACE.cocci
> @@
> expression a, b, c;
> @@
> 
> -	RT_TRACE(a, b, (c));
> 
> $ spatch -sp-file RT_TRACE.cocci drivers/staging/rtl8723bs/
> 
> And then clean up the various bits you think are inappropriately done.
> 
> 

thank you Joe, I tried with (RT_TRACE.cocci in parent folder)

user@host:~/src/git/kernels/staging$ spatch -sp-file ../RT_TRACE.cocci drivers/staging/rtl8723bs/
init_defs_builtins: /usr/local/bin/../lib/coccinelle/standard.h
0 files match

i don't know what's wrong... I run on a Debian Buster