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

[Outreachy] [PATCH v2 0/3] 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 lines into
multiple lines to respect the 80 character width limit. Also, as
suggested by Stefano Brivio <sbrivio@redhat.com>, this patchset also
removes extra unnecessary parentheses to improve code readability, all
for the same file rtw_ap.c present under drivers/staging/rtl8188eu/core
directory.

Changes in v2:	
  -PATCH 1/3: No change
  -PATCH 2/3: Add curly brackets in conditional statements to improve
	      code readability and properly align the lines of a
	      function call as suggested by Stefano Brivio.
  -PATCH 3/3: Remove extra unnecessary parentheses to improve code
	      readability as suggested by Stefano Brivio
	      <sbrivio@redhat.com>

Soumyajit Deb (3):
  staging: rtl8188eu: Properly structure the multiline comment
  staging: rtl8188eu: Line over 80 characters
  staging: rtl8188eu: Remove unnecessary extra parentheses

 drivers/staging/rtl8188eu/core/rtw_ap.c | 91 ++++++++++++++++---------
 1 file changed, 57 insertions(+), 34 deletions(-)

-- 
2.17.1

[Outreachy] [PATCH v2 1/3] 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 v2 2/3] 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 | 67 +++++++++++++++++--------
 1 file changed, 45 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c
index 5b2b4d1d56f6..6b9410531b15 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,21 @@ 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);
-			else
-				ret = issue_nulldata(padapter, psta->hwaddr, 0, 3, 50);
+			if (psta->state & WIFI_SLEEP_STATE) {
+				ret = issue_nulldata(padapter, psta->hwaddr, 0,
+						     1, 50);
+			} else {
+				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 +322,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 +440,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 +455,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 +467,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 +659,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 +670,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 +698,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 +788,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 +799,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 +808,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 +828,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 +846,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 v2 2/3] staging: rtl8188eu: Line over 80 characters

[Stefano Brivio]

On Wed,  1 Apr 2020 00:02:00 +0530
Soumyajit Deb <debsoumyajit100@gmail.com> wrote:

> @@ -445,7 +455,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;

Hmm, no, wait, what is 0x3f bit-wise intersected with?

a = lol;

a = lol *
    2;

a = (lol + foo) *
    2;

a = lol + foo *
          2;

but not:

a = (lol + foo) *
         2;

...right?

>  
>  		/* ap mode */
>  		rtw_hal_set_odm_var(padapter, HAL_ODM_STA_INFO, psta, true);
> @@ -456,7 +467,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 +659,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));

This is another pair of parentheses that you could remove: &a == (&a).

-- 
Stefano

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

[Soumyajit Deb]

On Wed, Apr 01, 2020 at 04:48:38AM +0200, Stefano Brivio wrote:
> On Wed,  1 Apr 2020 00:02:00 +0530
> Soumyajit Deb <debsoumyajit100@gmail.com> wrote:
> 
> > @@ -445,7 +455,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;
> 
> Hmm, no, wait, what is 0x3f bit-wise intersected with?
> 
> a = lol;
> 
> a = lol *
>     2;
> 
> a = (lol + foo) *
>     2;
> 
> a = lol + foo *
>           2;
> 
> but not:
> 
> a = (lol + foo) *
>          2;
> 
> ...right?
>

I am sorry but I couldn't understand what you mean.

As much I can see, a bitwise AND operation is being done between 0x3f and the
return value of get_highest_rate_idx() function. The first & operation
is between the arguments of get_highest_rate_idx() function.

If the current alignment is wrong, should I place 0x3f someplace else?

But as you suggested previously in this thread, that I should place 0x3f
below the start of first & operand.

> >  
> >  		/* ap mode */
> >  		rtw_hal_set_odm_var(padapter, HAL_ODM_STA_INFO, psta, true);
> > @@ -456,7 +467,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 +659,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));
> 
> This is another pair of parentheses that you could remove: &a == (&a).
> 
> -- 
> Stefano

I noticed those parentheses while removing other extra parentheses
but I didn't remove these as most of these parentheses are present
after a type cast.
Will it not look confusing if I remove these paretheses?
If not, I will remove them :)

