[Outreachy] [PATCH 0/2] staging: rtl8188eu: Resolve various warnings reported by checkpatch.pl

[Outreachy] [PATCH 0/2] staging: rtl8188eu: Resolve various warnings reported by checkpatch.pl

[Soumyajit Deb]

This patchset resolves coding style and indentation issues reported by
checkpatch.pl
This patchset properly structures a multiline comment by adding "*" at
the start of every line of the comment and also breaks various lines
into multiple lines to respect the 80 character width limit, all for the
same file rtw_ap.c present under drivers/staging/rtl8188eu/core
directory.

Soumyajit Deb (2):
  staging: rtl8188eu: Properly structure the multiline comment
  staging: rtl8188eu: Line over 80 characters

 drivers/staging/rtl8188eu/core/rtw_ap.c | 84 ++++++++++++++++---------
 1 file changed, 53 insertions(+), 31 deletions(-)

-- 
2.17.1

[Outreachy] [PATCH 1/2] staging: rtl8188eu: Properly structure the multiline comment

[Soumyajit Deb]

Add "*" at the start of each line of the multiline comment to improve
code readability and to adhere to the uniform Kernel coding style.
Reported by checkpatch.pl

Signed-off-by: Soumyajit Deb <debsoumyajit100@gmail.com>
---
 drivers/staging/rtl8188eu/core/rtw_ap.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c
