linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] staging: rtl8712: remove unused variable from rtl871x_mlme.c
@ 2021-04-08 12:09 Sergei Krainov
  2021-04-08 14:29 ` Edmundo Carmona Antoranz
  0 siblings, 1 reply; 3+ messages in thread
From: Sergei Krainov @ 2021-04-08 12:09 UTC (permalink / raw)
  To: Larry.Finger, florian.c.schilhabel, gregkh
  Cc: linux-staging, linux-kernel, kernel-janitors

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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] staging: rtl8712: remove unused variable from rtl871x_mlme.c
  2021-04-08 12:09 [PATCH v2] staging: rtl8712: remove unused variable from rtl871x_mlme.c Sergei Krainov
@ 2021-04-08 14:29 ` Edmundo Carmona Antoranz
  2021-04-08 15:29   ` Sergei Krainov
  0 siblings, 1 reply; 3+ messages in thread
From: Edmundo Carmona Antoranz @ 2021-04-08 14:29 UTC (permalink / raw)
  To: Sergei Krainov
  Cc: Larry Finger, florian.c.schilhabel, Greg Kroah-Hartman,
	linux-staging, linux-kernel, kernel-janitors

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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] staging: rtl8712: remove unused variable from rtl871x_mlme.c
  2021-04-08 14:29 ` Edmundo Carmona Antoranz
@ 2021-04-08 15:29   ` Sergei Krainov
  0 siblings, 0 replies; 3+ messages in thread
From: Sergei Krainov @ 2021-04-08 15:29 UTC (permalink / raw)
  To: Edmundo Carmona Antoranz
  Cc: Larry Finger, florian.c.schilhabel, Greg Kroah-Hartman,
	linux-staging, linux-kernel, kernel-janitors

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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-04-08 15:29 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-08 12:09 [PATCH v2] staging: rtl8712: remove unused variable from rtl871x_mlme.c Sergei Krainov
2021-04-08 14:29 ` Edmundo Carmona Antoranz
2021-04-08 15:29   ` Sergei Krainov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).