Thank you.

--
Soumyajit
> 

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

[Julia Lawall]

On Wed, 1 Apr 2020, Soumyajit Deb wrote:

> On Wed, Apr 01, 2020 at 04:48:38AM +0200, Stefano Brivio wrote:
> > On Wed,  1 Apr 2020 00:02:00 +0530
> > Soumyajit Deb <debsoumyajit100@gmail.com> wrote:
> >
> > > @@ -445,7 +455,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;
> >
> > Hmm, no, wait, what is 0x3f bit-wise intersected with?
> >
> > a = lol;
> >
> > a = lol *
> >     2;
> >
> > a = (lol + foo) *
> >     2;
> >
> > a = lol + foo *
> >           2;
> >
> > but not:
> >
> > a = (lol + foo) *
> >          2;
> >
> > ...right?
> >
>
> I am sorry but I couldn't understand what you mean.
>
> As much I can see, a bitwise AND operation is being done between 0x3f and the
> return value of get_highest_rate_idx() function. The first & operation
> is between the arguments of get_highest_rate_idx() function.

Putting the 0x3 underneath the middle of the argument list of the left
argument of the & is very strange.  get_highest_rate_idx is going to take
its argument and produce some value.  That value will be &ed with 0x3f.
There is no interaction between the arguments of this function and 0x3f,
so it is misleading to try to associate them in some way.  Lining up the
left side of get_highest_rate_idx (left argument of &) with the left side
of 0x3f (right argument of &) could be fine.

>
> If the current alignment is wrong, should I place 0x3f someplace else?
>
> But as you suggested previously in this thread, that I should place 0x3f
> below the start of first & operand.

There may have been a confusion because there are two &s in this code.

julia

>
> > >
> > >  		/* ap mode */
> > >  		rtw_hal_set_odm_var(padapter, HAL_ODM_STA_INFO, psta, true);
> > > @@ -456,7 +467,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 +659,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));
> >
> > This is another pair of parentheses that you could remove: &a == (&a).
> >
> > --
> > Stefano
>
> I noticed those parentheses while removing other extra parentheses
> but I didn't remove these as most of these parentheses are present
> after a type cast.
> Will it not look confusing if I remove these paretheses?
> If not, I will remove them :)
>
> Thank you.
>
> --
> Soumyajit
> >
>
> --
> 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/20200401075902.GA18430%40ubuntu.
>

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

[Soumyajit Deb]

On Wed, Apr 01, 2020 at 10:41:51AM +0200, Julia Lawall wrote:
> 
> 
> On Wed, 1 Apr 2020, Soumyajit Deb wrote:
> 
> > On Wed, Apr 01, 2020 at 04:48:38AM +0200, Stefano Brivio wrote:
> > > On Wed,  1 Apr 2020 00:02:00 +0530
> > > Soumyajit Deb <debsoumyajit100@gmail.com> wrote:
> > >
> > > > @@ -445,7 +455,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;
> > >
> > > Hmm, no, wait, what is 0x3f bit-wise intersected with?
> > >
> > > a = lol;
> > >
> > > a = lol *
> > >     2;
> > >
> > > a = (lol + foo) *
> > >     2;
> > >
> > > a = lol + foo *
> > >           2;
> > >
> > > but not:
> > >
> > > a = (lol + foo) *
> > >          2;
> > >
> > > ...right?
> > >
> >
> > I am sorry but I couldn't understand what you mean.
> >
> > As much I can see, a bitwise AND operation is being done between 0x3f and the
> > return value of get_highest_rate_idx() function. The first & operation
> > is between the arguments of get_highest_rate_idx() function.
> 
> Putting the 0x3 underneath the middle of the argument list of the left
> argument of the & is very strange.  get_highest_rate_idx is going to take
> its argument and produce some value.  That value will be &ed with 0x3f.
> There is no interaction between the arguments of this function and 0x3f,
> so it is misleading to try to associate them in some way.  Lining up the
> left side of get_highest_rate_idx (left argument of &) with the left side
> of 0x3f (right argument of &) could be fine.
> 
> >
> > If the current alignment is wrong, should I place 0x3f someplace else?
> >
> > But as you suggested previously in this thread, that I should place 0x3f
> > below the start of first & operand.
> 
> There may have been a confusion because there are two &s in this code.
> 
> julia
>
Hii Julia,

