From mboxrd@z Thu Jan 1 00:00:00 1970 X-GM-THRID: 6809913191851622400 X-Received: by 2002:a17:90a:db02:: with SMTP id g2mr4387408pjv.15.1585665252397; Tue, 31 Mar 2020 07:34:12 -0700 (PDT) X-BeenThere: outreachy-kernel@googlegroups.com Received: by 2002:aa7:9911:: with SMTP id z17ls14385630pff.5.gmail; Tue, 31 Mar 2020 07:34:10 -0700 (PDT) X-Google-Smtp-Source: APiQypLwIPtOmniUZdn4lFOUhPGHicM02BpbI5fVFRBEmaKTamwWo9O/jYqmFGV0KUeG9cl4Bkvm X-Received: by 2002:a62:cfc5:: with SMTP id b188mr1965629pfg.135.1585665250711; Tue, 31 Mar 2020 07:34:10 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1585665250; cv=none; d=google.com; s=arc-20160816; b=mbjsA01qjRj/zFr4FmQVhh5WDJhqcBl7PPGG7GjBmAE4FTt+ZG5aSz7xAmKBX7Khdc F063sAo3yq3jRG54/Z64qsNXPlLaPWpv+N30Mqh9ZG/Lw/CEwajl/gCY8SbVGtoXZPCo T3255aPj5YRxiSZnflKOHVseVjdILl72G3SxVE7uuqZ9PE+KH58pg3E+fwbPGRH6FXxO E6ab0NkvcbSNtn/C8eri7Cg2q28hkr7MyQkOJyigWpghIXz8rI84QgDhrP+6iqpyfppj yMDERhHg19AA2B1ji69aIl+nApiWjjyzteM16VoNdPNawl65W4KhnP0+K6l82+E0bB1y DU/A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:to:from:date:dkim-signature; bh=hmnDDu2nnGcqQ6flDKKiRPAVBJSljzIpmvuA4rbaLds=; b=YOC09V0drQK2L661Yk8nqvWsdZ3GJmeLG2hgmZf1FhTz3iMXMAeOSrToQiKQKv6B9f eyIMOFz7qPaLQgTmLdHZlEpOEQ63sW4iqAqVD6xVyvyDYXz6JhO+XQH+usXR44qYXT26 26mq+s1P0Dje9vizUCqzID6LaoQVkZ0Pj5IZKhzmf4/cghEf+YDPlsWCqe/rYBOzRQJs hnumOlpy2OtACFGhcNPMEjIdnFPtuKsvD+tR+QlI4Mo8EbdLkFQCSIoCmh26EkAKTsct nqG8qLpl6r2XB2aJXQP+HkeFQUb0sumMEhLCNGyONVeo7rdOOCXzb5MHufohcD5JnUOQ 9eRQ== ARC-Authentication-Results: i=1; gmr-mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=neIYX3Ln; spf=pass (google.com: domain of debsoumyajit100@gmail.com designates 2607:f8b0:4864:20::444 as permitted sender) smtp.mailfrom=debsoumyajit100@gmail.com; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from mail-pf1-x444.google.com (mail-pf1-x444.google.com. [2607:f8b0:4864:20::444]) by gmr-mx.google.com with ESMTPS id 59si960997ple.2.2020.03.31.07.34.10 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 31 Mar 2020 07:34:10 -0700 (PDT) Received-SPF: pass (google.com: domain of debsoumyajit100@gmail.com designates 2607:f8b0:4864:20::444 as permitted sender) client-ip=2607:f8b0:4864:20::444; Authentication-Results: gmr-mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=neIYX3Ln; spf=pass (google.com: domain of debsoumyajit100@gmail.com designates 2607:f8b0:4864:20::444 as permitted sender) smtp.mailfrom=debsoumyajit100@gmail.com; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: by mail-pf1-x444.google.com with SMTP id u65so194549pfb.4 for ; Tue, 31 Mar 2020 07:34:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=hmnDDu2nnGcqQ6flDKKiRPAVBJSljzIpmvuA4rbaLds=; b=neIYX3LnjODgvKwtgToDPkjzcJYmJwYEuF23FSfkASZu1uWajnYXQYD/AynLKXsLdR BlKJ7OZZVpP1L7xOwhi13AaJLOArytCEtgIDJjKdRDaQir/wOvOACtAtBuIye0ntjOcL 1A70i9NL52yjstLXj36x5b1B/gnZNRBMzsbe/EZnvgPchei+ppI+u13IQUycWxoO2xxE DRqss0bdk7685KmzmwQYU0ZFc46nqZZack4fs0DKXccqyFoYj5luX/zucAVdO9g22sAn 4+kyM/8reU8LuRQL0IxIlZU2/TLXMG38ZFu7fqbc8xfZhLiBLGbhFtzycYF1YJkHfz96 EJdg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=hmnDDu2nnGcqQ6flDKKiRPAVBJSljzIpmvuA4rbaLds=; b=Zpbnp4KEBQm4KUC1iHxe5bEFKSeQuJyt8RBjEeGumAhi5ee28vOY92HbVwTiohausO d2/0EtNLspqFYevN3tAKoKxJv16wUHA1fsr6bOV5nHXk2XBzqulbPyBy83KdcXb/YtRz lxRX8SmyQAjw6q9Coro9R8JUqff/zgk5mCUwgWNPICn7cjOinXlCjo5MVaBnrSUWald7 8cWhruEfQG739ZBSq0vaZHlCvq6lEqJ2tFhdZtSVNqK7XdOO8H9WnzDo7j3gbL3aXT2K Eb6lg5Py9EBvccYmwfe6SNFB3/tt+z6ZRa0iE1NRRScS6TxSj7j/V77at0AkqvnCABkv CGLg== X-Gm-Message-State: ANhLgQ3ft0JJaue6Abj3GK//Lst15t32GMpwRy/whkWR2mRMq79ZtSdF 7Y0JeDQAO8cFNivbufd2ii8= X-Received: by 2002:a62:868a:: with SMTP id x132mr17364729pfd.208.1585665249993; Tue, 31 Mar 2020 07:34:09 -0700 (PDT) Return-Path: Received: from ubuntu ([157.42.231.91]) by smtp.gmail.com with ESMTPSA id c83sm12458940pfb.44.2020.03.31.07.34.07 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 31 Mar 2020 07:34:09 -0700 (PDT) Date: Tue, 31 Mar 2020 20:04:02 +0530 From: Soumyajit Deb To: Stefano Brivio , outreachy-kernel@googlegroups.com Subject: Re: [Outreachy kernel] [Outreachy] [PATCH 2/2] staging: rtl8188eu: Line over 80 characters Message-ID: <20200331143356.GA12680@ubuntu> References: <20200330082036.12928-1-debsoumyajit100@gmail.com> <20200330082036.12928-3-debsoumyajit100@gmail.com> <20200330201701.719b8c0f@elisabeth> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200330201701.719b8c0f@elisabeth> User-Agent: Mutt/1.9.4 (2018-02-28) On Mon, Mar 30, 2020 at 08:17:01PM +0200, Stefano Brivio wrote: > On Mon, 30 Mar 2020 13:50:36 +0530 > Soumyajit Deb wrote: > > > Break various lines into multiple lines to respect the 80 character > > width limit. > > Reported by checkpatch.pl > > > > Signed-off-by: Soumyajit Deb > > --- > > 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. > Okay, yes I will add the curly brackets to the clauses and add them as new patches. Should I add this new patch to the v2 of this patchset or should I send as a seperate new patch? > > > > 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. > Okay, I will do that. To be honest, I didn't notice that earlier but now you have mentioned it, I can see the confusion current alignment is creating. > > > > /* 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. > Okay, I will remove the extra unnecessary parentheses. > > 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. > I did not understand what you mean by same for all these hunks. Do you mean I should remove extra unnecessary parentheses from these line too as suggested earlier? > -- > Stefano > Should I add these new chnages as new patches to the v2 of this patchset or should I send a altogether new patchset? Thank you