Staging: rtl8192u: ieee80211_rx: Remove unnecessary code

PATCH In review Shivani Bhardwaj 3 participants 8 messages Oct 14, 2015 – Oct 15, 2015 Staging

Share & tools

Shivani Bhardwaj

author

2015-10-14 17:19 UTC #0 lore raw

PATCH 1/1 Staging: rtl8192u: ieee80211_rx: Remove unnecessary code

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 <[email protected]>

---
 drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c | 4 ----
 1 file changed, 4 deletions(-)

drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c +0 -4

index 4b5f32d..130c852 100644
@@ -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;

Signature · 1 line

2.1.0

Sudip Mukherjee

2015-10-15 06:50 UTC #1 lore raw

Quoted 3 lines · Shivani Bhardwaj

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>

drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb, · line 1109

Quoted 9 lines

drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c +0 -3

@@ -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

Shivani Bhardwaj

author

2015-10-15 06:52 UTC #2 lore raw

Quoted 20 lines · Sudip Mukherjee

Quoted 3 lines · Shivani Bhardwaj
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>
drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb, · line 1109
Quoted 9 lines
drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c +0 -3
@@ -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;

Quoted 5 lines

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.

Sudip Mukherjee

2015-10-15 07:01 UTC #3 lore raw

Quoted 22 lines · Shivani Bhardwaj

Quoted 20 lines · Sudip Mukherjee
Quoted 3 lines · Shivani Bhardwaj
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>
drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb, · line 1109
Quoted 9 lines
drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c +0 -3
@@ -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;

Quoted 8 lines

Quoted 5 lines
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

Shivani Bhardwaj

author

2015-10-15 07:07 UTC #4 lore raw

Quoted 40 lines · Sudip Mukherjee

Quoted 22 lines · Shivani Bhardwaj
Quoted 20 lines · Sudip Mukherjee
Quoted 3 lines · Shivani Bhardwaj
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>
drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb, · line 1109
Quoted 9 lines
drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c +0 -3
@@ -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;
Quoted 8 lines
Quoted 5 lines
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?

Quoted 2 lines

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.

Sudip Mukherjee

2015-10-15 07:19 UTC #5 lore raw

Quoted 42 lines · Shivani Bhardwaj

Quoted 40 lines · Sudip Mukherjee
Quoted 22 lines · Shivani Bhardwaj
Quoted 20 lines · Sudip Mukherjee
Quoted 3 lines · Shivani Bhardwaj
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>
drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb, · line 1109
Quoted 9 lines
drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c +0 -3
@@ -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;
Quoted 8 lines
Quoted 5 lines
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?

Quoted 7 lines

Quoted 2 lines
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

Shivani Bhardwaj

author

2015-10-15 07:43 UTC #6 lore raw

Quoted 51 lines · Sudip Mukherjee

Quoted 42 lines · Shivani Bhardwaj
Quoted 40 lines · Sudip Mukherjee
Quoted 22 lines · Shivani Bhardwaj
Quoted 20 lines · Sudip Mukherjee
Quoted 3 lines · Shivani Bhardwaj
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>
drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb, · line 1109
Quoted 9 lines
drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c +0 -3
@@ -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;
Quoted 8 lines
Quoted 5 lines
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?
Quoted 7 lines
Quoted 2 lines
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?

Quoted 7 lines

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. :)

Julia Lawall

2015-10-15 16:08 UTC #7 lore raw

In reply to Sudip Mukherjee

Quoted 32 lines · Sudip Mukherjee

Quoted 22 lines · Shivani Bhardwaj
Quoted 20 lines · Sudip Mukherjee
Quoted 3 lines · Shivani Bhardwaj
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>
drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb, · line 1109
Quoted 9 lines
drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c +0 -3
@@ -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;
Quoted 8 lines
Quoted 5 lines
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.

Quoted 7 lines

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