Okay, understood.
Then should I place the 0x3f on the left end of the function argument
of get_highest_rate_idx() function and resend this patch series?

Thank you

--
Soumyajit

> >
> > > >
> > > >  		/* ap mode */
> > > >  		rtw_hal_set_odm_var(padapter, HAL_ODM_STA_INFO, psta, true);
> > > > @@ -456,7 +467,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 +659,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));
> > >
> > > This is another pair of parentheses that you could remove: &a == (&a).
> > >
> > > --
> > > Stefano
> >
> > I noticed those parentheses while removing other extra parentheses
> > but I didn't remove these as most of these parentheses are present
> > after a type cast.
> > Will it not look confusing if I remove these paretheses?
> > If not, I will remove them :)
> >
> > Thank you.
> >
> > --
> > Soumyajit
> > >
> >
> > --
> > 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/20200401075902.GA18430%40ubuntu.
> >

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

[Julia Lawall]

On Thu, 2 Apr 2020, Soumyajit Deb wrote:

> On Wed, Apr 01, 2020 at 10:41:51AM +0200, Julia Lawall wrote:
> >
> >
> > On Wed, 1 Apr 2020, Soumyajit Deb wrote:
> >
> > > On Wed, Apr 01, 2020 at 04:48:38AM +0200, Stefano Brivio wrote:
> > > > On Wed,  1 Apr 2020 00:02:00 +0530
> > > > Soumyajit Deb <debsoumyajit100@gmail.com> wrote:
> > > >
> > > > > @@ -445,7 +455,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;
> > > >
> > > > Hmm, no, wait, what is 0x3f bit-wise intersected with?
> > > >
> > > > a = lol;
> > > >
> > > > a = lol *
> > > >     2;
> > > >
> > > > a = (lol + foo) *
> > > >     2;
> > > >
> > > > a = lol + foo *
> > > >           2;
> > > >
> > > > but not:
> > > >
> > > > a = (lol + foo) *
> > > >          2;
> > > >
> > > > ...right?
> > > >
> > >
> > > I am sorry but I couldn't understand what you mean.
> > >
> > > As much I can see, a bitwise AND operation is being done between 0x3f and the
> > > return value of get_highest_rate_idx() function. The first & operation
> > > is between the arguments of get_highest_rate_idx() function.
> >
> > Putting the 0x3 underneath the middle of the argument list of the left
> > argument of the & is very strange.  get_highest_rate_idx is going to take
> > its argument and produce some value.  That value will be &ed with 0x3f.
> > There is no interaction between the arguments of this function and 0x3f,
> > so it is misleading to try to associate them in some way.  Lining up the
> > left side of get_highest_rate_idx (left argument of &) with the left side
> > of 0x3f (right argument of &) could be fine.
> >
> > >
> > > If the current alignment is wrong, should I place 0x3f someplace else?
> > >
> > > But as you suggested previously in this thread, that I should place 0x3f
> > > below the start of first & operand.
> >
> > There may have been a confusion because there are two &s in this code.
> >
> > julia
> >
> Hii Julia,
>
> Okay, understood.
> Then should I place the 0x3f on the left end of the function argument
> of get_highest_rate_idx() function and resend this patch series?

No, not the argument.  At the left end of the function call.  The & has
two arguments; the function call on the left and the 0x3f on the right.

julia

>
> Thank you
>
> --
> Soumyajit
>
> > >
> > > > >
> > > > >  		/* ap mode */
> > > > >  		rtw_hal_set_odm_var(padapter, HAL_ODM_STA_INFO, psta, true);
> > > > > @@ -456,7 +467,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 +659,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));
> > > >
> > > > This is another pair of parentheses that you could remove: &a == (&a).
> > > >
> > > > --
> > > > Stefano
> > >
> > > I noticed those parentheses while removing other extra parentheses
> > > but I didn't remove these as most of these parentheses are present
> > > after a type cast.
> > > Will it not look confusing if I remove these paretheses?
> > > If not, I will remove them :)
> > >
> > > Thank you.
> > >
> > > --
> > > Soumyajit
> > > >
> > >
> > > --
> > > 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/20200401075902.GA18430%40ubuntu.
> > >
>
> --
> 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/20200402130127.GA9847%40ubuntu.
>

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

