[PATCH v2] staging: rtl8712: remove unused variable from rtl871x_mlme.c

[PATCH v2] staging: rtl8712: remove unused variable from rtl871x_mlme.c

[Sergei Krainov]

Remove unused variable from rtl871x_mlme.c.

No side effects can be seen locally or in r8712_find_network()

Signed-off-by: Sergei Krainov <sergei.krainov.lkd@gmail.com>
---
Changes from v1: Rechecked if deleted code doesn't have any
side effects and added description that no observable
side effects were found
 drivers/staging/rtl8712/rtl871x_mlme.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c
index 8a97307fbbd6..4f41e321ea63 100644
--- a/drivers/staging/rtl8712/rtl871x_mlme.c
+++ b/drivers/staging/rtl8712/rtl871x_mlme.c
@@ -656,7 +656,7 @@ void r8712_joinbss_event_callback(struct _adapter *adapter, u8 *pbuf)
 	struct sta_priv	*pstapriv = &adapter->stapriv;
 	struct mlme_priv	*pmlmepriv = &adapter->mlmepriv;
 	struct wlan_network	*cur_network = &pmlmepriv->cur_network;
-	struct wlan_network	*pcur_wlan = NULL, *ptarget_wlan = NULL;
+	struct wlan_network	*ptarget_wlan = NULL;
 	unsigned int		the_same_macaddr = false;
 	struct wlan_network *pnetwork;
 
@@ -721,13 +721,6 @@ void r8712_joinbss_event_callback(struct _adapter *adapter, u8 *pbuf)
 					    scanned_queue,
 					    cur_network->network.MacAddress);
 				} else {
-					pcur_wlan =
-					     r8712_find_network(&pmlmepriv->
-					     scanned_queue,
-					     cur_network->network.MacAddress);
-					if (pcur_wlan)
-						pcur_wlan->fixed = false;
-
 					pcur_sta = r8712_get_stainfo(pstapriv,
 					     cur_network->network.MacAddress);
 					spin_lock_irqsave(&pstapriv->
-- 
2.25.1

Re: [PATCH v2] staging: rtl8712: remove unused variable from rtl871x_mlme.c

[Edmundo Carmona Antoranz]

On Thu, Apr 8, 2021 at 6:10 AM Sergei Krainov
<sergei.krainov.lkd@gmail.com> wrote:
> No side effects can be seen locally or in r8712_find_network()

I am sorry to jump in. Sergei, what Greg is asking is basically why
you want to delete the r8712_find_network call in the first place.
Deleting an unused variable is fine, but the problem here is that you
are _also_ deleting a call to a function that _probably_ does some
things that, even if you want to get rid of the variable, you would
probably like to keep on doing (and so the call would remain). Is that
call really not doing anything relevant? That's what you will have to
explain in the patch in order for it to make sense.

As a side note on top of the question about the call, it's not like
the variable is not being used. It's used right after the call to
r8712_find_network to check the result of the call... so is the real
purpose of the patch to remove the call in the first place and then
having the variable removed because there is no point in having it if
the call goes away?

I hope that helps.

Re: [PATCH v2] staging: rtl8712: remove unused variable from rtl871x_mlme.c

[Edmundo Carmona Antoranz]

On Thu, Apr 8, 2021 at 6:10 AM Sergei Krainov
<sergei.krainov.lkd@gmail.com> wrote:
> No side effects can be seen locally or in r8712_find_network()

I am sorry to jump in. Sergei, what Greg is asking is basically why
you want to delete the r8712_find_network call in the first place.
Deleting an unused variable is fine, but the problem here is that you
are _also_ deleting a call to a function that _probably_ does some
things that, even if you want to get rid of the variable, you would
probably like to keep on doing (and so the call would remain). Is that
call really not doing anything relevant? That's what you will have to
explain in the patch in order for it to make sense.

As a side note on top of the question about the call, it's not like
the variable is not being used. It's used right after the call to
r8712_find_network to check the result of the call... so is the real
purpose of the patch to remove the call in the first place and then
having the variable removed because there is no point in having it if
the call goes away?

I hope that helps.

Re: [PATCH v2] staging: rtl8712: remove unused variable from rtl871x_mlme.c

[Sergei Krainov]

On Thu, Apr 08, 2021 at 08:29:00AM -0600, Edmundo Carmona Antoranz wrote:
> On Thu, Apr 8, 2021 at 6:10 AM Sergei Krainov
> <sergei.krainov.lkd@gmail.com> wrote:
> > No side effects can be seen locally or in r8712_find_network()
> 
> I am sorry to jump in. Sergei, what Greg is asking is basically why
> you want to delete the r8712_find_network call in the first place.
> Deleting an unused variable is fine, but the problem here is that you
> are _also_ deleting a call to a function that _probably_ does some
> things that, even if you want to get rid of the variable, you would
> probably like to keep on doing (and so the call would remain). Is that
> call really not doing anything relevant? That's what you will have to
> explain in the patch in order for it to make sense.
> 
> As a side note on top of the question about the call, it's not like
> the variable is not being used. It's used right after the call to
> r8712_find_network to check the result of the call... so is the real
> purpose of the patch to remove the call in the first place and then
> having the variable removed because there is no point in having it if
> the call goes away?
> 
> I hope that helps.
Thank you for clarification, guess I really misunderstood the question
and didn't explain properly why I'm doing it.

In this block of code call to r8712_find_network() exist only for one
purpose, to return value to the pcur_wlan. And after that we're not
using pcur_wlan.

So in my opinion it looks like a very subtle bug where we have unused
variable, which is allocated by r8712_find_network(), and if that
succeeds we're also modifying value by pcur_wlan->fixed = false;.
And after all that we're not using variable and compiler has no chance
of catching that because of what we're doing with that value.

Please correct me if I'm wrong in something, I just found that
questionable behavior and decided to send patch, so someone can see
it and say if I'm wrong or not. In case I'm right this change can be
_possibly_ accepted.

Also sorry for asking here, but is it okay that my commit has [PATCH v2]
and subject is [PATCH v2] in mutt, but in mailing list I still see
[PATCH]?

Greg, thanks a lot for reviews of my code you did in past few days, I
really appreciate that, but just didn't want to flood mailing list with
my appreciation only.