1. Patchew
  2. linux
  3. View series

[PATCH] staging: rtl8723bs: Reduce indentation in rtw_joinbss_event_prehandle

Jose A. Perez de Azpillaga posted 1 patch 2 weeks, 4 days ago 
drivers/staging/rtl8723bs/core/rtw_mlme.c | 158 ++++++++++++----------
1 file changed, 83 insertions(+), 75 deletions(-)
[PATCH] staging: rtl8723bs: Reduce indentation in rtw_joinbss_event_prehandle
Posted by Jose A. Perez de Azpillaga 2 weeks, 4 days ago 
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 | 158 ++++++++++++----------
 1 file changed, 83 insertions(+), 75 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c b/drivers/staging/rtl8723bs/core/rtw_mlme.c
index 2adc98c955fd..09344997730f 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme.c
@@ -1176,83 +1176,20 @@ 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;
-					}
-				}
-
-			} 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;
-				}
-			}
-
-			/* 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;
-			}
-
-			/* 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;
-				}
-			}
-
-			/* 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);
-
-			spin_unlock_bh(&pmlmepriv->lock);
-			/* s5. Cancel assoc_timer */
-			timer_delete_sync(&pmlmepriv->assoc_timer);
-			spin_lock_bh(&pmlmepriv->lock);
-		} else {
-			spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
-		}
-	} else if (pnetwork->join_res == -4) {
+	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 */
+		goto ignore_joinbss_callback;
+	}
+
+	if (pnetwork->join_res <= 0) {
+#ifdef REJOIN
+		int res = _FAIL;
 
-		#ifdef REJOIN
-		res = _FAIL;
 		if (retry < 2)
 			res = rtw_select_and_join_from_scanned_queue(pmlmepriv);
 
@@ -1260,21 +1197,92 @@ void rtw_joinbss_event_prehandle(struct adapter *adapter, u8 *pbuf)
 			/* extend time of assoc_timer */
 			_set_timer(&pmlmepriv->assoc_timer, MAX_JOIN_TIMEOUT);
 			retry++;
-		} else if (res == 2) {/* there is no need to wait for join */
+		} else if (res == 2) { /* there is no need to wait for join */
 			_clr_fwstate_(pmlmepriv, _FW_UNDER_LINKING);
 			rtw_indicate_connect(adapter);
 		} else {
-		#endif
-
+#endif
 			_set_timer(&pmlmepriv->assoc_timer, 1);
 			_clr_fwstate_(pmlmepriv, _FW_UNDER_LINKING);
 
-		#ifdef REJOIN
+#ifdef REJOIN
 			retry = 0;
 		}
-		#endif
+#endif
+		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);
+
+			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;
+			}
+		}
+	} 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;
+		}
+	}
+
+	/* 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;
+	}
+
+	rtw_joinbss_update_network(adapter, ptarget_wlan, pnetwork);
+
+	/* s3. find ptarget_wlan & 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;
+		}
 	}
 
+	/* 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);
-- 
2.53.0
Re: [PATCH] staging: rtl8723bs: Reduce indentation in rtw_joinbss_event_prehandle
Posted by Dan Carpenter 2 weeks, 4 days ago 
On Wed, Mar 18, 2026 at 11:55:44PM +0100, Jose A. Perez de Azpillaga wrote:
> +	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 */
> +		goto ignore_joinbss_callback;
> +	}
> +
> +	if (pnetwork->join_res <= 0) {
> +#ifdef REJOIN

REJOIN is never defined.  Just delete this code in a separate patch
at the start.

> +		int res = _FAIL;
>  
> -		#ifdef REJOIN
> -		res = _FAIL;
>  		if (retry < 2)
>  			res = rtw_select_and_join_from_scanned_queue(pmlmepriv);
>  
> @@ -1260,21 +1197,92 @@ void rtw_joinbss_event_prehandle(struct adapter *adapter, u8 *pbuf)
>  			/* extend time of assoc_timer */
>  			_set_timer(&pmlmepriv->assoc_timer, MAX_JOIN_TIMEOUT);
>  			retry++;
> -		} else if (res == 2) {/* there is no need to wait for join */
> +		} else if (res == 2) { /* there is no need to wait for join */

Don't do unrelated changes.

The truth is I haven't reviewed this patch properly.  I see you have
made other changes unrelated to indenting.  Don't do things like:

-                                       ptarget_wlan = rtw_find_network(&pmlmepriv->scanned_queue, cur_network->network.mac_address);
+                       ptarget_wlan = rtw_find_network(&pmlmepriv->scanned_queue,
+                                                       cur_network->network.mac_address);

Those make it harder for me to review and they're against the rules.

regards,
dan carpenter
Re: [PATCH] staging: rtl8723bs: Reduce indentation in rtw_joinbss_event_prehandle
Posted by Luka Gejak 2 weeks, 4 days ago 
Hi Jose,

Thanks for this patch! Refactoring these deeply nested if-statements 
makes the mlme flow much easier to read and maintain.

Functionally, the logic and lock handling look correct. Great catch on 
removing the redundant spin_lock_bh(&pmlmepriv->lock) before the 
function returns. That is a nice optimization of the original flow.

I have two minor notes for a v2:

1. Subject Line: The standard practice for staging is to use a 
lowercase letter after the prefix. It should be: "staging: rtl8723bs: 
reduce indentation..." instead of "Reduce".

2. Comment Typo: In the refactored step s3:
> +	/* s3. find ptarget_wlan & update ptarget_sta after update
> +	 * cur_network only for station mode
> +	 */
The code here is calling rtw_joinbss_update_stainfo(), so the original 
comment "find ptarget_sta" was actually correct. Since ptarget_wlan was 
already found in s1, you might want to revert that word to avoid
confusion.

Best regards,
Luka Gejak

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.