index 93283c7deec4..5b2b4d1d56f6 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ap.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
@@ -1226,17 +1226,17 @@ void update_beacon(struct adapter *padapter, u8 ie_id, u8 *oui, u8 tx)
 }
 
 /*
-op_mode
-Set to 0 (HT pure) under the following conditions
-	- all STAs in the BSS are 20/40 MHz HT in 20/40 MHz BSS or
-	- all STAs in the BSS are 20 MHz HT in 20 MHz BSS
-Set to 1 (HT non-member protection) if there may be non-HT STAs
-	in both the primary and the secondary channel
-Set to 2 if only HT STAs are associated in BSS,
-	however and at least one 20 MHz HT STA is associated
-Set to 3 (HT mixed mode) when one or more non-HT STAs are associated
-	(currently non-GF HT station is considered as non-HT STA also)
-*/
+ *op_mode
+ *Set to 0 (HT pure) under the following conditions
+ *	- all STAs in the BSS are 20/40 MHz HT in 20/40 MHz BSS or
+ *	- all STAs in the BSS are 20 MHz HT in 20 MHz BSS
+ *Set to 1 (HT non-member protection) if there may be non-HT STAs
+ *	in both the primary and the secondary channel
+ *Set to 2 if only HT STAs are associated in BSS,
+ *	however and at least one 20 MHz HT STA is associated
+ *Set to 3 (HT mixed mode) when one or more non-HT STAs are associated
+ *	(currently non-GF HT station is considered as non-HT STA also)
+ */
 static int rtw_ht_operation_update(struct adapter *padapter)
 {
 	u16 cur_op_mode, new_op_mode;
-- 
2.17.1

[Outreachy] [PATCH 2/2] staging: rtl8188eu: Line over 80 characters

[Soumyajit Deb]

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);
 
 			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));
 				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;
 
 		/* 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)));
 	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;
-- 
2.17.1

Re: [Outreachy kernel] [Outreachy] [PATCH 2/2] staging: rtl8188eu: Line over 80 characters

[Stefano Brivio]

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

Re: [Outreachy kernel] [Outreachy] [PATCH 2/2] staging: rtl8188eu: Line over 80 characters

[Soumyajit Deb]

On Mon, Mar 30, 2020 at 08:17:01PM +0200, Stefano Brivio wrote:
> 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.
>

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

Re: [Outreachy kernel] [Outreachy] [PATCH 2/2] staging: rtl8188eu: Line over 80 characters

[Julia Lawall]

On Tue, 31 Mar 2020, Soumyajit Deb wrote:

> On Mon, Mar 30, 2020 at 08:17:01PM +0200, Stefano Brivio wrote:
> > 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.
> >
>
> 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?

If you make multiple changes in one file, they have to be in a series so
Greg knows the order in which to apply them.

julia

>
>
>
> > >
> > >  			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
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20200331143356.GA12680%40ubuntu.
>

Re: [Outreachy kernel] [Outreachy] [PATCH 2/2] staging: rtl8188eu: Line over 80 characters

[Soumyajit Deb]

On Tue, Mar 31, 2020 at 04:35:46PM +0200, Julia Lawall wrote:
> 
> 
> On Tue, 31 Mar 2020, Soumyajit Deb wrote:
> 
> > On Mon, Mar 30, 2020 at 08:17:01PM +0200, Stefano Brivio wrote:
> > > 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.
> > >
> >
> > 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?
> 
> If you make multiple changes in one file, they have to be in a series so
> Greg knows the order in which to apply them.
> 
> julia
>

Okay, but my question meant something different. 
Sorry,if my question wasn't clear enough.

My question was do I have to add the new changes as suggested by Stefano
as new patches to the v2 of the current existing patchset or should I
send the changes suggested by Stefano as totally different patches?

Thank you.
--
Soumyajit

> >
> >
> >
> > > >
> > > >  			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
> >
> > --
> > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20200331143356.GA12680%40ubuntu.
> >

Re: [Outreachy kernel] [Outreachy] [PATCH 2/2] staging: rtl8188eu: Line over 80 characters

[Stefano Brivio]

On Tue, 31 Mar 2020 20:04:02 +0530
Soumyajit Deb <debsoumyajit100@gmail.com> wrote:

> On Mon, Mar 30, 2020 at 08:17:01PM +0200, Stefano Brivio wrote:
> > 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.
> >  
> 
> Okay, yes I will add the curly brackets to the clauses and add them as
> new patches. 

No, wait, the curly brackets are needed as soon as you split those
lines. They need to be in the same patch.

> Should I add this new patch to the v2 of this patchset or should I send
> as a seperate new patch?

Yes, v2 of this patchset, but for this change specifically, it's not a
new patch.

> [...]
>
> > > @@ -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.

Just for clarity: that needs to be done in a separate patch, in the
same series.

> > >  	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?

Yes, see https://en.wiktionary.org/wiki/hunk#Noun in the computing
sense.

> Should I add these new chnages as new patches to the v2 of this patchset
> or should I send a altogether new patchset?

Some of it (removing ()) is a new patch that needs to be added as v2 of
this patchset (also called "series"). Some of it are new versions of the
patch you already posted. You need anyway to re-send the full patchset.
That is:

- the current patchset has:
	- patch 1/2
	- patch 2/2

- the new patchset (v2) will have:
	- patch 1/3 v2 (no changes)
	- patch 2/3 v2 (for example, if you remove () here)
	- patch 3/3 v2 (this current patch with the changes I suggested)

-- 
Stefano

Re: [Outreachy kernel] [Outreachy] [PATCH 2/2] staging: rtl8188eu: Line over 80 characters

[Soumyajit Deb]

On Tue, Mar 31, 2020 at 04:49:47PM +0200, Stefano Brivio wrote:
> On Tue, 31 Mar 2020 20:04:02 +0530
> Soumyajit Deb <debsoumyajit100@gmail.com> wrote:
> 
> > On Mon, Mar 30, 2020 at 08:17:01PM +0200, Stefano Brivio wrote:
> > > 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.
> > >  
> > 
> > Okay, yes I will add the curly brackets to the clauses and add them as
> > new patches. 
> 
> No, wait, the curly brackets are needed as soon as you split those
> lines. They need to be in the same patch.
> 
> > Should I add this new patch to the v2 of this patchset or should I send
> > as a seperate new patch?
> 
> Yes, v2 of this patchset, but for this change specifically, it's not a
> new patch.
> 
> > [...]
> >
> > > > @@ -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.
> 
> Just for clarity: that needs to be done in a separate patch, in the
> same series.
> 
> > > >  	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?
> 
> Yes, see https://en.wiktionary.org/wiki/hunk#Noun in the computing
> sense.
> 
> > Should I add these new chnages as new patches to the v2 of this patchset
> > or should I send a altogether new patchset?
> 
> Some of it (removing ()) is a new patch that needs to be added as v2 of
> this patchset (also called "series"). Some of it are new versions of the
> patch you already posted. You need anyway to re-send the full patchset.
> That is:
> 
> - the current patchset has:
> 	- patch 1/2
> 	- patch 2/2
> 
> - the new patchset (v2) will have:
> 	- patch 1/3 v2 (no changes)
> 	- patch 2/3 v2 (for example, if you remove () here)
> 	- patch 3/3 v2 (this current patch with the changes I suggested)
> 
> -- 
> Stefano
>

Thank you, Stefano
I will make the changes you suggested and will add them in  v2 of this
patch series.

--
Soumyajit