[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <2026011624-walnut-dinginess-e201@gregkh>
Date: Fri, 16 Jan 2026 14:05:51 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: William Hansen-Baird <william.hansen.baird@...il.com>
Cc: linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] staging: rtl8723bs: core/rtw_mlme_ext.c: flatten
control flow in OnAssocReq()
On Tue, Jan 13, 2026 at 04:14:32PM -0500, William Hansen-Baird wrote:
> Rewrite if-else construct with an early exit to reduce indentation,
> and make the execution clearer.
>
> Signed-off-by: William Hansen-Baird <william.hansen.baird@...il.com>
> ---
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 59 +++++++++----------
> 1 file changed, 29 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> index ac49bfbaa5bb..d3b395894d4b 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> @@ -1181,47 +1181,46 @@ unsigned int OnAssocReq(struct adapter *padapter, union recv_frame *precv_frame)
> p = pframe + WLAN_HDR_A3_LEN + ie_offset; ie_len = 0;
> for (;;) {
> p = rtw_get_ie(p, WLAN_EID_VENDOR_SPECIFIC, &ie_len, pkt_len - WLAN_HDR_A3_LEN - ie_offset);
> - if (p) {
> - if (!memcmp(p+2, WMM_IE, 6)) {
> + if (!p)
> + break;
>
> - pstat->flags |= WLAN_STA_WME;
> + if (!memcmp(p+2, WMM_IE, 6)) {
>
> - pstat->qos_option = 1;
> - pstat->qos_info = *(p+8);
> + pstat->flags |= WLAN_STA_WME;
>
> - pstat->max_sp_len = (pstat->qos_info>>5)&0x3;
> + pstat->qos_option = 1;
> + pstat->qos_info = *(p+8);
>
> - if ((pstat->qos_info&0xf) != 0xf)
> - pstat->has_legacy_ac = true;
> - else
> - pstat->has_legacy_ac = false;
> + pstat->max_sp_len = (pstat->qos_info>>5)&0x3;
>
> - if (pstat->qos_info&0xf) {
> - if (pstat->qos_info&BIT(0))
> - pstat->uapsd_vo = BIT(0)|BIT(1);
> - else
> - pstat->uapsd_vo = 0;
> + if ((pstat->qos_info&0xf) != 0xf)
> + pstat->has_legacy_ac = true;
> + else
> + pstat->has_legacy_ac = false;
>
> - if (pstat->qos_info&BIT(1))
> - pstat->uapsd_vi = BIT(0)|BIT(1);
> - else
> - pstat->uapsd_vi = 0;
> + if (pstat->qos_info&0xf) {
> + if (pstat->qos_info&BIT(0))
> + pstat->uapsd_vo = BIT(0)|BIT(1);
> + else
> + pstat->uapsd_vo = 0;
>
> - if (pstat->qos_info&BIT(2))
> - pstat->uapsd_bk = BIT(0)|BIT(1);
> - else
> - pstat->uapsd_bk = 0;
> + if (pstat->qos_info&BIT(1))
> + pstat->uapsd_vi = BIT(0)|BIT(1);
> + else
> + pstat->uapsd_vi = 0;
>
> - if (pstat->qos_info&BIT(3))
> - pstat->uapsd_be = BIT(0)|BIT(1);
> - else
> - pstat->uapsd_be = 0;
> + if (pstat->qos_info&BIT(2))
> + pstat->uapsd_bk = BIT(0)|BIT(1);
> + else
> + pstat->uapsd_bk = 0;
>
> - }
> + if (pstat->qos_info&BIT(3))
> + pstat->uapsd_be = BIT(0)|BIT(1);
> + else
> + pstat->uapsd_be = 0;
>
> - break;
> }
> - } else {
> +
> break;
If all ways through this loop ends up in a break, why is this a loop at
all?
thanks,
greg k-h
Powered by blists - more mailing lists