[Soumyajit Deb]

On Thu, Apr 02, 2020 at 03:04:56PM +0200, Julia Lawall wrote:
> 
> 
> On Thu, 2 Apr 2020, Soumyajit Deb wrote:
> 
> > On Wed, Apr 01, 2020 at 10:41:51AM +0200, Julia Lawall wrote:
> > >
> > >
> > > On Wed, 1 Apr 2020, Soumyajit Deb wrote:
> > >
> > > > On Wed, Apr 01, 2020 at 04:48:38AM +0200, Stefano Brivio wrote:
> > > > > On Wed,  1 Apr 2020 00:02:00 +0530
> > > > > Soumyajit Deb <debsoumyajit100@gmail.com> wrote:
> > > > >
> > > > > > @@ -445,7 +455,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;
> > > > >
> > > > > Hmm, no, wait, what is 0x3f bit-wise intersected with?
> > > > >
> > > > > a = lol;
> > > > >
> > > > > a = lol *
> > > > >     2;
> > > > >
> > > > > a = (lol + foo) *
> > > > >     2;
> > > > >
> > > > > a = lol + foo *
> > > > >           2;
> > > > >
> > > > > but not:
> > > > >
> > > > > a = (lol + foo) *
> > > > >          2;
> > > > >
> > > > > ...right?
> > > > >
> > > >
> > > > I am sorry but I couldn't understand what you mean.
> > > >
> > > > As much I can see, a bitwise AND operation is being done between 0x3f and the
> > > > return value of get_highest_rate_idx() function. The first & operation
> > > > is between the arguments of get_highest_rate_idx() function.
> > >
> > > Putting the 0x3 underneath the middle of the argument list of the left
> > > argument of the & is very strange.  get_highest_rate_idx is going to take
> > > its argument and produce some value.  That value will be &ed with 0x3f.
> > > There is no interaction between the arguments of this function and 0x3f,
> > > so it is misleading to try to associate them in some way.  Lining up the
> > > left side of get_highest_rate_idx (left argument of &) with the left side
> > > of 0x3f (right argument of &) could be fine.
> > >
> > > >
> > > > If the current alignment is wrong, should I place 0x3f someplace else?
> > > >
> > > > But as you suggested previously in this thread, that I should place 0x3f
> > > > below the start of first & operand.
> > >
> > > There may have been a confusion because there are two &s in this code.
> > >
> > > julia
> > >
> > Hii Julia,
> >
> > Okay, understood.
> > Then should I place the 0x3f on the left end of the function argument
> > of get_highest_rate_idx() function and resend this patch series?
> 
> No, not the argument.  At the left end of the function call.  The & has
> two arguments; the function call on the left and the 0x3f on the right.
> 
> julia
>

Okay, understood.

My bad, for a minute I got confused and started to consider 0x3f as a
part of the argument. 

I will make the required changes and resend the v3 of the patch
series.

Thank you

--
Soumyajit


> >
> > Thank you
> >
> > --
> > Soumyajit
> >
> > > >
> > > > > >
> > > > > >  		/* ap mode */
> > > > > >  		rtw_hal_set_odm_var(padapter, HAL_ODM_STA_INFO, psta, true);
> > > > > > @@ -456,7 +467,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 +659,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));
> > > > >
> > > > > This is another pair of parentheses that you could remove: &a == (&a).
> > > > >
> > > > > --
> > > > > Stefano
> > > >
> > > > I noticed those parentheses while removing other extra parentheses
> > > > but I didn't remove these as most of these parentheses are present
> > > > after a type cast.
> > > > Will it not look confusing if I remove these paretheses?
> > > > If not, I will remove them :)
> > > >
> > > > Thank you.
> > > >
> > > > --
> > > > Soumyajit
> > > > >
> > > >
> > > > --
> > > > 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/20200401075902.GA18430%40ubuntu.
> > > >
> >
> > --
> > 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/20200402130127.GA9847%40ubuntu.
> >

