Re: [PATCH v3 2/5] staging: rtl8723bs: rtw_mlme: fix lines exceeding 100 columns

Dan Carpenter
2026-04-27T09:11:02Z

On Mon, Apr 27, 2026 at 01:54:59AM +0300, Salman Alghamdi wrote:
> Wrap long lines and extract local variables to bring all lines
> within the 100 column limit.
>
> Signed-off-by: Salman Alghamdi <me@cipherat.com>
> ---

This patch is too big and too complicated. If you send a patch that
only adds newlines, then I have automated ways to review that but
when you're adding variables and change code to use min_t() then
it's hard to review.

> drivers/staging/rtl8723bs/core/rtw_mlme.c | 284 +++++++++++++++-------
> 1 file changed, 201 insertions(+), 83 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c b/drivers/staging/rtl8723bs/core/rtw_mlme.c
> index 268f294528e6..cdc631464565 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_mlme.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_mlme.c
> @@ -52,7 +52,10 @@ int rtw_init_mlme_priv(struct adapter *padapter)
> pmlmepriv->pscanned = NULL;
> pmlmepriv->fw_state = WIFI_STATION_STATE; /* Must sync with rtw_wdev_alloc() */
> pmlmepriv->cur_network.network.infrastructure_mode = Ndis802_11AutoUnknown;
> - pmlmepriv->scan_mode = SCAN_ACTIVE;/* 1: active, 0: passive. Maybe someday we should rename this varable to "active_mode" (Jeff) */
> + /* 1: active, 0: passive. Maybe someday we should rename
> + * this varable to "active_mode" (Jeff)
> + */
> + pmlmepriv->scan_mode = SCAN_ACTIVE;
>
> spin_lock_init(&pmlmepriv->lock);
> INIT_LIST_HEAD(&pmlmepriv->free_bss_pool.queue);
> @@ -125,7 +128,8 @@ void rtw_free_mlme_priv_ie_data(struct mlme_priv *pmlmepriv)
> rtw_free_mlme_ie_data(&pmlmepriv->p2p_beacon_ie, &pmlmepriv->p2p_beacon_ie_len);
> rtw_free_mlme_ie_data(&pmlmepriv->p2p_probe_req_ie, &pmlmepriv->p2p_probe_req_ie_len);
> rtw_free_mlme_ie_data(&pmlmepriv->p2p_probe_resp_ie, &pmlmepriv->p2p_probe_resp_ie_len);
> - rtw_free_mlme_ie_data(&pmlmepriv->p2p_go_probe_resp_ie, &pmlmepriv->p2p_go_probe_resp_ie_len);
> + rtw_free_mlme_ie_data(&pmlmepriv->p2p_go_probe_resp_ie,
> + &pmlmepriv->p2p_go_probe_resp_ie_len);
> rtw_free_mlme_ie_data(&pmlmepriv->p2p_assoc_req_ie, &pmlmepriv->p2p_assoc_req_ie_len);
> }
>
> @@ -369,7 +373,8 @@ int is_same_network(struct wlan_bssid_ex *src, struct wlan_bssid_ex *dst, u8 fea
> (d_cap & WLAN_CAPABILITY_ESS));
> }
>
> -struct wlan_network *_rtw_find_same_network(struct __queue *scanned_queue, struct wlan_network *network)
> +struct wlan_network *_rtw_find_same_network(struct __queue *scanned_queue,
> + struct wlan_network *network)
> {
> struct list_head *phead, *plist;
> struct wlan_network *found = NULL;
> @@ -420,7 +425,8 @@ void update_network(struct wlan_bssid_ex *dst, struct wlan_bssid_ex *src,
> long rssi_final;
>
> /* The rule below is 1/5 for sample value, 4/5 for history value */
> - if (check_fwstate(&padapter->mlmepriv, _FW_LINKED) && is_same_network(&padapter->mlmepriv.cur_network.network, src, 0)) {
> + if (check_fwstate(&padapter->mlmepriv, _FW_LINKED) &&
> + is_same_network(&padapter->mlmepriv.cur_network.network, src, 0)) {
> /* Take the recvpriv's value for the connected AP*/
> ss_final = padapter->recvpriv.signal_strength;
> sq_final = padapter->recvpriv.signal_qual;
> @@ -431,11 +437,15 @@ void update_network(struct wlan_bssid_ex *dst, struct wlan_bssid_ex *src,
> rssi_final = rssi_ori;
> } else {
> if (sq_smp != 101) { /* from the right channel */
> - ss_final = ((u32)(src->phy_info.signal_strength) + (u32)(dst->phy_info.signal_strength) * 4) / 5;
> - sq_final = ((u32)(src->phy_info.signal_quality) + (u32)(dst->phy_info.signal_quality) * 4) / 5;
> + ss_final = ((u32)(src->phy_info.signal_strength) +
> + (u32)(dst->phy_info.signal_strength) * 4) / 5;
> + sq_final = ((u32)(src->phy_info.signal_quality) +
> + (u32)(dst->phy_info.signal_quality) * 4) / 5;
> rssi_final = (src->rssi + dst->rssi * 4) / 5;
> } else {
> - /* bss info not receiving from the right channel, use the original RX signal infos */
> + /* bss info not receiving from the right channel, use
> + * the original RX signal infos
> + */
> ss_final = dst->phy_info.signal_strength;
> sq_final = dst->phy_info.signal_quality;
> rssi_final = dst->rssi;
> @@ -462,13 +472,18 @@ static void update_current_network(struct adapter *adapter, struct wlan_bssid_ex
> &pmlmepriv->cur_network.network,
> &pmlmepriv->cur_network.network);
>
> - if (check_fwstate(pmlmepriv, _FW_LINKED) && (is_same_network(&pmlmepriv->cur_network.network, pnetwork, 0))) {
> + if (check_fwstate(pmlmepriv, _FW_LINKED) &&
> + (is_same_network(&pmlmepriv->cur_network.network, pnetwork, 0))) {
> + u32 ie_len;
> + u8 *ie;
> +
> update_network(&pmlmepriv->cur_network.network, pnetwork, adapter, true);
> - if (pmlmepriv->cur_network.network.ie_length < sizeof(struct ndis_802_11_fix_ie))
> + ie_len = pmlmepriv->cur_network.network.ie_length;
> + if (ie_len < sizeof(struct ndis_802_11_fix_ie))
> return;
>
> - rtw_update_protection(adapter, (pmlmepriv->cur_network.network.ies) + sizeof(struct ndis_802_11_fix_ie),
> - pmlmepriv->cur_network.network.ie_length - sizeof(struct ndis_802_11_fix_ie));
> + ie = pmlmepriv->cur_network.network.ies + sizeof(struct ndis_802_11_fix_ie);
> + rtw_update_protection(adapter, ie, ie_len - sizeof(struct ndis_802_11_fix_ie));
> }
> }
>
> @@ -604,7 +619,16 @@ static bool rtw_is_desired_network(struct adapter *adapter, struct wlan_network
> privacy = pnetwork->network.privacy;
>
> if (check_fwstate(pmlmepriv, WIFI_UNDER_WPS)) {
> - if (rtw_get_wps_ie(pnetwork->network.ies + _FIXED_IE_LENGTH_, pnetwork->network.ie_length - _FIXED_IE_LENGTH_, NULL, &wps_ielen))
> + u8 *ie_ptr;
> + u32 ie_len_val;

Why is this not call ie and ie_len like the earlier code?
Also the declarations should be in reverse Christmas tree
order.

long long variable_name;
medium name;
short n;

When you start doing things which are more complicated than
just adding newlines, then it gets controversial in ways you
don't expect. For example, we don't really use min_t() these
days.

> +
> + if (pnetwork->network.ie_length < _FIXED_IE_LENGTH_)
> + return false;

This is a behavior change so we can't merge it.

regards,
dan carpenter

> +
> + ie_ptr = pnetwork->network.ies + _FIXED_IE_LENGTH_;
> + ie_len_val = pnetwork->network.ie_length - _FIXED_IE_LENGTH_;
> +
> + if (rtw_get_wps_ie(ie_ptr, ie_len_val, NULL, &wps_ielen))
> return true;
> else
> return false;