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

Dan Carpenter
2026-04-28T09:10:44Z

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
>