* [PATCH] Staging: rtl8192u: ieee80211_rx: Remove unnecessary code @ 2015-10-14 17:18 Shivani Bhardwaj 2015-10-15 6:50 ` [Outreachy kernel] " Sudip Mukherjee 0 siblings, 1 reply; 8+ messages in thread From: Shivani Bhardwaj @ 2015-10-14 17:18 UTC (permalink / raw) To: outreachy-kernel The variable frame_authorized is declared and mentioned in the code but is not used anywhere so, it should be removed. Also, two case statements are merged by removing an extra break. Semantic patch used: @@ type T; identifier i; constant C; position p != e.p; @@ - T i@p; <+... when != i - i = C; ...+> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com> --- drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c index 4b5f32d..130c852 100644 --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c @@ -899,7 +899,6 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb, struct net_device *wds = NULL; struct sk_buff *skb2 = NULL; struct net_device *wds = NULL; - int frame_authorized = 0; int from_assoc_ap = 0; void *sta = NULL; #endif @@ -1106,10 +1105,7 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb, switch (hostap_handle_sta_rx(ieee, dev, skb, rx_stats, wds != NULL)) { case AP_RX_CONTINUE_NOT_AUTHORIZED: - frame_authorized = 0; - break; case AP_RX_CONTINUE: - frame_authorized = 1; break; case AP_RX_DROP: goto rx_dropped; -- 2.1.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Outreachy kernel] [PATCH] Staging: rtl8192u: ieee80211_rx: Remove unnecessary code 2015-10-14 17:18 [PATCH] Staging: rtl8192u: ieee80211_rx: Remove unnecessary code Shivani Bhardwaj @ 2015-10-15 6:50 ` Sudip Mukherjee 2015-10-15 6:52 ` Shivani Bhardwaj 0 siblings, 1 reply; 8+ messages in thread From: Sudip Mukherjee @ 2015-10-15 6:50 UTC (permalink / raw) To: Shivani Bhardwaj; +Cc: outreachy-kernel On Wed, Oct 14, 2015 at 10:48:50PM +0530, Shivani Bhardwaj wrote: > The variable frame_authorized is declared and mentioned in the code but > is not used anywhere so, it should be removed. Also, two case statements > are merged by removing an extra break. <snip> > @@ -1106,10 +1105,7 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb, > switch (hostap_handle_sta_rx(ieee, dev, skb, rx_stats, > wds != NULL)) { > case AP_RX_CONTINUE_NOT_AUTHORIZED: > - frame_authorized = 0; > - break; > case AP_RX_CONTINUE: > - frame_authorized = 1; > break; Just a doubt, it now becomes: case AP_RX_CONTINUE_NOT_AUTHORIZED: case AP_RX_CONTINUE: break; If these two cases are not doing anything then what is the use of having them here in the switch-case? regards sudip ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Outreachy kernel] [PATCH] Staging: rtl8192u: ieee80211_rx: Remove unnecessary code 2015-10-15 6:50 ` [Outreachy kernel] " Sudip Mukherjee @ 2015-10-15 6:52 ` Shivani Bhardwaj 2015-10-15 7:01 ` Sudip Mukherjee 0 siblings, 1 reply; 8+ messages in thread From: Shivani Bhardwaj @ 2015-10-15 6:52 UTC (permalink / raw) To: Sudip Mukherjee; +Cc: outreachy-kernel On Thu, Oct 15, 2015 at 12:20 PM, Sudip Mukherjee <sudipm.mukherjee@gmail.com> wrote: > On Wed, Oct 14, 2015 at 10:48:50PM +0530, Shivani Bhardwaj wrote: >> The variable frame_authorized is declared and mentioned in the code but >> is not used anywhere so, it should be removed. Also, two case statements >> are merged by removing an extra break. > <snip> >> @@ -1106,10 +1105,7 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb, >> switch (hostap_handle_sta_rx(ieee, dev, skb, rx_stats, >> wds != NULL)) { >> case AP_RX_CONTINUE_NOT_AUTHORIZED: >> - frame_authorized = 0; >> - break; >> case AP_RX_CONTINUE: >> - frame_authorized = 1; >> break; > > Just a doubt, it now becomes: > case AP_RX_CONTINUE_NOT_AUTHORIZED: > case AP_RX_CONTINUE: > break; > > If these two cases are not doing anything then what is the use of having > them here in the switch-case? > > regards > sudip Because we don't have a default case here and the variable in switch might still get these values based on some other code. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Outreachy kernel] [PATCH] Staging: rtl8192u: ieee80211_rx: Remove unnecessary code 2015-10-15 6:52 ` Shivani Bhardwaj @ 2015-10-15 7:01 ` Sudip Mukherjee 2015-10-15 7:07 ` Shivani Bhardwaj 2015-10-15 16:08 ` Julia Lawall 0 siblings, 2 replies; 8+ messages in thread From: Sudip Mukherjee @ 2015-10-15 7:01 UTC (permalink / raw) To: Shivani Bhardwaj; +Cc: outreachy-kernel On Thu, Oct 15, 2015 at 12:22:35PM +0530, Shivani Bhardwaj wrote: > On Thu, Oct 15, 2015 at 12:20 PM, Sudip Mukherjee > <sudipm.mukherjee@gmail.com> wrote: > > On Wed, Oct 14, 2015 at 10:48:50PM +0530, Shivani Bhardwaj wrote: > >> The variable frame_authorized is declared and mentioned in the code but > >> is not used anywhere so, it should be removed. Also, two case statements > >> are merged by removing an extra break. > > <snip> > >> @@ -1106,10 +1105,7 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb, > >> switch (hostap_handle_sta_rx(ieee, dev, skb, rx_stats, > >> wds != NULL)) { > >> case AP_RX_CONTINUE_NOT_AUTHORIZED: > >> - frame_authorized = 0; > >> - break; > >> case AP_RX_CONTINUE: > >> - frame_authorized = 1; > >> break; > > > > Just a doubt, it now becomes: > > case AP_RX_CONTINUE_NOT_AUTHORIZED: > > case AP_RX_CONTINUE: > > break; > > > > If these two cases are not doing anything then what is the use of having > > them here in the switch-case? > > > > regards > > sudip > > Because we don't have a default case here and the variable in switch > might still get these values based on some other code. I think default case is to handle all cases which are not given by case statement. What will happen if variable gets these values but these cases are not mentioned? switch-case will see that these cases are not mentioned and will exit the switch-case. In the situation where these cases are mentioned, switch-case will see these cases exist, will come to these cases and breaks out of switch-case. Am I understanding anything wrong here? regards sudip ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Outreachy kernel] [PATCH] Staging: rtl8192u: ieee80211_rx: Remove unnecessary code 2015-10-15 7:01 ` Sudip Mukherjee @ 2015-10-15 7:07 ` Shivani Bhardwaj 2015-10-15 7:19 ` Sudip Mukherjee 2015-10-15 16:08 ` Julia Lawall 1 sibling, 1 reply; 8+ messages in thread From: Shivani Bhardwaj @ 2015-10-15 7:07 UTC (permalink / raw) To: Sudip Mukherjee; +Cc: outreachy-kernel On Thu, Oct 15, 2015 at 12:31 PM, Sudip Mukherjee <sudipm.mukherjee@gmail.com> wrote: > On Thu, Oct 15, 2015 at 12:22:35PM +0530, Shivani Bhardwaj wrote: >> On Thu, Oct 15, 2015 at 12:20 PM, Sudip Mukherjee >> <sudipm.mukherjee@gmail.com> wrote: >> > On Wed, Oct 14, 2015 at 10:48:50PM +0530, Shivani Bhardwaj wrote: >> >> The variable frame_authorized is declared and mentioned in the code but >> >> is not used anywhere so, it should be removed. Also, two case statements >> >> are merged by removing an extra break. >> > <snip> >> >> @@ -1106,10 +1105,7 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb, >> >> switch (hostap_handle_sta_rx(ieee, dev, skb, rx_stats, >> >> wds != NULL)) { >> >> case AP_RX_CONTINUE_NOT_AUTHORIZED: >> >> - frame_authorized = 0; >> >> - break; >> >> case AP_RX_CONTINUE: >> >> - frame_authorized = 1; >> >> break; >> > >> > Just a doubt, it now becomes: >> > case AP_RX_CONTINUE_NOT_AUTHORIZED: >> > case AP_RX_CONTINUE: >> > break; >> > >> > If these two cases are not doing anything then what is the use of having >> > them here in the switch-case? >> > >> > regards >> > sudip >> >> Because we don't have a default case here and the variable in switch >> might still get these values based on some other code. > > I think default case is to handle all cases which are not given by case > statement. What will happen if variable gets these values but these > cases are not mentioned? switch-case will see that these cases are not > mentioned and will exit the switch-case. In the situation where these > cases are mentioned, switch-case will see these cases exist, will come > to these cases and breaks out of switch-case. > Am I understanding anything wrong here? > > regards > sudip No, you are completely right. :) So, should I remove these cases? And if these cases are to be removed there won't be a need of switch case at all. Or should I leave it thinking that programmer might have to make some changes? Thank you. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Outreachy kernel] [PATCH] Staging: rtl8192u: ieee80211_rx: Remove unnecessary code 2015-10-15 7:07 ` Shivani Bhardwaj @ 2015-10-15 7:19 ` Sudip Mukherjee 2015-10-15 7:43 ` Shivani Bhardwaj 0 siblings, 1 reply; 8+ messages in thread From: Sudip Mukherjee @ 2015-10-15 7:19 UTC (permalink / raw) To: Shivani Bhardwaj; +Cc: outreachy-kernel On Thu, Oct 15, 2015 at 12:37:42PM +0530, Shivani Bhardwaj wrote: > On Thu, Oct 15, 2015 at 12:31 PM, Sudip Mukherjee > <sudipm.mukherjee@gmail.com> wrote: > > On Thu, Oct 15, 2015 at 12:22:35PM +0530, Shivani Bhardwaj wrote: > >> On Thu, Oct 15, 2015 at 12:20 PM, Sudip Mukherjee > >> <sudipm.mukherjee@gmail.com> wrote: > >> > On Wed, Oct 14, 2015 at 10:48:50PM +0530, Shivani Bhardwaj wrote: > >> >> The variable frame_authorized is declared and mentioned in the code but > >> >> is not used anywhere so, it should be removed. Also, two case statements > >> >> are merged by removing an extra break. > >> > <snip> > >> >> @@ -1106,10 +1105,7 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb, > >> >> switch (hostap_handle_sta_rx(ieee, dev, skb, rx_stats, > >> >> wds != NULL)) { > >> >> case AP_RX_CONTINUE_NOT_AUTHORIZED: > >> >> - frame_authorized = 0; > >> >> - break; > >> >> case AP_RX_CONTINUE: > >> >> - frame_authorized = 1; > >> >> break; > >> > > >> > Just a doubt, it now becomes: > >> > case AP_RX_CONTINUE_NOT_AUTHORIZED: > >> > case AP_RX_CONTINUE: > >> > break; > >> > > >> > If these two cases are not doing anything then what is the use of having > >> > them here in the switch-case? > >> > > >> > regards > >> > sudip > >> > >> Because we don't have a default case here and the variable in switch > >> might still get these values based on some other code. > > > > I think default case is to handle all cases which are not given by case > > statement. What will happen if variable gets these values but these > > cases are not mentioned? switch-case will see that these cases are not > > mentioned and will exit the switch-case. In the situation where these > > cases are mentioned, switch-case will see these cases exist, will come > > to these cases and breaks out of switch-case. > > Am I understanding anything wrong here? > > > > regards > > sudip > > No, you are completely right. :) So, should I remove these cases? And > if these cases are to be removed there won't be a need of switch case > at all. Or should I leave it thinking that programmer might have to > make some changes? The main change required for this driver will be to use mac80211, so that means all these codes regarding ieee80211 has to be removed one day. So, I guess you can remove these cases. Anyways, lets wait for Greg's comment then change if required. regards sudip ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Outreachy kernel] [PATCH] Staging: rtl8192u: ieee80211_rx: Remove unnecessary code 2015-10-15 7:19 ` Sudip Mukherjee @ 2015-10-15 7:43 ` Shivani Bhardwaj 0 siblings, 0 replies; 8+ messages in thread From: Shivani Bhardwaj @ 2015-10-15 7:43 UTC (permalink / raw) To: Sudip Mukherjee; +Cc: outreachy-kernel On Thu, Oct 15, 2015 at 12:49 PM, Sudip Mukherjee <sudipm.mukherjee@gmail.com> wrote: > On Thu, Oct 15, 2015 at 12:37:42PM +0530, Shivani Bhardwaj wrote: >> On Thu, Oct 15, 2015 at 12:31 PM, Sudip Mukherjee >> <sudipm.mukherjee@gmail.com> wrote: >> > On Thu, Oct 15, 2015 at 12:22:35PM +0530, Shivani Bhardwaj wrote: >> >> On Thu, Oct 15, 2015 at 12:20 PM, Sudip Mukherjee >> >> <sudipm.mukherjee@gmail.com> wrote: >> >> > On Wed, Oct 14, 2015 at 10:48:50PM +0530, Shivani Bhardwaj wrote: >> >> >> The variable frame_authorized is declared and mentioned in the code but >> >> >> is not used anywhere so, it should be removed. Also, two case statements >> >> >> are merged by removing an extra break. >> >> > <snip> >> >> >> @@ -1106,10 +1105,7 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb, >> >> >> switch (hostap_handle_sta_rx(ieee, dev, skb, rx_stats, >> >> >> wds != NULL)) { >> >> >> case AP_RX_CONTINUE_NOT_AUTHORIZED: >> >> >> - frame_authorized = 0; >> >> >> - break; >> >> >> case AP_RX_CONTINUE: >> >> >> - frame_authorized = 1; >> >> >> break; >> >> > >> >> > Just a doubt, it now becomes: >> >> > case AP_RX_CONTINUE_NOT_AUTHORIZED: >> >> > case AP_RX_CONTINUE: >> >> > break; >> >> > >> >> > If these two cases are not doing anything then what is the use of having >> >> > them here in the switch-case? >> >> > >> >> > regards >> >> > sudip >> >> >> >> Because we don't have a default case here and the variable in switch >> >> might still get these values based on some other code. >> > >> > I think default case is to handle all cases which are not given by case >> > statement. What will happen if variable gets these values but these >> > cases are not mentioned? switch-case will see that these cases are not >> > mentioned and will exit the switch-case. In the situation where these >> > cases are mentioned, switch-case will see these cases exist, will come >> > to these cases and breaks out of switch-case. >> > Am I understanding anything wrong here? >> > >> > regards >> > sudip >> >> No, you are completely right. :) So, should I remove these cases? And >> if these cases are to be removed there won't be a need of switch case >> at all. Or should I leave it thinking that programmer might have to >> make some changes? > > The main change required for this driver will be to use mac80211, so > that means all these codes regarding ieee80211 has to be removed one > day. So, I guess you can remove these cases. Anyways, lets wait for > Greg's comment then change if required. > > regards > sudip All right. Thank you. :) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Outreachy kernel] [PATCH] Staging: rtl8192u: ieee80211_rx: Remove unnecessary code 2015-10-15 7:01 ` Sudip Mukherjee 2015-10-15 7:07 ` Shivani Bhardwaj @ 2015-10-15 16:08 ` Julia Lawall 1 sibling, 0 replies; 8+ messages in thread From: Julia Lawall @ 2015-10-15 16:08 UTC (permalink / raw) To: Sudip Mukherjee; +Cc: Shivani Bhardwaj, outreachy-kernel On Thu, 15 Oct 2015, Sudip Mukherjee wrote: > On Thu, Oct 15, 2015 at 12:22:35PM +0530, Shivani Bhardwaj wrote: > > On Thu, Oct 15, 2015 at 12:20 PM, Sudip Mukherjee > > <sudipm.mukherjee@gmail.com> wrote: > > > On Wed, Oct 14, 2015 at 10:48:50PM +0530, Shivani Bhardwaj wrote: > > >> The variable frame_authorized is declared and mentioned in the code but > > >> is not used anywhere so, it should be removed. Also, two case statements > > >> are merged by removing an extra break. > > > <snip> > > >> @@ -1106,10 +1105,7 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb, > > >> switch (hostap_handle_sta_rx(ieee, dev, skb, rx_stats, > > >> wds != NULL)) { > > >> case AP_RX_CONTINUE_NOT_AUTHORIZED: > > >> - frame_authorized = 0; > > >> - break; > > >> case AP_RX_CONTINUE: > > >> - frame_authorized = 1; > > >> break; > > > > > > Just a doubt, it now becomes: > > > case AP_RX_CONTINUE_NOT_AUTHORIZED: > > > case AP_RX_CONTINUE: > > > break; > > > > > > If these two cases are not doing anything then what is the use of having > > > them here in the switch-case? > > > > > > regards > > > sudip > > > > Because we don't have a default case here and the variable in switch > > might still get these values based on some other code. > > I think default case is to handle all cases which are not given by case > statement. What will happen if variable gets these values but these > cases are not mentioned? switch-case will see that these cases are not > mentioned and will exit the switch-case. In the situation where these > cases are mentioned, switch-case will see these cases exist, will come > to these cases and breaks out of switch-case. > Am I understanding anything wrong here? If there is no default and a case is not mentioned, then nothing should happen on the switch. I guess one could test this easily, if there are any doubts. Maybe having cases that do nothing protects against adding a default that does something? julia ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-10-15 16:08 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-10-14 17:18 [PATCH] Staging: rtl8192u: ieee80211_rx: Remove unnecessary code Shivani Bhardwaj 2015-10-15 6:50 ` [Outreachy kernel] " Sudip Mukherjee 2015-10-15 6:52 ` Shivani Bhardwaj 2015-10-15 7:01 ` Sudip Mukherjee 2015-10-15 7:07 ` Shivani Bhardwaj 2015-10-15 7:19 ` Sudip Mukherjee 2015-10-15 7:43 ` Shivani Bhardwaj 2015-10-15 16:08 ` Julia Lawall
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.