[PATCH v3 0/7] staging: r8188eu: fix warnings reported by checkpatch

[PATCH v3 0/7] staging: r8188eu: fix warnings reported by checkpatch

[Jaehee Park]

These patches address style issues found by checkpatch in the
core/rtw_mlme.c file.

changes in v3:
- first patch that removes an unused member needed more removal of 
  unused variables. ubuf has been removed. 
- the log of the third patch has been edited.
- another small patch was added to remove un unused else condition.


Jaehee Park (7):
  staging: r8188eu: remove unused member free_bss_buf
  staging: r8188eu: remove spaces before tabs
  staging: r8188eu: remove 'added by' author comments
  staging: r8188eu: place constants on the right side of tests
  staging: r8188eu: replace spaces with tabs
  staging: r8188eu: correct typo in comments
  staging: r8188eu: remove unused else condition

 drivers/staging/r8188eu/include/rtw_mlme.h |  1 -
 drivers/staging/r8188eu/core/rtw_mlme.c    | 63 +++++++---------------
 2 files changed, 19 insertions(+), 45 deletions(-)

-- 
2.25.1

[PATCH v3 1/7] staging: r8188eu: remove unused member free_bss_buf

[Jaehee Park]

The free_bss_buf member of pmlmepriv is unused. Remove all related
lines.

Suggested-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
---
 drivers/staging/r8188eu/include/rtw_mlme.h |  1 -
 drivers/staging/r8188eu/core/rtw_mlme.c    | 21 +--------------------
 2 files changed, 1 insertion(+), 21 deletions(-)

