[PATCH 1/6] staging: rtl8188eu: cleanup comments in mlme_linux.c

[PATCH 1/6] staging: rtl8188eu: cleanup comments in mlme_linux.c

[Michael Straube]

Cleanup comments to avoid lines over 80 characters
and follow kernel style for block comments.

Signed-off-by: Michael Straube <straube.linux@gmail.com>
---
 drivers/staging/rtl8188eu/os_dep/mlme_linux.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/rtl8188eu/os_dep/mlme_linux.c b/drivers/staging/rtl8188eu/os_dep/mlme_linux.c
index 2bc49d074ce4..0321f1d45809 100644
--- a/drivers/staging/rtl8188eu/os_dep/mlme_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/mlme_linux.c
@@ -41,17 +41,21 @@ void rtw_reset_securitypriv(struct adapter *adapter)
 	u32	backup_time = 0;
 
 	if (adapter->securitypriv.dot11AuthAlgrthm == dot11AuthAlgrthm_8021X) {
-		/* 802.1x */
-		/*  We have to backup the PMK information for WiFi PMK Caching test item. */
-		/*  Backup the btkip_countermeasure information. */
-		/*  When the countermeasure is trigger, the driver have to disconnect with AP for 60 seconds. */
+		/* 802.1x
+		 * We have to backup the PMK information for WiFi PMK Caching
+		 * test item. Backup the btkip_countermeasure information. When
+		 * the countermeasure is trigger, the driver have to disconnect
+		 * with AP for 60 seconds.
+		 */
 		memcpy(&backup_pmkid[0], &adapter->securitypriv.PMKIDList[0], sizeof(struct rt_pmkid_list) * NUM_PMKID_CACHE);
 		backup_index = adapter->securitypriv.PMKIDIndex;
 		backup_counter = adapter->securitypriv.btkip_countermeasure;
 		backup_time = adapter->securitypriv.btkip_countermeasure_time;
 		memset((unsigned char *)&adapter->securitypriv, 0, sizeof(struct security_priv));
 
-		/*  Restore the PMK information to securitypriv structure for the following connection. */
+		/* Restore the PMK information to securitypriv structure
+		 * for the following connection.
+		 */
 		memcpy(&adapter->securitypriv.PMKIDList[0], &backup_pmkid[0],
 		       sizeof(struct rt_pmkid_list) * NUM_PMKID_CACHE);
 		adapter->securitypriv.PMKIDIndex = backup_index;
@@ -75,7 +79,8 @@ void rtw_reset_securitypriv(struct adapter *adapter)
 
 void rtw_os_indicate_disconnect(struct adapter *adapter)
 {
-	netif_carrier_off(adapter->pnetdev); /*  Do it first for tx broadcast pkt after disconnection issue! */
+	/* Do it first for tx broadcast pkt after disconnection issue! */
+	netif_carrier_off(adapter->pnetdev);
 	rtw_indicate_wx_disassoc_event(adapter);
 	rtw_reset_securitypriv(adapter);
 }
-- 
2.20.1

[PATCH 2/6] staging: rtl8188eu: remove unnecessary initializations

[Michael Straube]

The local variables backup_index, backup_counter and backup_time
in rtw_reset_securitypriv() are all asigned before their uses, so
initialization to zero is not necessary.

Signed-off-by: Michael Straube <straube.linux@gmail.com>
---
 drivers/staging/rtl8188eu/os_dep/mlme_linux.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8188eu/os_dep/mlme_linux.c b/drivers/staging/rtl8188eu/os_dep/mlme_linux.c
index 0321f1d45809..cd6298e4920d 100644
--- a/drivers/staging/rtl8188eu/os_dep/mlme_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/mlme_linux.c
@@ -36,9 +36,9 @@ static struct rt_pmkid_list backup_pmkid[NUM_PMKID_CACHE];
 
 void rtw_reset_securitypriv(struct adapter *adapter)
 {
-	u8	backup_index = 0;
-	u8	backup_counter = 0x00;
-	u32	backup_time = 0;
+	u8 backup_index;
+	u8 backup_counter;
+	u32 backup_time;
 
 	if (adapter->securitypriv.dot11AuthAlgrthm == dot11AuthAlgrthm_8021X) {
 		/* 802.1x
-- 
2.20.1

[PATCH 3/6] staging: rtl8188eu: cleanup declarations in mlme_linux.c

[Michael Straube]

Replace tabs with spaces in declarations to cleanup whitespace.

Signed-off-by: Michael Straube <straube.linux@gmail.com>
---
 drivers/staging/rtl8188eu/os_dep/mlme_linux.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8188eu/os_dep/mlme_linux.c b/drivers/staging/rtl8188eu/os_dep/mlme_linux.c
index cd6298e4920d..142f8f9f6b1a 100644
--- a/drivers/staging/rtl8188eu/os_dep/mlme_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/mlme_linux.c
@@ -13,7 +13,7 @@
 
 void rtw_init_mlme_timer(struct adapter *padapter)
 {
-	struct	mlme_priv *pmlmepriv = &padapter->mlmepriv;
+	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
 
 	timer_setup(&pmlmepriv->assoc_timer, _rtw_join_timeout_handler, 0);
 	timer_setup(&pmlmepriv->scan_to_timer, rtw_scan_timeout_handler, 0);
@@ -87,8 +87,8 @@ void rtw_os_indicate_disconnect(struct adapter *adapter)
 
 void rtw_report_sec_ie(struct adapter *adapter, u8 authmode, u8 *sec_ie)
 {
-	uint	len;
-	u8	*buff, *p, i;
+	uint len;
+	u8 *buff, *p, i;
 	union iwreq_data wrqu;
 
 	RT_TRACE(_module_mlme_osdep_c_, _drv_info_,
@@ -123,7 +123,7 @@ void init_addba_retry_timer(struct adapter *padapter, struct sta_info *psta)
 
 void init_mlme_ext_timer(struct adapter *padapter)
 {
-	struct	mlme_ext_priv *pmlmeext = &padapter->mlmeextpriv;
+	struct mlme_ext_priv *pmlmeext = &padapter->mlmeextpriv;
 
 	timer_setup(&pmlmeext->survey_timer, survey_timer_hdl, 0);
 	timer_setup(&pmlmeext->link_timer, link_timer_hdl, 0);
-- 
2.20.1

[PATCH 4/6] staging: rtl8188eu: remove unncessary asignment to cleanup long line

[Michael Straube]

Instead of first asign 'wrqu.data.length = p - buff' use 'p - buff'
directly in min_t() in the subsequent asignment. Clears a line over
80 characters checkpatch warning.

Signed-off-by: Michael Straube <straube.linux@gmail.com>
---
 drivers/staging/rtl8188eu/os_dep/mlme_linux.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8188eu/os_dep/mlme_linux.c b/drivers/staging/rtl8188eu/os_dep/mlme_linux.c
index 142f8f9f6b1a..03d4cfefc9bc 100644
--- a/drivers/staging/rtl8188eu/os_dep/mlme_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/mlme_linux.c
@@ -109,8 +109,7 @@ void rtw_report_sec_ie(struct adapter *adapter, u8 authmode, u8 *sec_ie)
 			p += sprintf(p, "%02x", sec_ie[i]);
 		p += sprintf(p, ")");
 		memset(&wrqu, 0, sizeof(wrqu));
-		wrqu.data.length = p - buff;
-		wrqu.data.length = min_t(__u16, wrqu.data.length, IW_CUSTOM_MAX);
+		wrqu.data.length = min_t(__u16, p - buff, IW_CUSTOM_MAX);
 		wireless_send_event(adapter->pnetdev, IWEVCUSTOM, &wrqu, buff);
 		kfree(buff);
 	}
-- 
2.20.1

[PATCH 5/6] staging: rtl8188eu: &array[0] -> array

[Michael Straube]

Change '&array[0]' to just 'array' in rtw_reset_securitypriv().

Signed-off-by: Michael Straube <straube.linux@gmail.com>
---
 drivers/staging/rtl8188eu/os_dep/mlme_linux.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8188eu/os_dep/mlme_linux.c b/drivers/staging/rtl8188eu/os_dep/mlme_linux.c
index 03d4cfefc9bc..caba3fe0250c 100644
--- a/drivers/staging/rtl8188eu/os_dep/mlme_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/mlme_linux.c
@@ -47,7 +47,7 @@ void rtw_reset_securitypriv(struct adapter *adapter)
 		 * the countermeasure is trigger, the driver have to disconnect
 		 * with AP for 60 seconds.
 		 */
-		memcpy(&backup_pmkid[0], &adapter->securitypriv.PMKIDList[0], sizeof(struct rt_pmkid_list) * NUM_PMKID_CACHE);
+		memcpy(backup_pmkid, adapter->securitypriv.PMKIDList, sizeof(struct rt_pmkid_list) * NUM_PMKID_CACHE);
 		backup_index = adapter->securitypriv.PMKIDIndex;
 		backup_counter = adapter->securitypriv.btkip_countermeasure;
 		backup_time = adapter->securitypriv.btkip_countermeasure_time;
@@ -56,7 +56,7 @@ void rtw_reset_securitypriv(struct adapter *adapter)
 		/* Restore the PMK information to securitypriv structure
 		 * for the following connection.
 		 */
-		memcpy(&adapter->securitypriv.PMKIDList[0], &backup_pmkid[0],
+		memcpy(adapter->securitypriv.PMKIDList, backup_pmkid,
 		       sizeof(struct rt_pmkid_list) * NUM_PMKID_CACHE);
 		adapter->securitypriv.PMKIDIndex = backup_index;
 		adapter->securitypriv.btkip_countermeasure = backup_counter;
-- 
2.20.1

[PATCH 6/6] staging: rtl8188eu: refactor rtw_reset_securitypriv()

[Michael Straube]

Move the declaration of 'psec_priv' out of the else path and use it
in the if path as well to improve readability. Also clears line over
80 characters checkpatch warnings.

Suggested-by: Joe Perches <joe@perches.com>
Signed-off-by: Michael Straube <straube.linux@gmail.com>
---
 drivers/staging/rtl8188eu/os_dep/mlme_linux.c | 29 ++++++++++---------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/rtl8188eu/os_dep/mlme_linux.c b/drivers/staging/rtl8188eu/os_dep/mlme_linux.c
index caba3fe0250c..9db11b16cb93 100644
--- a/drivers/staging/rtl8188eu/os_dep/mlme_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/mlme_linux.c
@@ -39,34 +39,35 @@ void rtw_reset_securitypriv(struct adapter *adapter)
 	u8 backup_index;
 	u8 backup_counter;
 	u32 backup_time;
+	struct security_priv *psec_priv = &adapter->securitypriv;
 
-	if (adapter->securitypriv.dot11AuthAlgrthm == dot11AuthAlgrthm_8021X) {
+	if (psec_priv->dot11AuthAlgrthm == dot11AuthAlgrthm_8021X) {
 		/* 802.1x
 		 * We have to backup the PMK information for WiFi PMK Caching
 		 * test item. Backup the btkip_countermeasure information. When
 		 * the countermeasure is trigger, the driver have to disconnect
 		 * with AP for 60 seconds.
 		 */
-		memcpy(backup_pmkid, adapter->securitypriv.PMKIDList, sizeof(struct rt_pmkid_list) * NUM_PMKID_CACHE);
-		backup_index = adapter->securitypriv.PMKIDIndex;
-		backup_counter = adapter->securitypriv.btkip_countermeasure;
-		backup_time = adapter->securitypriv.btkip_countermeasure_time;
-		memset((unsigned char *)&adapter->securitypriv, 0, sizeof(struct security_priv));
+		memcpy(backup_pmkid, psec_priv->PMKIDList,
+		       sizeof(struct rt_pmkid_list) * NUM_PMKID_CACHE);
+		backup_index = psec_priv->PMKIDIndex;
+		backup_counter = psec_priv->btkip_countermeasure;
+		backup_time = psec_priv->btkip_countermeasure_time;
+
+		memset(psec_priv, 0, sizeof(*psec_priv));
 
 		/* Restore the PMK information to securitypriv structure
 		 * for the following connection.
 		 */
-		memcpy(adapter->securitypriv.PMKIDList, backup_pmkid,
+		memcpy(psec_priv->PMKIDList, backup_pmkid,
 		       sizeof(struct rt_pmkid_list) * NUM_PMKID_CACHE);
-		adapter->securitypriv.PMKIDIndex = backup_index;
-		adapter->securitypriv.btkip_countermeasure = backup_counter;
-		adapter->securitypriv.btkip_countermeasure_time = backup_time;
-		adapter->securitypriv.ndisauthtype = Ndis802_11AuthModeOpen;
-		adapter->securitypriv.ndisencryptstatus = Ndis802_11WEPDisabled;
+		psec_priv->PMKIDIndex = backup_index;
+		psec_priv->btkip_countermeasure = backup_counter;
+		psec_priv->btkip_countermeasure_time = backup_time;
+		psec_priv->ndisauthtype = Ndis802_11AuthModeOpen;
+		psec_priv->ndisencryptstatus = Ndis802_11WEPDisabled;
 	} else {
 		/* reset values in securitypriv */
-		struct security_priv *psec_priv = &adapter->securitypriv;
-
 		psec_priv->dot11AuthAlgrthm = dot11AuthAlgrthm_Open;
 		psec_priv->dot11PrivacyAlgrthm = _NO_PRIVACY_;
 		psec_priv->dot11PrivacyKeyIndex = 0;
-- 
2.20.1

Re: [PATCH 4/6] staging: rtl8188eu: remove unncessary asignment to cleanup long line

[Joe Perches]

On Mon, 2019-01-28 at 10:51 +0100, Michael Straube wrote:
> Instead of first asign 'wrqu.data.length = p - buff' use 'p - buff'
> directly in min_t() in the subsequent asignment. Clears a line over
> 80 characters checkpatch warning.

Thank you.  I believe you should include Larry Finger
in the cc list for this driver

Using scripts/get_maintainer.pl should tell you that.

And while this is true, another way to reduce line length
is to reduce indentation.

In this function, it is reasonable to change the

	if (authmode == _WPA_IE_ID_) {
		...
	}
}

to

	if (authmode != _WPA_IE_ID_)
		return;
	...
}

Another issue with this code is the sprintf.

It expands a u8 buffer of a maximum of IW_CUSTOM_MAX size
into an allocated buffer of IW_CUSTOM_MAX size with %02x;

That could overflow the allocated buffer.

> diff --git a/drivers/staging/rtl8188eu/os_dep/mlme_linux.c b/drivers/staging/rtl8188eu/os_dep/mlme_linux.c
[]
> @@ -109,8 +109,7 @@ void rtw_report_sec_ie(struct adapter *adapter, u8 authmode, u8 *sec_ie)
>  			p += sprintf(p, "%02x", sec_ie[i]);
>  		p += sprintf(p, ")");
>  		memset(&wrqu, 0, sizeof(wrqu));
> -		wrqu.data.length = p - buff;
> -		wrqu.data.length = min_t(__u16, wrqu.data.length, IW_CUSTOM_MAX);
> +		wrqu.data.length = min_t(__u16, p - buff, IW_CUSTOM_MAX);
>  		wireless_send_event(adapter->pnetdev, IWEVCUSTOM, &wrqu, buff);
>  		kfree(buff);
>  	}