mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Luka Gejak <luka.gejak@linux.dev>
To: "Jose A. Perez de Azpillaga" <azpijr@gmail.com>,
	linux-staging@lists.linux.dev
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Michael Straube <straube.linux@gmail.com>,
	Hans de Goede <hansg@kernel.org>,
	Khushal Chitturi <khushalchitturi@gmail.com>,
	Dan Carpenter <dan.carpenter@linaro.org>,
	Ethan Tidmore <ethantidmore06@gmail.com>,
	Vivek BalachandharTN <vivek.balachandhar@gmail.com>,
	Artur Stupa <arthur.stupa@gmail.com>,
	Zhuoheng Li <lizhuoheng@kylinos.cn>,
	Nino Zhang <ninozhang001@gmail.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/2] staging: rtl8723bs: refactor rtw_joinbss_event_prehandle to reduce indentation
Date: Fri, 20 Mar 2026 09:05:21 +0100	[thread overview]
Message-ID: <EF78B3DD-16FB-42FA-A618-8E991093B1E3@linux.dev> (raw)
In-Reply-To: <20260319231344.569418-3-azpijr@gmail.com>

On March 20, 2026 12:13:35 AM GMT+01:00, "Jose A. Perez de Azpillaga" <azpijr@gmail.com> wrote:
>The rtw_joinbss_event_prehandle function has excessive indentation due
>to deeply nested if-statements.
>
>Refactor the function using early returns and guard clauses for the
>failure paths. This flattens the code and significantly improves
>readability.
>
>Signed-off-by: Jose A. Perez de Azpillaga <azpijr@gmail.com>
>---
> drivers/staging/rtl8723bs/core/rtw_mlme.c | 134 +++++++++++-----------
> 1 file changed, 69 insertions(+), 65 deletions(-)
>
>diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c b/drivers/staging/rtl8723bs/core/rtw_mlme.c
>index 805427353aa6..2a192c1dd038 100644
>--- a/drivers/staging/rtl8723bs/core/rtw_mlme.c
>+++ b/drivers/staging/rtl8723bs/core/rtw_mlme.c
>@@ -1175,84 +1175,88 @@ void rtw_joinbss_event_prehandle(struct adapter *adapter, u8 *pbuf)
> 	pmlmepriv->link_detect_info.traffic_transition_count = 0;
> 	pmlmepriv->link_detect_info.low_power_transition_count = 0;
> 
>-	if (pnetwork->join_res > 0) {
>-		spin_lock_bh(&pmlmepriv->scanned_queue.lock);
>-		retry = 0;
>-		if (check_fwstate(pmlmepriv, _FW_UNDER_LINKING)) {
>-			/* s1. find ptarget_wlan */
>-			if (check_fwstate(pmlmepriv, _FW_LINKED)) {
>-				if (the_same_macaddr) {
>-					ptarget_wlan = rtw_find_network(&pmlmepriv->scanned_queue, cur_network->network.mac_address);
>-				} else {
>-					pcur_wlan = rtw_find_network(&pmlmepriv->scanned_queue, cur_network->network.mac_address);
>-					if (pcur_wlan)
>-						pcur_wlan->fixed = false;
>-
>-					pcur_sta = rtw_get_stainfo(pstapriv, cur_network->network.mac_address);
>-					if (pcur_sta)
>-						rtw_free_stainfo(adapter,  pcur_sta);
>-
>-					ptarget_wlan = rtw_find_network(&pmlmepriv->scanned_queue, pnetwork->network.mac_address);
>-					if (check_fwstate(pmlmepriv, WIFI_STATION_STATE)) {
>-						if (ptarget_wlan)
>-							ptarget_wlan->fixed = true;
>-					}
>-				}
>+	if (pnetwork->join_res == -4) {
>+		rtw_reset_securitypriv(adapter);
>+		_set_timer(&pmlmepriv->assoc_timer, 1);
> 
>-			} else {
>-				ptarget_wlan = _rtw_find_same_network(&pmlmepriv->scanned_queue, pnetwork);
>-				if (check_fwstate(pmlmepriv, WIFI_STATION_STATE)) {
>-					if (ptarget_wlan)
>-						ptarget_wlan->fixed = true;
>-				}
>-			}
>+		if (check_fwstate(pmlmepriv, _FW_UNDER_LINKING))
>+			_clr_fwstate_(pmlmepriv, _FW_UNDER_LINKING);
> 
>-			/* s2. update cur_network */
>-			if (ptarget_wlan) {
>-				rtw_joinbss_update_network(adapter, ptarget_wlan, pnetwork);
>-			} else {
>-				netdev_dbg(adapter->pnetdev,
>-					   "Can't find ptarget_wlan when joinbss_event callback\n");
>-				spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
>-				goto ignore_joinbss_callback;
>-			}
>+		goto ignore_joinbss_callback;
>+	}
> 
>-			/* s3. find ptarget_sta & update ptarget_sta after update cur_network only for station mode */
>-			if (check_fwstate(pmlmepriv, WIFI_STATION_STATE)) {
>-				ptarget_sta = rtw_joinbss_update_stainfo(adapter, pnetwork);
>-				if (!ptarget_sta) {
>-					spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
>-					goto ignore_joinbss_callback;
>-				}
>-			}
>+	if (pnetwork->join_res <= 0) {
>+		_set_timer(&pmlmepriv->assoc_timer, 1);
>+		_clr_fwstate_(pmlmepriv, _FW_UNDER_LINKING);
>+		goto ignore_joinbss_callback;
>+	}
>+
>+	spin_lock_bh(&pmlmepriv->scanned_queue.lock);
>+	retry = 0;
>+
>+	if (!check_fwstate(pmlmepriv, _FW_UNDER_LINKING)) {
>+		spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
>+		goto ignore_joinbss_callback;
>+	}
>+
>+	/* s1. find ptarget_wlan */
>+	if (check_fwstate(pmlmepriv, _FW_LINKED)) {
>+		if (the_same_macaddr) {
>+			ptarget_wlan = rtw_find_network(&pmlmepriv->scanned_queue, cur_network->network.mac_address);
>+		} else {
>+			pcur_wlan = rtw_find_network(&pmlmepriv->scanned_queue, cur_network->network.mac_address);
>+			if (pcur_wlan)
>+				pcur_wlan->fixed = false;
>+
>+			pcur_sta = rtw_get_stainfo(pstapriv, cur_network->network.mac_address);
>+			if (pcur_sta)
>+				rtw_free_stainfo(adapter, pcur_sta);
> 
>-			/* s4. indicate connect */
>+			ptarget_wlan = rtw_find_network(&pmlmepriv->scanned_queue, pnetwork->network.mac_address);
> 			if (check_fwstate(pmlmepriv, WIFI_STATION_STATE)) {
>-				pmlmepriv->cur_network_scanned = ptarget_wlan;
>-				rtw_indicate_connect(adapter);
>+				if (ptarget_wlan)
>+					ptarget_wlan->fixed = true;
> 			}
>+		}
>+	} else {
>+		ptarget_wlan = _rtw_find_same_network(&pmlmepriv->scanned_queue, pnetwork);
>+		if (check_fwstate(pmlmepriv, WIFI_STATION_STATE)) {
>+			if (ptarget_wlan)
>+				ptarget_wlan->fixed = true;
>+		}
>+	}
> 
>-			spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
>+	/* s2. update cur_network */
>+	if (!ptarget_wlan) {
>+		netdev_dbg(adapter->pnetdev, "Can't find ptarget_wlan when joinbss_event callback\n");
>+		spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
>+		goto ignore_joinbss_callback;
>+	}
> 
>-			spin_unlock_bh(&pmlmepriv->lock);
>-			/* s5. Cancel assoc_timer */
>-			timer_delete_sync(&pmlmepriv->assoc_timer);
>-			spin_lock_bh(&pmlmepriv->lock);
>-		} else {
>+	rtw_joinbss_update_network(adapter, ptarget_wlan, pnetwork);
>+
>+	/* s3. find ptarget_sta & update ptarget_sta after update cur_network only for station mode */
>+	if (check_fwstate(pmlmepriv, WIFI_STATION_STATE)) {
>+		ptarget_sta = rtw_joinbss_update_stainfo(adapter, pnetwork);
>+		if (!ptarget_sta) {
> 			spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
>+			goto ignore_joinbss_callback;
> 		}
>-	} else if (pnetwork->join_res == -4) {
>-		rtw_reset_securitypriv(adapter);
>-		_set_timer(&pmlmepriv->assoc_timer, 1);
>-
>-		if (check_fwstate(pmlmepriv, _FW_UNDER_LINKING))
>-			_clr_fwstate_(pmlmepriv, _FW_UNDER_LINKING);
>+	}
> 
>-	} else {/* if join_res < 0 (join fails), then try again */
>-		_set_timer(&pmlmepriv->assoc_timer, 1);
>-		_clr_fwstate_(pmlmepriv, _FW_UNDER_LINKING);
>+	/* s4. indicate connect */
>+	if (check_fwstate(pmlmepriv, WIFI_STATION_STATE)) {
>+		pmlmepriv->cur_network_scanned = ptarget_wlan;
>+		rtw_indicate_connect(adapter);
> 	}
> 
>+	spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
>+
>+	/* s5. Cancel assoc_timer */
>+	spin_unlock_bh(&pmlmepriv->lock);
>+	timer_delete_sync(&pmlmepriv->assoc_timer);
>+	return;
>+
> ignore_joinbss_callback:
> 
> 	spin_unlock_bh(&pmlmepriv->lock);

One minor note: Since the REJOIN logic is now gone, the static u8 retry
 variable at line 1150 is now only assigned to 0 and never used. It 
doesn't break the build thanks to __maybe_unused, but it should be 
removed in a v3 of the series. Also include everyone from To and Cc for
 cover letter email as well.

  reply	other threads:[~2026-03-20  8:05 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260319231344.569418-1-azpijr@gmail.com>
2026-03-19 23:13 ` [PATCH v2 1/2] staging: rtl8723bs: remove dead REJOIN code Jose A. Perez de Azpillaga
2026-03-19 23:13 ` [PATCH v2 2/2] staging: rtl8723bs: refactor rtw_joinbss_event_prehandle to reduce indentation Jose A. Perez de Azpillaga
2026-03-20  8:05   ` Luka Gejak [this message]
2026-03-20  9:40   ` Dan Carpenter

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=EF78B3DD-16FB-42FA-A618-8E991093B1E3@linux.dev \
    --to=luka.gejak@linux.dev \
    --cc=arthur.stupa@gmail.com \
    --cc=azpijr@gmail.com \
    --cc=dan.carpenter@linaro.org \
    --cc=ethantidmore06@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hansg@kernel.org \
    --cc=khushalchitturi@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=lizhuoheng@kylinos.cn \
    --cc=ninozhang001@gmail.com \
    --cc=straube.linux@gmail.com \
    --cc=vivek.balachandhar@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®