Re: [PATCH v4 4/7] staging: rtl8723bs: rtw_mlme: extract local variables for long expressions

[Dan Carpenter <error27@gmail.com>]

On Mon, Apr 27, 2026 at 10:05:32PM +0300, Salman Alghamdi wrote:
> @@ -772,9 +783,14 @@ void rtw_surveydone_event_callback(struct adapter	*adapter, u8 *pbuf)
>  				rtw_indicate_connect(adapter);
>  			} else {
>  				if (rtw_to_roam(adapter) != 0) {
> -					if (rtw_dec_to_roam(adapter) == 0
> -						|| _SUCCESS != rtw_sitesurvey_cmd(adapter, &pmlmepriv->assoc_ssid, 1, NULL, 0)
> -					) {
> +					bool roam_failed = false;
> +
> +					if (rtw_dec_to_roam(adapter) == 0)
> +						roam_failed = true;
> +					else if (rtw_sitesurvey_cmd(adapter, &pmlmepriv->assoc_ssid,
> +								    1, NULL, 0) != _SUCCESS)
> +						roam_failed = true;
> +					if (roam_failed) {

Just write this as:
					if (!rtw_dec_to_roam(adapter) ||
					    rtw_sitesurvey_cmd(adapter, &pmlmepriv->assoc_ssid,
							       1, NULL, 0) != _SUCCESS) {

The "roam_failed" variable isn't beautiful and the if else makes the line
five characters longer than it would be otherwise.  Eventually we'll get
rid of the _SUCCESS but I like it to look ugly so we know which code to
change.

>  						rtw_set_to_roam(adapter, 0);
>  						rtw_free_assoc_resources(adapter, 1);
>  						rtw_indicate_disconnect(adapter);
> @@ -792,8 +808,10 @@ void rtw_surveydone_event_callback(struct adapter	*adapter, u8 *pbuf)
>  			if (check_fwstate(pmlmepriv, WIFI_STATION_STATE)
>  				&& check_fwstate(pmlmepriv, _FW_LINKED)) {
>  				if (rtw_select_roaming_candidate(pmlmepriv) == _SUCCESS) {
> -					receive_disconnect(adapter, pmlmepriv->cur_network.network.mac_address
> -						, WLAN_REASON_ACTIVE_ROAM);
> +					u8 *mac_addr = pmlmepriv->cur_network.network.mac_address;
> +
> +					receive_disconnect(adapter, mac_address,
> +							   WLAN_REASON_ACTIVE_ROAM);
>  				}
>  			}
>  		}
> @@ -1071,6 +1089,7 @@ static void rtw_joinbss_update_network(struct adapter *padapter,
>  {
>  	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
>  	struct wlan_network  *cur_network = &pmlmepriv->cur_network;
> +	u8 signal_strength = ptarget_wlan->network.phy_info.signal_strength;
>  
>  	/*  why not use ptarget_wlan?? */
>  	memcpy(&cur_network->network, &pnetwork->network, pnetwork->network.length);
> @@ -1082,12 +1101,12 @@ static void rtw_joinbss_update_network(struct adapter *padapter,
>  
>  	rtw_set_signal_stat_timer(&padapter->recvpriv);
>  
> -	padapter->recvpriv.signal_strength = ptarget_wlan->network.phy_info.signal_strength;
> +	padapter->recvpriv.signal_strength = signal_strength;
>  	padapter->recvpriv.signal_qual = ptarget_wlan->network.phy_info.signal_quality;
>  	/* the ptarget_wlan->network.rssi is raw data, we use
>  	 * ptarget_wlan->network.phy_info.signal_strength instead (has scaled)
>  	 */
> -	padapter->recvpriv.rssi = translate_percentage_to_dbm(ptarget_wlan->network.phy_info.signal_strength);
> +	padapter->recvpriv.rssi = translate_percentage_to_dbm(signal_strength);
>  
>  	rtw_set_signal_stat_timer(&padapter->recvpriv);
>  
> @@ -1742,6 +1761,7 @@ static int rtw_check_roaming_candidate(struct mlme_priv *mlme
>  {
>  	int updated = false;
>  	struct adapter *adapter = container_of(mlme, struct adapter, mlmepriv);
> +	long rssi_diff;
>  
>  	if (is_same_ess(&competitor->network, &mlme->cur_network.network) == false)
>  		goto exit;
> @@ -1759,7 +1779,8 @@ static int rtw_check_roaming_candidate(struct mlme_priv *mlme
>  	if (jiffies_to_msecs(jiffies - competitor->last_scanned) >= mlme->roam_scanr_exp_ms)
>  		goto exit;
>  
> -	if (competitor->network.rssi - mlme->cur_network_scanned->network.rssi < mlme->roam_rssi_diff_th)
> +	rssi_diff = competitor->network.rssi - mlme->cur_network_scanned->network.rssi;
> +	if (rssi_diff < mlme->roam_rssi_diff_th)
>  		goto exit;
>  
>  	if (*candidate && (*candidate)->network.rssi >= competitor->network.rssi)
> @@ -2133,7 +2154,7 @@ static void rtw_report_sec_ie(struct adapter *adapter, u8 authmode, u8 *sec_ie)
>  
>  		wrqu.data.length = p - buff;
>  
> -		wrqu.data.length = (wrqu.data.length < IW_CUSTOM_MAX) ? wrqu.data.length : IW_CUSTOM_MAX;
> +		wrqu.data.length = umin(wrqu.data.length, IW_CUSTOM_MAX);
>  
>  		kfree(buff);
>  	}
> @@ -2522,13 +2543,14 @@ void rtw_update_ht_cap(struct adapter *padapter, u8 *pie, uint ie_len, u8 channe
>  	    (le16_to_cpu(pmlmeinfo->HT_caps.u.HT_cap_element.HT_caps_info) &
>  	      BIT(1)) && (pmlmeinfo->HT_info.infos[0] & BIT(2))) {
>  		int i;
> +		u8 *mcs_rate = pmlmeinfo->HT_caps.u.HT_cap_element.MCS_rate;

Reverse Christmas tree.

regards,
dan carpenter

>  
>  		/* update the MCS set */
>  		for (i = 0; i < 16; i++)
> -			pmlmeinfo->HT_caps.u.HT_cap_element.MCS_rate[i] &= pmlmeext->default_supported_mcs_set[i];
> +			mcs_rate[i] &= pmlmeext->default_supported_mcs_set[i];
>  
>  		/* update the MCS rates */
> -		set_mcs_rate_by_mask(pmlmeinfo->HT_caps.u.HT_cap_element.MCS_rate, MCS_RATE_1R);
> +		set_mcs_rate_by_mask(mcs_rate, MCS_RATE_1R);
>  
>  		/* switch to the 40M Hz mode according to the AP */
>  		/* pmlmeext->cur_bwmode = CHANNEL_WIDTH_40; */
> -- 
> 2.54.0
>