linux-kernel - Re: [PATCH v3 1/2] staging: rtl8723bs: refactor nested loops to reduce indentation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aXSmDkzs4P79BZRW@stanley.mountain>
Date: Sat, 24 Jan 2026 13:59:26 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Michael Huang <tehsiu.huang@...il.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/2] staging: rtl8723bs: refactor nested loops to
 reduce indentation

On Fri, Jan 23, 2026 at 01:19:03PM -0800, Michael Huang wrote:
> Use guard clauses (continue statements) to flatten the logic in

Just say continue statements.  I saw "Use guard" and thought you
were going to use "guard statements" which is a different thing in
the kernel.

> rtw_mlme_ext.c. This improves code readability by reducing deep
> nesting levels.
> 
> Signed-off-by: Michael Huang <tehsiu.huang@...il.com>
> ---
>  drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 61 +++++++++++--------
>  1 file changed, 35 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> index fa1e3ad59254..0ab3629608ce 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> @@ -3682,32 +3682,31 @@ static void issue_action_BSSCoexistPacket(struct adapter *padapter)
>  
>  		spin_unlock_bh(&(pmlmepriv->scanned_queue.lock));
>  
> -
>  		for (i = 0; i < 8; i++) {
> -			if (ICS[i][0] == 1) {
> -				int j, k = 0;
> +			int j, k = 0;
>  
> -				InfoContent[k] = i;
> -				/* SET_BSS_INTOLERANT_ELE_REG_CLASS(InfoContent, i); */
> -				k++;
> +			if (ICS[i][0] != 1)
> +				continue;
>  
> -				for (j = 1; j <= 14; j++) {
> -					if (ICS[i][j] == 1) {
> -						if (k < 16) {
> -							InfoContent[k] = j; /* channel number */
> -							/* SET_BSS_INTOLERANT_ELE_CHANNEL(InfoContent+k, j); */
> -							k++;
> -						}
> -					}
> -				}
> +			InfoContent[k] = i;
> +			/* SET_BSS_INTOLERANT_ELE_REG_CLASS(InfoContent, i); */
> +			k++;
>  
> -				pframe = rtw_set_ie(pframe, WLAN_EID_BSS_INTOLERANT_CHL_REPORT, k, InfoContent, &(pattrib->pktlen));
> +			for (j = 1; j <= 14; j++) {
> +				if (ICS[i][j] != 1)
> +					continue;
>  
> +				if (k < 16) {
> +					InfoContent[k] = j; /* channel number */
> +					/* SET_BSS_INTOLERANT_ELE_CHANNEL(InfoContent+k, j); */
> +					k++;
> +				}
>  			}
>  
> +			pframe = rtw_set_ie(pframe,
> +					    WLAN_EID_BSS_INTOLERANT_CHL_REPORT,
> +					    k, InfoContent, &pattrib->pktlen);

This is unrelated.  Do it in a separate patch.

>  		}
> -
> -
>  	}
>  
>  
> @@ -3831,14 +3830,24 @@ void site_survey(struct adapter *padapter)
>  				int i;
>  
>  				for (i = 0; i < RTW_SSID_SCAN_AMOUNT; i++) {
> -					if (pmlmeext->sitesurvey_res.ssid[i].ssid_length) {
> -						/* IOT issue, When wifi_spec is not set, send one probe req without WPS IE. */
> -						if (padapter->registrypriv.wifi_spec)
> -							issue_probereq(padapter, &(pmlmeext->sitesurvey_res.ssid[i]), NULL);
> -						else
> -							issue_probereq_ex(padapter, &(pmlmeext->sitesurvey_res.ssid[i]), NULL, 0, 0, 0, 0);
> -						issue_probereq(padapter, &(pmlmeext->sitesurvey_res.ssid[i]), NULL);
> -					}
> +					if (!pmlmeext->sitesurvey_res.ssid[i].ssid_length)
> +						continue;
> +
> +					/* IOT issue, When wifi_spec is not set. */
> +					/* Send one probe req without WPS IE. */

You can flip the conditions around without rephrasing the comments.
Do that in a separate patch.

> +					if (padapter->registrypriv.wifi_spec)
> +						issue_probereq(padapter,
> +							       &pmlmeext->sitesurvey_res.ssid[i],
> +							       NULL);

All these reformating and removing extra parens makes the patch harder
to review.  I understand that checkpatch complains if you don't do it,
but it's unrelated to the change you are making so it needs to be done
in a separate step.  (I'm not your boss and I can't tell you what to
do.  If you don't want to do the second step you can also skip it.  The
point is don't do unrelated things in the same patch).

regards,
dan carpenter