[Outreachy] [PATCH v2 3/3] staging: rtl8188eu: Remove unnecessary extra parentheses

[Soumyajit Deb]

Remove unnecessary extra parentheses to improve code readability.

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

diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c
index 6b9410531b15..9d71a3d2fd19 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ap.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
@@ -310,7 +310,7 @@ void expire_timeout_chk(struct adapter *padapter)
 			psta->keep_alive_trycnt++;
 			if (ret == _SUCCESS) {
 				DBG_88E("asoc check, sta(%pM) is alive\n",
-					(psta->hwaddr));
+					psta->hwaddr);
 				psta->expire_to = pstapriv->expire_to;
 				psta->keep_alive_trycnt = 0;
 				continue;
@@ -670,10 +670,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)),
+	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)));
+		       pnetwork->ie_length -
+		       sizeof(struct ndis_802_11_fixed_ie));
 	if (p && ie_len) {
 		pht_info = (struct HT_info_element *)(p + 2);
 
@@ -789,10 +789,10 @@ int rtw_check_beacon_data(struct adapter *padapter, u8 *pbuf,  int len)
 
 	/* SSID */
 	p = rtw_get_ie(ie + _BEACON_IE_OFFSET_, _SSID_IE_, &ie_len,
-		       (pbss_network->ie_length - _BEACON_IE_OFFSET_));
+		       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);
+		memcpy(pbss_network->ssid.ssid, p + 2, ie_len);
 		pbss_network->ssid.ssid_length = ie_len;
 	}
 
@@ -800,7 +800,7 @@ int rtw_check_beacon_data(struct adapter *padapter, u8 *pbuf,  int len)
 	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_));
+		       pbss_network->ie_length - _BEACON_IE_OFFSET_);
 	if (p && ie_len > 0)
 		channel = *(p + 2);
 
@@ -809,7 +809,7 @@ 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_));
+		       pbss_network->ie_length - _BEACON_IE_OFFSET_);
 	if (p) {
 		memcpy(supportRate, p + 2, ie_len);
 		supportRateNum = ie_len;
@@ -829,7 +829,7 @@ int rtw_check_beacon_data(struct adapter *padapter, u8 *pbuf,  int len)
 
 	/* parsing ERP_IE */
 	p = rtw_get_ie(ie + _BEACON_IE_OFFSET_, _ERPINFO_IE_, &ie_len,
-		       (pbss_network->ie_length - _BEACON_IE_OFFSET_));
+		       pbss_network->ie_length - _BEACON_IE_OFFSET_);
 	if (p && ie_len > 0)
 		ERP_IE_handler(padapter, (struct ndis_802_11_var_ie *)p);
 
@@ -847,7 +847,7 @@ int rtw_check_beacon_data(struct adapter *padapter, u8 *pbuf,  int len)
 	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_));
+		       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 v2 0/3] staging: rtl8188eu: Resolve various warnings reported by checkpatch.pl

[Stefano Brivio]

On Wed,  1 Apr 2020 00:01:58 +0530
Soumyajit Deb <debsoumyajit100@gmail.com> wrote:

> Changes in v2:	
>   -PATCH 1/3: No change
>   -PATCH 2/3: Add curly brackets in conditional statements to improve
> 	      code readability and properly align the lines of a
> 	      function call as suggested by Stefano Brivio.
>   -PATCH 3/3: Remove extra unnecessary parentheses to improve code
> 	      readability as suggested by Stefano Brivio
> 	      <sbrivio@redhat.com>

On Mon, 30 Mar 2020 20:17:01 +0200
Stefano Brivio <sbrivio@redhat.com> wrote:

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

I really meant: remove them first. See also:

On Tue, 31 Mar 2020 16:49:47 +0200
Stefano Brivio <sbrivio@redhat.com> wrote:

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

This way, I could review 3/3 much quicker. Note that your patches are
also (almost) correct as they are, so this is not really a requirement
-- it would just help my eyes a bit.

-- 
Stefano