staging: rtl8723bs: simplify control flow

Checkpatch issues "WARNING: else is not generally useful after a break or return" for the following code:

while (1) {
	do_join_r = rtw_do_join(padapter);
	if (do_join_r == _SUCCESS) {
		break;
	} else {
		rtw_dec_to_roam(padapter);

		if (rtw_to_roam(padapter) > 0) {
			continue;
		} else {
			rtw_indicate_disconnect(padapter);
			break;
		}
	}
}

We simplify this code in multiple steps. First, we remove do_join_r

variable because it is only used right after it is assigned. Second, we remove the unnecessary else statement right after break:

while (1) {
	if (rtw_do_join(padapter) == _SUCCESS)
		break;
	rtw_dec_to_roam(padapter);

	if (rtw_to_roam(padapter) > 0) {
		continue;
	} else {
		rtw_indicate_disconnect(padapter);
		break;
	}
}

Next, we move the call to rtw_do_join into the while test because the while will loop only until the call is successful:

while (rtw_do_join(padapter) != _SUCCESS) {
	rtw_dec_to_roam(padapter);
	if (rtw_to_roam(padapter) > 0) {
		continue;
	} else {
		rtw_indicate_disconnect(padapter);
		break;
	}
}

Finally, looking at the code above, it is clear that the code will break out of the loop if rtw_to_roam call is <= 0. Hence:

while (rtw_do_join(padapter) != _SUCCESS) {
	rtw_dec_to_roam(padapter);
	if (rtw_to_roam(padapter) <= 0) {
		rtw_indicate_disconnect(padapter);
		break;
	}
}

Signed-off-by Sevinj Aghayeva <[email protected]>

---
 drivers/staging/rtl8723bs/core/rtw_mlme.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

drivers/staging/rtl8723bs/core/rtw_mlme.c +4 -14

index 3eacf8f9d236..a45df775d535 100644
@@ -2594,30 +2594,20 @@ void _rtw_roaming(struct adapter *padapter, struct wlan_network *tgt_network)
 {
 	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
 	struct wlan_network *cur_network = &pmlmepriv->cur_network;
-	int do_join_r;
 
 	if (rtw_to_roam(padapter) > 0) {
 		memcpy(&pmlmepriv->assoc_ssid, &cur_network->network.ssid, sizeof(struct ndis_802_11_ssid));
 
 		pmlmepriv->assoc_by_bssid = false;
 
-		while (1) {
-			do_join_r = rtw_do_join(padapter);
-			if (do_join_r == _SUCCESS) {
+		while (rtw_do_join(padapter) != _SUCCESS) {
+			rtw_dec_to_roam(padapter);
+			if (rtw_to_roam(padapter) <= 0) {
+				rtw_indicate_disconnect(padapter);
 				break;
-			} else {
-				rtw_dec_to_roam(padapter);
-
-				if (rtw_to_roam(padapter) > 0) {
-					continue;
-				} else {
-					rtw_indicate_disconnect(padapter);
-					break;
-				}
 			}
 		}
 	}
-
 }
 
 signed int rtw_linked_check(struct adapter *padapter)

Signature · 1 line

2.25.1

Quoted 66 lines · Ira Weiny

Quoted 19 lines · Sevinj Aghayeva

Checkpatch issues "WARNING: else is not generally useful after a break or return" for the following code:

while (1) {
      do_join_r = rtw_do_join(padapter);
      if (do_join_r == _SUCCESS) {
              break;
      } else {
              rtw_dec_to_roam(padapter);

              if (rtw_to_roam(padapter) > 0) {
                      continue;
              } else {
                      rtw_indicate_disconnect(padapter);
                      break;
              }
      }
}

Quoted 1 line

We simplify this code in multiple steps. First, we remove do_join_r

I can't say how Greg would like to see a change like this but my gut says that each of these steps should be a patch in a series...

Quoted 32 lines

variable because it is only used right after it is assigned. Second, we remove the unnecessary else statement right after break:

while (1) {
      if (rtw_do_join(padapter) == _SUCCESS)
              break;
      rtw_dec_to_roam(padapter);

      if (rtw_to_roam(padapter) > 0) {
              continue;
      } else {
              rtw_indicate_disconnect(padapter);
              break;
      }
}

Next, we move the call to rtw_do_join into the while test because the while will loop only until the call is successful:

while (rtw_do_join(padapter) != _SUCCESS) {
      rtw_dec_to_roam(padapter);
      if (rtw_to_roam(padapter) > 0) {
              continue;
      } else {
              rtw_indicate_disconnect(padapter);
              break;
      }
}

Finally, looking at the code above, it is clear that the code will break out of the loop if rtw_to_roam call is <= 0. Hence:

Quoted 7 lines

while (rtw_do_join(padapter) != _SUCCESS) {
      rtw_dec_to_roam(padapter);
      if (rtw_to_roam(padapter) <= 0) {
              rtw_indicate_disconnect(padapter);
              break;
      }
}

Quoted 2 lines

... that said, this commit message made reviewing the change much easier, thanks.

This has landed a week ago, but thanks for the review anyway!

Quoted 3 lines

Did you submit a patch for the r8188eu driver too? I just noticed it has a similar loop in _rtw_roaming().

Right, there is also another similar loop in the same file as well. I think I wasn't sure that this patch was going to get accepted since it was a big change, so I didn't fix the others before this one got it. In the meantime I hit slightly more than 10 cleanup patches and moved on to working with my mentors. But I can fix the other loops if you think I should do it.

Quoted 47 lines

Reviewed-by Ira Weiny <[email protected]>

Quoted 44 lines

Signed-off-by Sevinj Aghayeva <[email protected]>

---
 drivers/staging/rtl8723bs/core/rtw_mlme.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

drivers/staging/rtl8723bs/core/rtw_mlme.c +4 -14

index 3eacf8f9d236..a45df775d535 100644
@@ -2594,30 +2594,20 @@ void _rtw_roaming(struct adapter *padapter, struct wlan_network *tgt_network)
 {
      struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
      struct wlan_network *cur_network = &pmlmepriv->cur_network;
-     int do_join_r;
 
      if (rtw_to_roam(padapter) > 0) {
              memcpy(&pmlmepriv->assoc_ssid, &cur_network->network.ssid, sizeof(struct ndis_802_11_ssid));
 
              pmlmepriv->assoc_by_bssid = false;
 
-             while (1) {
-                     do_join_r = rtw_do_join(padapter);
-                     if (do_join_r == _SUCCESS) {
+             while (rtw_do_join(padapter) != _SUCCESS) {
+                     rtw_dec_to_roam(padapter);
+                     if (rtw_to_roam(padapter) <= 0) {
+                             rtw_indicate_disconnect(padapter);
                              break;
-                     } else {
-                             rtw_dec_to_roam(padapter);
-
-                             if (rtw_to_roam(padapter) > 0) {
-                                     continue;
-                             } else {
-                                     rtw_indicate_disconnect(padapter);
-                                     break;
-                             }
                      }
              }
      }
-
 }
 

Quoted 4 lines

Quoted 4 lines

drivers/staging/rtl8723bs/core/rtw_mlme.c +0 -1

 signed int rtw_linked_check(struct adapter *padapter)
--

2.25.1

--

Sevinj.Aghayeva