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

[Dan Carpenter <dan.carpenter@linaro.org>]

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@gmail.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