Re: [PATCH 1/5] staging: rtl8723bs: replace deeply nested if-else with switch-case

[Nikolay Kulikov <nikolayof23@gmail.com>]

On Fri, Mar 20, 2026 at 12:30:31PM -0500, Ethan Tidmore wrote:
> On Fri Mar 20, 2026 at 7:14 AM CDT, Nikolay Kulikov wrote:
> > The main logic of the validate_recv_mgnt_frame() function is deeply
> > nested due to multiple if-else statements and additional block scope.
> > Fix this by replacing identical if-else with switch-case statements,
> > which will improve code readability and correct checkpatch.pl warnings
> > about line lengths.
> >
> > Signed-off-by: Nikolay Kulikov <nikolayof23@gmail.com>
> > ---
> >  drivers/staging/rtl8723bs/core/rtw_recv.c | 48 +++++++++++++----------
> >  1 file changed, 28 insertions(+), 20 deletions(-)
> >
> 
> ...
> 
> 
> >  	/* struct mlme_priv *pmlmepriv = &adapter->mlmepriv; */
> 
> Maybe create a patch to delete this commented out code since you're
> already here.
> 
> > +	struct sta_info *psta = NULL;
> 
> You don't have to declare this as NULL rtw_get_stainfo() returns NULL.
> 

I will do it in v2.


> >  
> >  	precv_frame = recvframe_chk_defrag(padapter, precv_frame);
> >  	if (!precv_frame)
> >  		return _SUCCESS;
> >  
> > -	{
> > -		/* for rx pkt statistics */
> > -		struct sta_info *psta = rtw_get_stainfo(&padapter->stapriv, GetAddr2Ptr(precv_frame->u.hdr.rx_data));
> > -
> > -		if (psta) {
> > -			psta->sta_stats.rx_mgnt_pkts++;
> > -			if (GetFrameSubType(precv_frame->u.hdr.rx_data) == WIFI_BEACON)
> > -				psta->sta_stats.rx_beacon_pkts++;
> > -			else if (GetFrameSubType(precv_frame->u.hdr.rx_data) == WIFI_PROBEREQ)
> > -				psta->sta_stats.rx_probereq_pkts++;
> > -			else if (GetFrameSubType(precv_frame->u.hdr.rx_data) == WIFI_PROBERSP) {
> > -				if (!memcmp(padapter->eeprompriv.mac_addr, GetAddr1Ptr(precv_frame->u.hdr.rx_data), ETH_ALEN))
> > -					psta->sta_stats.rx_probersp_pkts++;
> > -				else if (is_broadcast_mac_addr(GetAddr1Ptr(precv_frame->u.hdr.rx_data)) ||
> > -					 is_multicast_mac_addr(GetAddr1Ptr(precv_frame->u.hdr.rx_data)))
> > -					psta->sta_stats.rx_probersp_bm_pkts++;
> > -				else
> > -					psta->sta_stats.rx_probersp_uo_pkts++;
> > -			}
> > -		}
> > +	/* for rx pkt statistics */
> > +	psta = rtw_get_stainfo(&padapter->stapriv, GetAddr2Ptr(precv_frame->u.hdr.rx_data));
> > +	if (!psta)
> > +		goto exit;
> 
> Instead of adding a goto here, it'd be easier to read if this was just
> wrapped in an if statement.

According to the Kernel Coding style (Section 1), heavy indentation
should be avoided. If we wrap all of this in:
	if (psta) {
		...
	}
then there will be four tabs before the deepest lines, leaving little
room for the actual work, and the reader will have ro remember too
much context from all the conditions.


Thanks,
Nikolay