All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ethan Tidmore" <ethantidmore06@gmail.com>
To: "Nikolay Kulikov" <nikolayof23@gmail.com>, <gregkh@linuxfoundation.org>
Cc: <linux-staging@lists.linux.dev>
Subject: Re: [PATCH 1/5] staging: rtl8723bs: replace deeply nested if-else with switch-case
Date: Fri, 20 Mar 2026 12:30:31 -0500	[thread overview]
Message-ID: <DH7SDQSN6CJX.O6RII1OVBDQK@gmail.com> (raw)
In-Reply-To: <20260320122119.19563-2-nikolayof23@gmail.com>

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.

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

Thanks,

ET

  reply	other threads:[~2026-03-20 17:30 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-20 12:14 [PATCH 0/5] replace custom functions with kernel network Nikolay Kulikov
2026-03-20 12:14 ` [PATCH 1/5] staging: rtl8723bs: replace deeply nested if-else with switch-case Nikolay Kulikov
2026-03-20 17:30   ` Ethan Tidmore [this message]
2026-03-22  8:19     ` Nikolay Kulikov
2026-03-20 12:15 ` [PATCH 2/5] staging: rtl8723bs: move logical operators to end of previous line Nikolay Kulikov
2026-03-20 12:15 ` [PATCH 3/5] staging: rtl8723bs: remove custom is_zero_mac_addr() function Nikolay Kulikov
2026-03-20 12:15 ` [PATCH 4/5] staging: rtl8723bs: remove custom is_broadcast_mac_addr() function Nikolay Kulikov
2026-03-20 12:15 ` [PATCH 5/5] staging: rtl8723bs: remove custom is_multicast_mac_addr() function Nikolay Kulikov
2026-03-23 15:06 ` [PATCH v2 0/6] replace custom functions with kernel network functions Nikolay Kulikov
2026-03-23 15:06   ` [PATCH v2 1/6] staging: rtl8723bs: replace deeply nested if-else with switch-case Nikolay Kulikov
2026-03-23 15:06   ` [PATCH v2 2/6] staging: rtl8723bs: remove dead code in validate_recv_mgnt_frame() Nikolay Kulikov
2026-03-23 15:06   ` [PATCH v2 3/6] staging: rtl8723bs: move logical operators to end of previous line Nikolay Kulikov
2026-03-23 15:06   ` [PATCH v2 4/6] staging: rtl8723bs: remove custom is_zero_mac_addr() function Nikolay Kulikov
2026-03-23 15:06   ` [PATCH v2 5/6] staging: rtl8723bs: remove custom is_broadcast_mac_addr() function Nikolay Kulikov
2026-03-23 15:06   ` [PATCH v2 6/6] staging: rtl8723bs: remove custom is_multicast_mac_addr() function Nikolay Kulikov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DH7SDQSN6CJX.O6RII1OVBDQK@gmail.com \
    --to=ethantidmore06@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=nikolayof23@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.