From: Stefano Brivio <sbrivio@redhat.com>
To: Soumyajit Deb <debsoumyajit100@gmail.com>
Cc: gregkh@linuxfoundation.org, Larry.Finger@lwfinger.net,
outreachy-kernel@googlegroups.com
Subject: Re: [Outreachy kernel] [Outreachy] [PATCH 2/2] staging: rtl8188eu: Line over 80 characters
Date: Mon, 30 Mar 2020 20:17:01 +0200 [thread overview]
Message-ID: <20200330201701.719b8c0f@elisabeth> (raw)
In-Reply-To: <20200330082036.12928-3-debsoumyajit100@gmail.com>
On Mon, 30 Mar 2020 13:50:36 +0530
Soumyajit Deb <debsoumyajit100@gmail.com> wrote:
> Break various lines into multiple lines to respect the 80 character
> width limit.
> Reported by checkpatch.pl
>
> Signed-off-by: Soumyajit Deb <debsoumyajit100@gmail.com>
> ---
> drivers/staging/rtl8188eu/core/rtw_ap.c | 62 +++++++++++++++++--------
> 1 file changed, 42 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c
> index 5b2b4d1d56f6..94cfd45e3eaa 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_ap.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
> @@ -264,8 +264,10 @@ void expire_timeout_chk(struct adapter *padapter)
> list_del_init(&psta->asoc_list);
> pstapriv->asoc_list_cnt--;
>
> - DBG_88E("asoc expire %pM, state = 0x%x\n", (psta->hwaddr), psta->state);
> - updated = ap_free_sta(padapter, psta, true, WLAN_REASON_DEAUTH_LEAVING);
> + DBG_88E("asoc expire %pM, state = 0x%x\n",
> + (psta->hwaddr), psta->state);
> + updated = ap_free_sta(padapter, psta, true,
> + WLAN_REASON_DEAUTH_LEAVING);
> } else {
> /* TODO: Aging mechanism to digest frames in sleep_q to avoid running out of xmitframe */
> if (psta->sleepq_len > (NR_XMITFRAME / pstapriv->asoc_list_cnt) &&
> @@ -294,16 +296,20 @@ void expire_timeout_chk(struct adapter *padapter)
> for (i = 0; i < chk_alive_num; i++) {
> int ret = _FAIL;
>
> - psta = rtw_get_stainfo_by_offset(pstapriv, chk_alive_list[i]);
> + psta = rtw_get_stainfo_by_offset(pstapriv,
> + chk_alive_list[i]);
>
> if (psta->state & WIFI_SLEEP_STATE)
> - ret = issue_nulldata(padapter, psta->hwaddr, 0, 1, 50);
> + ret = issue_nulldata(padapter, psta->hwaddr, 0,
> + 1, 50);
> else
> - ret = issue_nulldata(padapter, psta->hwaddr, 0, 3, 50);
> + ret = issue_nulldata(padapter, psta->hwaddr, 0,
> + 3, 50);
For both clauses, here, you need curly brackets, checkpatch.pl should
report. Not because they are functionally needed, rather because they
are visually helpful to group the statements and avoid that a second
statement is inadvertently added without curly brackets.
>
> psta->keep_alive_trycnt++;
> if (ret == _SUCCESS) {
> - DBG_88E("asoc check, sta(%pM) is alive\n", (psta->hwaddr));
> + DBG_88E("asoc check, sta(%pM) is alive\n",
> + (psta->hwaddr));
Further cleanup (unrelated with this patch): excess parentheses.
> psta->expire_to = pstapriv->expire_to;
> psta->keep_alive_trycnt = 0;
> continue;
> @@ -315,11 +321,13 @@ void expire_timeout_chk(struct adapter *padapter)
>
> psta->keep_alive_trycnt = 0;
>
> - DBG_88E("asoc expire %pM, state = 0x%x\n", (psta->hwaddr), psta->state);
> + DBG_88E("asoc expire %pM, state = 0x%x\n",
> + (psta->hwaddr), psta->state);
> spin_lock_bh(&pstapriv->asoc_list_lock);
> list_del_init(&psta->asoc_list);
> pstapriv->asoc_list_cnt--;
> - updated = ap_free_sta(padapter, psta, true, WLAN_REASON_DEAUTH_LEAVING);
> + updated = ap_free_sta(padapter, psta, true,
> + WLAN_REASON_DEAUTH_LEAVING);
> spin_unlock_bh(&pstapriv->asoc_list_lock);
> }
>
> @@ -431,7 +439,8 @@ static void update_bmc_sta(struct adapter *padapter)
> supportRateNum = rtw_get_rateset_len((u8 *)&pcur_network->SupportedRates);
> network_type = rtw_check_network_type((u8 *)&pcur_network->SupportedRates);
>
> - memcpy(psta->bssrateset, &pcur_network->SupportedRates, supportRateNum);
> + memcpy(psta->bssrateset, &pcur_network->SupportedRates,
> + supportRateNum);
> psta->bssratelen = supportRateNum;
>
> /* b/g mode ra_bitmap */
> @@ -445,7 +454,8 @@ static void update_bmc_sta(struct adapter *padapter)
> tx_ra_bitmap = 0xf;
>
> raid = networktype_to_raid(network_type);
> - init_rate = get_highest_rate_idx(tx_ra_bitmap & 0x0fffffff) & 0x3f;
> + init_rate = get_highest_rate_idx(tx_ra_bitmap & 0x0fffffff) &
> + 0x3f;
This is misleading, it looks like 0x3f somehow belongs to the argument
of get_highest_rate_idx(). It shouldn't be there, it should be aligned
under the start of the first & operand.
>
> /* ap mode */
> rtw_hal_set_odm_var(padapter, HAL_ODM_STA_INFO, psta, true);
> @@ -456,7 +466,8 @@ static void update_bmc_sta(struct adapter *padapter)
> arg = psta->mac_id & 0x1f;
> arg |= BIT(7);
> tx_ra_bitmap |= ((raid << 28) & 0xf0000000);
> - DBG_88E("%s, mask = 0x%x, arg = 0x%x\n", __func__, tx_ra_bitmap, arg);
> + DBG_88E("%s, mask = 0x%x, arg = 0x%x\n", __func__,
> + tx_ra_bitmap, arg);
>
> /* bitmap[0:27] = tx_rate_bitmap */
> /* bitmap[28:31]= Rate Adaptive id */
> @@ -647,7 +658,8 @@ static void start_bss_network(struct adapter *padapter, u8 *pbuf)
> rtw_hal_set_hwreg(padapter, HW_VAR_SEC_CFG, (u8 *)(&val8));
>
> /* Beacon Control related register */
> - rtw_hal_set_hwreg(padapter, HW_VAR_BEACON_INTERVAL, (u8 *)(&bcn_interval));
> + rtw_hal_set_hwreg(padapter, HW_VAR_BEACON_INTERVAL,
> + (u8 *)(&bcn_interval));
>
> UpdateBrateTbl(padapter, pnetwork->SupportedRates);
> rtw_hal_set_hwreg(padapter, HW_VAR_BASIC_RATE, pnetwork->SupportedRates);
> @@ -657,7 +669,10 @@ static void start_bss_network(struct adapter *padapter, u8 *pbuf)
> Switch_DM_Func(padapter, DYNAMIC_ALL_FUNC_ENABLE, true);
> }
> /* set channel, bwmode */
> - p = rtw_get_ie((pnetwork->ies + sizeof(struct ndis_802_11_fixed_ie)), _HT_ADD_INFO_IE_, &ie_len, (pnetwork->ie_length - sizeof(struct ndis_802_11_fixed_ie)));
> + p = rtw_get_ie((pnetwork->ies + sizeof(struct ndis_802_11_fixed_ie)),
> + HT_ADD_INFO_IE_, &ie_len,
> + (pnetwork->ie_length -
> + sizeof(struct ndis_802_11_fixed_ie)));
This looks correct, but still it's horrible, all those excess
parentheses make it quite hard to read. If you plan to remove them,
perhaps it's easier to have a change doing that first, in the same
series, so that this change can be checked more easily.
> if (p && ie_len) {
> pht_info = (struct HT_info_element *)(p + 2);
>
> @@ -682,7 +697,8 @@ static void start_bss_network(struct adapter *padapter, u8 *pbuf)
> */
> set_channel_bwmode(padapter, cur_channel, cur_ch_offset, cur_bwmode);
>
> - DBG_88E("CH =%d, BW =%d, offset =%d\n", cur_channel, cur_bwmode, cur_ch_offset);
> + DBG_88E("CH =%d, BW =%d, offset =%d\n", cur_channel, cur_bwmode,
> + cur_ch_offset);
>
> /* */
> pmlmeext->cur_channel = cur_channel;
> @@ -771,7 +787,8 @@ int rtw_check_beacon_data(struct adapter *padapter, u8 *pbuf, int len)
> cap = get_unaligned_le16(ie);
>
> /* SSID */
> - p = rtw_get_ie(ie + _BEACON_IE_OFFSET_, _SSID_IE_, &ie_len, (pbss_network->ie_length - _BEACON_IE_OFFSET_));
> + p = rtw_get_ie(ie + _BEACON_IE_OFFSET_, _SSID_IE_, &ie_len,
> + (pbss_network->ie_length - _BEACON_IE_OFFSET_));
> if (p && ie_len > 0) {
> memset(&pbss_network->ssid, 0, sizeof(struct ndis_802_11_ssid));
> memcpy(pbss_network->ssid.ssid, (p + 2), ie_len);
> @@ -781,7 +798,8 @@ int rtw_check_beacon_data(struct adapter *padapter, u8 *pbuf, int len)
> /* channel */
> channel = 0;
> pbss_network->Configuration.Length = 0;
> - p = rtw_get_ie(ie + _BEACON_IE_OFFSET_, _DSSET_IE_, &ie_len, (pbss_network->ie_length - _BEACON_IE_OFFSET_));
> + p = rtw_get_ie(ie + _BEACON_IE_OFFSET_, _DSSET_IE_, &ie_len,
> + (pbss_network->ie_length - _BEACON_IE_OFFSET_));
> if (p && ie_len > 0)
> channel = *(p + 2);
>
> @@ -789,14 +807,16 @@ int rtw_check_beacon_data(struct adapter *padapter, u8 *pbuf, int len)
>
> memset(supportRate, 0, NDIS_802_11_LENGTH_RATES_EX);
> /* get supported rates */
> - p = rtw_get_ie(ie + _BEACON_IE_OFFSET_, _SUPPORTEDRATES_IE_, &ie_len, (pbss_network->ie_length - _BEACON_IE_OFFSET_));
> + p = rtw_get_ie(ie + _BEACON_IE_OFFSET_, _SUPPORTEDRATES_IE_, &ie_len,
> + (pbss_network->ie_length - _BEACON_IE_OFFSET_));
> if (p) {
> memcpy(supportRate, p + 2, ie_len);
> supportRateNum = ie_len;
> }
>
> /* get ext_supported rates */
> - p = rtw_get_ie(ie + _BEACON_IE_OFFSET_, _EXT_SUPPORTEDRATES_IE_, &ie_len, pbss_network->ie_length - _BEACON_IE_OFFSET_);
> + p = rtw_get_ie(ie + _BEACON_IE_OFFSET_, _EXT_SUPPORTEDRATES_IE_,
> + &ie_len, pbss_network->ie_length - _BEACON_IE_OFFSET_);
> if (p) {
> memcpy(supportRate + supportRateNum, p + 2, ie_len);
> supportRateNum += ie_len;
> @@ -807,7 +827,8 @@ int rtw_check_beacon_data(struct adapter *padapter, u8 *pbuf, int len)
> rtw_set_supported_rate(pbss_network->SupportedRates, network_type);
>
> /* parsing ERP_IE */
> - p = rtw_get_ie(ie + _BEACON_IE_OFFSET_, _ERPINFO_IE_, &ie_len, (pbss_network->ie_length - _BEACON_IE_OFFSET_));
> + p = rtw_get_ie(ie + _BEACON_IE_OFFSET_, _ERPINFO_IE_, &ie_len,
> + (pbss_network->ie_length - _BEACON_IE_OFFSET_));
> if (p && ie_len > 0)
> ERP_IE_handler(padapter, (struct ndis_802_11_var_ie *)p);
>
> @@ -824,7 +845,8 @@ int rtw_check_beacon_data(struct adapter *padapter, u8 *pbuf, int len)
> pairwise_cipher = 0;
> psecuritypriv->wpa2_group_cipher = _NO_PRIVACY_;
> psecuritypriv->wpa2_pairwise_cipher = _NO_PRIVACY_;
> - p = rtw_get_ie(ie + _BEACON_IE_OFFSET_, _RSN_IE_2_, &ie_len, (pbss_network->ie_length - _BEACON_IE_OFFSET_));
> + p = rtw_get_ie(ie + _BEACON_IE_OFFSET_, _RSN_IE_2_, &ie_len,
> + (pbss_network->ie_length - _BEACON_IE_OFFSET_));
> if (p && ie_len > 0) {
> if (rtw_parse_wpa2_ie(p, ie_len + 2, &group_cipher, &pairwise_cipher, NULL) == _SUCCESS) {
> psecuritypriv->dot11AuthAlgrthm = dot11AuthAlgrthm_8021X;
Same also for all these hunks.
--
Stefano
next prev parent reply other threads:[~2020-03-30 18:17 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-30 8:20 [Outreachy] [PATCH 0/2] staging: rtl8188eu: Resolve various warnings reported by checkpatch.pl Soumyajit Deb
2020-03-30 8:20 ` [Outreachy] [PATCH 1/2] staging: rtl8188eu: Properly structure the multiline comment Soumyajit Deb
2020-03-30 8:20 ` [Outreachy] [PATCH 2/2] staging: rtl8188eu: Line over 80 characters Soumyajit Deb
2020-03-30 18:17 ` Stefano Brivio [this message]
2020-03-31 14:34 ` [Outreachy kernel] " Soumyajit Deb
2020-03-31 14:35 ` Julia Lawall
2020-03-31 14:49 ` Soumyajit Deb
2020-03-31 14:49 ` Stefano Brivio
2020-03-31 15:06 ` Soumyajit Deb
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=20200330201701.719b8c0f@elisabeth \
--to=sbrivio@redhat.com \
--cc=Larry.Finger@lwfinger.net \
--cc=debsoumyajit100@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=outreachy-kernel@googlegroups.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.