Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <2014e0fc141d73d907c781ca31c822c96d3a7c47.camel@perches.com>
Subject: Re: [PATCH 14/16] staging: rtl8723bs: remove all RT_TRACE logs in
 core/rtw_wlan_util.c
From:   Joe Perches <joe@perches.com>
To:     Fabio Aiuto <fabioaiuto83@gmail.com>, gregkh@linuxfoundation.org
Cc:     dan.carpenter@oracle.com, linux-staging@lists.linux.dev,
        linux-kernel@vger.kernel.org
Date:   Fri, 02 Apr 2021 03:37:57 -0700
In-Reply-To: <34bf6cdc7bdf5c64d47cb3525897970644de7052.1617356821.git.fabioaiuto83@gmail.com>
References: <cover.1617356821.git.fabioaiuto83@gmail.com>
         <34bf6cdc7bdf5c64d47cb3525897970644de7052.1617356821.git.fabioaiuto83@gmail.com>
Content-Type: text/plain; charset="ISO-8859-1"
User-Agent: Evolution 3.38.1-1 
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

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@gmail.com>
> ---
> ?.../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.