diff --git a/drivers/staging/r8188eu/include/rtw_mlme.h b/drivers/staging/r8188eu/include/rtw_mlme.h
index 1dc1fbf049af..0f03ac43079c 100644
--- a/drivers/staging/r8188eu/include/rtw_mlme.h
+++ b/drivers/staging/r8188eu/include/rtw_mlme.h
@@ -319,7 +319,6 @@ struct mlme_priv {
 	struct list_head *pscanned;
 	struct __queue free_bss_pool;
 	struct __queue scanned_queue;
-	u8 *free_bss_buf;
 	u8	key_mask; /* use to restore wep key after hal_init */
 	u32	num_of_scanned;
 
diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
index 3e9882f89f76..8af11626a3e7 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme.c
@@ -33,8 +33,7 @@ u8 rtw_to_roaming(struct adapter *adapter)
 static int _rtw_init_mlme_priv(struct adapter *padapter)
 {
 	int	i;
-	u8	*pbuf;
-	struct wlan_network	*pnetwork;
+	struct wlan_network	*pnetwork = NULL;
 	struct mlme_priv		*pmlmepriv = &padapter->mlmepriv;
 	int	res = _SUCCESS;
 
@@ -55,16 +54,6 @@ static int _rtw_init_mlme_priv(struct adapter *padapter)
 
 	memset(&pmlmepriv->assoc_ssid, 0, sizeof(struct ndis_802_11_ssid));
 
-	pbuf = vzalloc(MAX_BSS_CNT * (sizeof(struct wlan_network)));
-
-	if (!pbuf) {
-		res = _FAIL;
-		goto exit;
-	}
-	pmlmepriv->free_bss_buf = pbuf;
-
-	pnetwork = (struct wlan_network *)pbuf;
-
 	for (i = 0; i < MAX_BSS_CNT; i++) {
 		INIT_LIST_HEAD(&pnetwork->list);
 
@@ -79,8 +68,6 @@ static int _rtw_init_mlme_priv(struct adapter *padapter)
 
 	rtw_init_mlme_timer(padapter);
 
-exit:
-
 	return res;
 }
 
@@ -109,13 +96,7 @@ void rtw_free_mlme_priv_ie_data(struct mlme_priv *pmlmepriv)
 
 void _rtw_free_mlme_priv(struct mlme_priv *pmlmepriv)
 {
-
 	rtw_free_mlme_priv_ie_data(pmlmepriv);
-
-	if (pmlmepriv) {
-		vfree(pmlmepriv->free_bss_buf);
-	}
-
 }
 
 struct	wlan_network *_rtw_alloc_network(struct	mlme_priv *pmlmepriv)/* _queue *free_queue) */
-- 
2.25.1

[PATCH v3 2/7] staging: r8188eu: remove spaces before tabs

[Jaehee Park]

Delete spaces before tabs in the comments. Issue found with checkpatch.
WARNING: please, no space before tabs

Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
---
 drivers/staging/r8188eu/core/rtw_mlme.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
index 8af11626a3e7..72963ed7b665 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme.c
@@ -555,8 +555,8 @@ static void rtw_add_network(struct adapter *adapter,
 
 /* select the desired network based on the capability of the (i)bss. */
 /*  check items:	(1) security */
-/* 			(2) network_type */
-/* 			(3) WMM */
+/*			(2) network_type */
+/*			(3) WMM */
 /*			(4) HT */
 /*			(5) others */
 static bool rtw_is_desired_network(struct adapter *adapter, struct wlan_network *pnetwork)
@@ -891,9 +891,9 @@ static struct sta_info *rtw_joinbss_update_stainfo(struct adapter *padapter, str
 			memset((u8 *)&psta->dot11txpn, 0, sizeof(union pn48));
 			memset((u8 *)&psta->dot11rxpn, 0, sizeof(union pn48));
 		}
-		/* 	Commented by Albert 2012/07/21 */
-		/* 	When doing the WPS, the wps_ie_len won't equal to 0 */
-		/* 	And the Wi-Fi driver shouldn't allow the data packet to be tramsmitted. */
+		/*	Commented by Albert 2012/07/21 */
+		/*	When doing the WPS, the wps_ie_len won't equal to 0 */
+		/*	And the Wi-Fi driver shouldn't allow the data packet to be tramsmitted. */
 		if (padapter->securitypriv.wps_ie_len != 0) {
 			psta->ieee8021x_blocked = true;
 			padapter->securitypriv.wps_ie_len = 0;
@@ -1616,8 +1616,8 @@ int rtw_restruct_wmm_ie(struct adapter *adapter, u8 *in_ie, u8 *out_ie, uint in_
 /*  */
 /*  Search by BSSID, */
 /*  Return Value: */
-/* 		-1		:if there is no pre-auth key in the  table */
-/* 		>= 0		:if there is pre-auth key, and   return the entry id */
+/*		-1		:if there is no pre-auth key in the  table */
+/*		>= 0		:if there is pre-auth key, and   return the entry id */
 /*  */
 /*  */
 
-- 
2.25.1

[PATCH v3 3/7] staging: r8188eu: remove 'added by' author comments

[Jaehee Park]

Author comments "Added by Albert" and "Added by Annie" are sprinkled
through the file. These comments are not useful and can be removed.

Suggested-by: Alison Schofield <alison.schofield@intel.com>
Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
---
 drivers/staging/r8188eu/core/rtw_mlme.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
index 72963ed7b665..47f528c14b25 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme.c
@@ -891,7 +891,6 @@ static struct sta_info *rtw_joinbss_update_stainfo(struct adapter *padapter, str
 			memset((u8 *)&psta->dot11txpn, 0, sizeof(union pn48));
 			memset((u8 *)&psta->dot11rxpn, 0, sizeof(union pn48));
 		}
-		/*	Commented by Albert 2012/07/21 */
 		/*	When doing the WPS, the wps_ie_len won't equal to 0 */
 		/*	And the Wi-Fi driver shouldn't allow the data packet to be tramsmitted. */
 		if (padapter->securitypriv.wps_ie_len != 0) {
@@ -1610,9 +1609,6 @@ int rtw_restruct_wmm_ie(struct adapter *adapter, u8 *in_ie, u8 *out_ie, uint in_
 	return ielength;
 }
 
-/*  */
-/*  Ported from 8185: IsInPreAuthKeyList(). (Renamed from SecIsInPreAuthKeyList(), 2006-10-13.) */
-/*  Added by Annie, 2006-05-07. */
 /*  */
 /*  Search by BSSID, */
 /*  Return Value: */
-- 
2.25.1

[PATCH v3 4/7] staging: r8188eu: place constants on the right side of tests

[Jaehee Park]

To comply with the linux coding style, place constants on the right
side of the test in comparisons. Issue found with checkpatch.
WARNING: Comparisons should place the constant on the right side of
the test.

Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
---
 drivers/staging/r8188eu/core/rtw_mlme.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
index 47f528c14b25..adaf0f813a97 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme.c
@@ -703,7 +703,7 @@ void rtw_surveydone_event_callback(struct adapter	*adapter, u8 *pbuf)
 			set_fwstate(pmlmepriv, _FW_UNDER_LINKING);
 			pmlmepriv->to_join = false;
 			s_ret = rtw_select_and_join_from_scanned_queue(pmlmepriv);
-			if (_SUCCESS == s_ret) {
+			if (s_ret == _SUCCESS) {
 			     _set_timer(&pmlmepriv->assoc_timer, MAX_JOIN_TIMEOUT);
 			} else if (s_ret == 2) { /* there is no need to wait for join */
 				_clr_fwstate_(pmlmepriv, _FW_UNDER_LINKING);
@@ -711,7 +711,8 @@ void rtw_surveydone_event_callback(struct adapter	*adapter, u8 *pbuf)
 			} else {
 				if (rtw_to_roaming(adapter) != 0) {
 					if (--pmlmepriv->to_roaming == 0 ||
-					    _SUCCESS != rtw_sitesurvey_cmd(adapter, &pmlmepriv->assoc_ssid, 1, NULL, 0)) {
+					    rtw_sitesurvey_cmd(adapter, &pmlmepriv->assoc_ssid,
+							       1, NULL, 0) != _SUCCESS) {
 						rtw_set_roaming(adapter, 0);
 						rtw_free_assoc_resources(adapter, 1);
 						rtw_indicate_disconnect(adapter);
@@ -1952,7 +1953,7 @@ void rtw_issue_addbareq_cmd(struct adapter *padapter, struct xmit_frame *pxmitfr
 		issued = (phtpriv->agg_enable_bitmap >> priority) & 0x1;
 		issued |= (phtpriv->candidate_tid_bitmap >> priority) & 0x1;
 
-		if (0 == issued) {
+		if (issued == 0) {
 			psta->htpriv.candidate_tid_bitmap |= BIT((u8)priority);
 			rtw_addbareq_cmd(padapter, (u8)priority, pattrib->ra);
 		}
@@ -1979,19 +1980,19 @@ void _rtw_roaming(struct adapter *padapter, struct wlan_network *tgt_network)
 	else
 		pnetwork = &pmlmepriv->cur_network;
 
-	if (0 < rtw_to_roaming(padapter)) {
+	if (rtw_to_roaming(padapter) > 0) {
 		memcpy(&pmlmepriv->assoc_ssid, &pnetwork->network.Ssid, sizeof(struct ndis_802_11_ssid));
 
 		pmlmepriv->assoc_by_bssid = false;
 
 		while (1) {
 			do_join_r = rtw_do_join(padapter);
-			if (_SUCCESS == do_join_r) {
+			if (do_join_r == _SUCCESS) {
 				break;
 			} else {
 				pmlmepriv->to_roaming--;
 
-				if (0 < pmlmepriv->to_roaming) {
+				if (pmlmepriv->to_roaming > 0) {
 					continue;
 				} else {
 					rtw_indicate_disconnect(padapter);
-- 
2.25.1

[PATCH v3 5/7] staging: r8188eu: replace spaces with tabs

[Jaehee Park]

Use tabs instead of spaces. Issue found with checkpatch.
WARNING: suspect code indent for conditional statements

Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
---
 drivers/staging/r8188eu/core/rtw_mlme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
index adaf0f813a97..173754c127e3 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme.c
@@ -704,7 +704,7 @@ void rtw_surveydone_event_callback(struct adapter	*adapter, u8 *pbuf)
 			pmlmepriv->to_join = false;
 			s_ret = rtw_select_and_join_from_scanned_queue(pmlmepriv);
 			if (s_ret == _SUCCESS) {
-			     _set_timer(&pmlmepriv->assoc_timer, MAX_JOIN_TIMEOUT);
+				_set_timer(&pmlmepriv->assoc_timer, MAX_JOIN_TIMEOUT);
 			} else if (s_ret == 2) { /* there is no need to wait for join */
 				_clr_fwstate_(pmlmepriv, _FW_UNDER_LINKING);
 				rtw_indicate_connect(adapter);
-- 
2.25.1

[PATCH v3 6/7] staging: r8188eu: correct typo in comments

[Jaehee Park]

Correct misspellings in the comments. Issue found with checkpatch.

Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
---
 drivers/staging/r8188eu/core/rtw_mlme.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
index 173754c127e3..5ada336ac723 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme.c
@@ -174,7 +174,7 @@ void _rtw_free_network_nolock(struct	mlme_priv *pmlmepriv, struct wlan_network *
 /*
 	return the wlan_network with the matching addr
 
-	Shall be calle under atomic context... to avoid possible racing condition...
+	Shall be called under atomic context... to avoid possible racing condition...
 */
 struct wlan_network *_rtw_find_network(struct __queue *scanned_queue, u8 *addr)
 {
@@ -310,7 +310,7 @@ void rtw_free_network_queue(struct adapter *dev, u8 isfreeall)
 /*
 	return the wlan_network with the matching addr
 
-	Shall be calle under atomic context... to avoid possible racing condition...
+	Shall be called under atomic context... to avoid possible racing condition...
 */
 struct	wlan_network *rtw_find_network(struct __queue *scanned_queue, u8 *addr)
 {
@@ -893,7 +893,7 @@ static struct sta_info *rtw_joinbss_update_stainfo(struct adapter *padapter, str
 			memset((u8 *)&psta->dot11rxpn, 0, sizeof(union pn48));
 		}
 		/*	When doing the WPS, the wps_ie_len won't equal to 0 */
-		/*	And the Wi-Fi driver shouldn't allow the data packet to be tramsmitted. */
+		/*	And the Wi-Fi driver shouldn't allow the data packet to be transmitted. */
 		if (padapter->securitypriv.wps_ie_len != 0) {
 			psta->ieee8021x_blocked = true;
 			padapter->securitypriv.wps_ie_len = 0;
@@ -1287,7 +1287,7 @@ void rtw_stadel_event_callback(struct adapter *adapter, u8 *pbuf)
 }
 
 /*
-* _rtw_join_timeout_handler - Timeout/faliure handler for CMD JoinBss
+* _rtw_join_timeout_handler - Timeout/failure handler for CMD JoinBss
 * @adapter: pointer to struct adapter structure
 */
 void _rtw_join_timeout_handler (struct adapter *adapter)
@@ -1322,7 +1322,7 @@ void _rtw_join_timeout_handler (struct adapter *adapter)
 }
 
 /*
-* rtw_scan_timeout_handler - Timeout/Faliure handler for CMD SiteSurvey
+* rtw_scan_timeout_handler - Timeout/Failure handler for CMD SiteSurvey
 * @adapter: pointer to struct adapter structure
 */
 void rtw_scan_timeout_handler (struct adapter *adapter)
-- 
2.25.1

[PATCH v3 7/7] staging: r8188eu: remove unused else condition

[Jaehee Park]

s_ret cannot return '2' so this else condition is unused. Remove this
unnecessary else statement.

Suggested-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
---
 drivers/staging/r8188eu/core/rtw_mlme.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
index 5ada336ac723..e60e1903950f 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme.c
@@ -705,9 +705,6 @@ void rtw_surveydone_event_callback(struct adapter	*adapter, u8 *pbuf)
 			s_ret = rtw_select_and_join_from_scanned_queue(pmlmepriv);
 			if (s_ret == _SUCCESS) {
 				_set_timer(&pmlmepriv->assoc_timer, MAX_JOIN_TIMEOUT);
-			} else if (s_ret == 2) { /* there is no need to wait for join */
-				_clr_fwstate_(pmlmepriv, _FW_UNDER_LINKING);
-				rtw_indicate_connect(adapter);
 			} else {
 				if (rtw_to_roaming(adapter) != 0) {
 					if (--pmlmepriv->to_roaming == 0 ||
-- 
2.25.1

Re: [PATCH v3 1/7] staging: r8188eu: remove unused member free_bss_buf

[Pavel Skripkin]

Hi Jaehee,

On 4/19/22 21:19, Jaehee Park wrote:
> The free_bss_buf member of pmlmepriv is unused. Remove all related
> lines.
> 
> Suggested-by: Pavel Skripkin <paskripkin@gmail.com>
> Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
> ---

[code snip]

> @@ -55,16 +54,6 @@ static int _rtw_init_mlme_priv(struct adapter *padapter)
>   
>   	memset(&pmlmepriv->assoc_ssid, 0, sizeof(struct ndis_802_11_ssid));
>   
> -	pbuf = vzalloc(MAX_BSS_CNT * (sizeof(struct wlan_network)));
> -
> -	if (!pbuf) {
> -		res = _FAIL;
> -		goto exit;
> -	}
> -	pmlmepriv->free_bss_buf = pbuf;
> -
> -	pnetwork = (struct wlan_network *)pbuf;
> -
>   	for (i = 0; i < MAX_BSS_CNT; i++) {
>   		INIT_LIST_HEAD(&pnetwork->list);
>   

And now kernel will just boom here :)

I am sorry for not noticing that from the beginning, that's my fault. 
This pointer is used in very weird way, so let's come back to initial 
patch: just remove 'if' before vfree().

Refactoring that place is the separate task


I am again sorry for wrong suggestion,


With regards,
Pavel Skripkin

Re: [PATCH v3 1/7] staging: r8188eu: remove unused member free_bss_buf

[Dan Carpenter]

As a kernel community, I don't think we are pro-active enough about
preventing bugs.  One of my co-workers and I have started a bi weekly
phone call to look at CVEs from the previous week and discuss how they
could have been prevented or detected earlier.  Of course, my solutions
are always centered around static analysis because that's my deal but
some bugs are caused by process issues, or could be detected with better
testing.

In this case there were two bugs proposed bugs.

1) The memory leak that Fabio noticed.  Smatch is bad at detecting
   memory leaks.  It's a hard problem, because in this case it's
   across function boundaries.

Fabio caught the leak.  I don't know if I would have.

2) The NULL dereference.

   The "&pnetwork->list" expression is not a dereference so this is also
   a cross function thing.  I thought I used to have an unpublished check
   for bogus addresses like that where pnetwork is NULL.

   Another is idea is that when you have pnetwork++ and it's a NULL
   pointer then print an error message.  Or even potentially NULL.
   There are various heuristics to use which mean that "A reasonable
   person would think this could be NULL".

   Or another idea would be that we could test patches.  Right now we
   don't really have a way to test these.  But, of course, we wish we
   did.

It's not super likely that we would have committed the NULL deref
patch.  I would have caught that one if you didn't and Fabio likely
would have as well.  But I like to remove the human error whenever I
can.

regards,
dan carpenter

Re: [PATCH v3 1/7] staging: r8188eu: remove unused member free_bss_buf

[Jaehee Park]

On Wed, Apr 20, 2022 at 02:55:34PM +0300, Dan Carpenter wrote:
> As a kernel community, I don't think we are pro-active enough about
> preventing bugs.  One of my co-workers and I have started a bi weekly
> phone call to look at CVEs from the previous week and discuss how they
> could have been prevented or detected earlier.  Of course, my solutions
> are always centered around static analysis because that's my deal but
> some bugs are caused by process issues, or could be detected with better
> testing.
> 
> In this case there were two bugs proposed bugs.
> 
> 1) The memory leak that Fabio noticed.  Smatch is bad at detecting
>    memory leaks.  It's a hard problem, because in this case it's
>    across function boundaries.
> 
> Fabio caught the leak.  I don't know if I would have.
> 
> 2) The NULL dereference.
> 
>    The "&pnetwork->list" expression is not a dereference so this is also
>    a cross function thing.  I thought I used to have an unpublished check
>    for bogus addresses like that where pnetwork is NULL.
> 
>    Another is idea is that when you have pnetwork++ and it's a NULL
>    pointer then print an error message.  Or even potentially NULL.
>    There are various heuristics to use which mean that "A reasonable
>    person would think this could be NULL".
> 
>    Or another idea would be that we could test patches.  Right now we
>    don't really have a way to test these.  But, of course, we wish we
>    did.
> 
> It's not super likely that we would have committed the NULL deref
> patch.  I would have caught that one if you didn't and Fabio likely
> would have as well.  But I like to remove the human error whenever I
> can.
> 
> regards,
> dan carpenter

I'm sorry about the NULL dereference. I wasn't sure about pnetwork
and I should've asked. I wanted to ask why pbuf should be removed
when it was being used for pnetwork but ended up not asking and
sent a faulty patch. Sorry again and thank you for explaining the
errors. I will be more careful about memory leaks and dereferencing
errors. Are there checks that I can run to detect these?

Thanks,
Jaehee

Re: [PATCH v3 1/7] staging: r8188eu: remove unused member free_bss_buf

[Greg KH]

On Tue, Apr 19, 2022 at 02:19:32PM -0400, Jaehee Park wrote:
> The free_bss_buf member of pmlmepriv is unused. Remove all related
> lines.
> 
> Suggested-by: Pavel Skripkin <paskripkin@gmail.com>
> Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
> ---
>  drivers/staging/r8188eu/include/rtw_mlme.h |  1 -
>  drivers/staging/r8188eu/core/rtw_mlme.c    | 21 +--------------------
>  2 files changed, 1 insertion(+), 21 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/include/rtw_mlme.h b/drivers/staging/r8188eu/include/rtw_mlme.h
> index 1dc1fbf049af..0f03ac43079c 100644
> --- a/drivers/staging/r8188eu/include/rtw_mlme.h
> +++ b/drivers/staging/r8188eu/include/rtw_mlme.h
> @@ -319,7 +319,6 @@ struct mlme_priv {
>  	struct list_head *pscanned;
>  	struct __queue free_bss_pool;
>  	struct __queue scanned_queue;
> -	u8 *free_bss_buf;
>  	u8	key_mask; /* use to restore wep key after hal_init */
>  	u32	num_of_scanned;
>  
> diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
> index 3e9882f89f76..8af11626a3e7 100644
> --- a/drivers/staging/r8188eu/core/rtw_mlme.c
> +++ b/drivers/staging/r8188eu/core/rtw_mlme.c
> @@ -33,8 +33,7 @@ u8 rtw_to_roaming(struct adapter *adapter)
>  static int _rtw_init_mlme_priv(struct adapter *padapter)
>  {
>  	int	i;
> -	u8	*pbuf;
> -	struct wlan_network	*pnetwork;
> +	struct wlan_network	*pnetwork = NULL;

Why did you add this line?  It caused the real error to be ignored that
the compiler was trying to tell you :(

Re: [PATCH v3 1/7] staging: r8188eu: remove unused member free_bss_buf

[Dan Carpenter]

On Wed, Apr 20, 2022 at 10:48:34AM -0400, Jaehee Park wrote:
> On Wed, Apr 20, 2022 at 02:55:34PM +0300, Dan Carpenter wrote:
> > As a kernel community, I don't think we are pro-active enough about
> > preventing bugs.  One of my co-workers and I have started a bi weekly
> > phone call to look at CVEs from the previous week and discuss how they
> > could have been prevented or detected earlier.  Of course, my solutions
> > are always centered around static analysis because that's my deal but
> > some bugs are caused by process issues, or could be detected with better
> > testing.
> > 
> > In this case there were two bugs proposed bugs.
> > 
> > 1) The memory leak that Fabio noticed.  Smatch is bad at detecting
> >    memory leaks.  It's a hard problem, because in this case it's
> >    across function boundaries.
> > 
> > Fabio caught the leak.  I don't know if I would have.
> > 
> > 2) The NULL dereference.
> > 
> >    The "&pnetwork->list" expression is not a dereference so this is also
> >    a cross function thing.  I thought I used to have an unpublished check
> >    for bogus addresses like that where pnetwork is NULL.
> > 
> >    Another is idea is that when you have pnetwork++ and it's a NULL
> >    pointer then print an error message.  Or even potentially NULL.
> >    There are various heuristics to use which mean that "A reasonable
> >    person would think this could be NULL".
> > 
> >    Or another idea would be that we could test patches.  Right now we
> >    don't really have a way to test these.  But, of course, we wish we
> >    did.
> > 
> > It's not super likely that we would have committed the NULL deref
> > patch.  I would have caught that one if you didn't and Fabio likely
> > would have as well.  But I like to remove the human error whenever I
> > can.
> > 
> > regards,
> > dan carpenter
> 
> I'm sorry about the NULL dereference. I wasn't sure about pnetwork
> and I should've asked. I wanted to ask why pbuf should be removed
> when it was being used for pnetwork but ended up not asking and
> sent a faulty patch. Sorry again and thank you for explaining the
> errors. I will be more careful about memory leaks and dereferencing
> errors. Are there checks that I can run to detect these?

I was going to suggest that you could write a check yourself, but then
when I looked at it it became quite complicated.  :P

How about I write the check and send you the output tomorrow?

regards,
dan carpenter

Re: [PATCH v3 1/7] staging: r8188eu: remove unused member free_bss_buf

[Dan Carpenter]

On Wed, Apr 20, 2022 at 08:39:26PM +0300, Dan Carpenter wrote:
> > >    Another is idea is that when you have pnetwork++ and it's a NULL
> > >    pointer then print an error message.  Or even potentially NULL.
> > >    There are various heuristics to use which mean that "A reasonable
> > >    person would think this could be NULL".
> > > 

I wrote the check but it turns out none of the results are that
interesting after all.

drivers/atm/horizon.c:1844 hrz_init() warn: incrementing NULL pointer 'mem' rl='0'
drivers/scsi/qla2xxx/qla_iocb.c:665 qla24xx_build_scsi_type_6_iocbs() warn: incrementing NULL pointer 'cur_dsd' rl='4097-ptr_max'
drivers/scsi/be2iscsi/be_main.c:2686 beiscsi_init_wrb_handle() warn: incrementing NULL pointer 'pwrb' rl='0-u64max'
drivers/net/ethernet/nvidia/forcedeth.c:2304 nv_start_xmit() warn: incrementing NULL pointer 'tmp_tx_ctx' rl='0-u64max'
drivers/net/ethernet/nvidia/forcedeth.c:2482 nv_start_xmit_optimized() warn: incrementing NULL pointer 'tmp_tx_ctx' rl='0-u64max'
arch/x86/boot/cmdline.c:66 __cmdline_find_option() warn: incrementing NULL pointer 'opptr' rl='0,4096-ptr_max'
arch/x86/boot/compressed/../cmdline.c:66 __cmdline_find_option() warn: incrementing NULL pointer 'opptr' rl='0,4096-ptr_max'
arch/x86/lib/cmdline.c:84 __cmdline_find_option_bool() warn: incrementing NULL pointer 'opptr' rl='1295896293336907776,2458024882201202688,3889929980400390144,6141874131669250048,6180548506814574592,6275458169940578304,6344884530654208000'
arch/x86/lib/cmdline.c:167 __cmdline_find_option() warn: incrementing NULL pointer 'opptr' rl='0-u64max'

regards,
dan carpenter