[PATCH] Staging: rtl8192u: Remove else after return

[PATCH] Staging: rtl8192u: Remove else after return

[Ksenija Stanojevic]

This patch simplifies the code by removing else and fixes
the following checkpatch.pl warning: "else is not useful after
break or return".

Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
---
 drivers/staging/rtl8192u/r819xU_phy.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8192u/r819xU_phy.c b/drivers/staging/rtl8192u/r819xU_phy.c
index e210664..00be1fd 100644
--- a/drivers/staging/rtl8192u/r819xU_phy.c
+++ b/drivers/staging/rtl8192u/r819xU_phy.c
@@ -1363,11 +1363,11 @@ static u8 rtl8192_phy_SwChnlStepByStep(struct net_device *dev, u8 channel,
 			if ((*stage) == 2) {
 				(*delay) = CurrentCmd->msDelay;
 				return true;
-			} else {
-				(*stage)++;
-				(*step) = 0;
-				continue;
 			}
+			(*stage)++;
+			(*step) = 0;
+			continue;
+
 		}
 
 		switch (CurrentCmd->CmdID) {
-- 
1.9.1

Re: [Outreachy kernel] [PATCH] Staging: rtl8192u: Remove else after return

[Jes Sorensen]

On 02/20/15 13:31, Ksenija Stanojevic wrote:
> This patch simplifies the code by removing else and fixes
> the following checkpatch.pl warning: "else is not useful after
> break or return".
> 
> Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
> ---
>  drivers/staging/rtl8192u/r819xU_phy.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192u/r819xU_phy.c b/drivers/staging/rtl8192u/r819xU_phy.c
> index e210664..00be1fd 100644
> --- a/drivers/staging/rtl8192u/r819xU_phy.c
> +++ b/drivers/staging/rtl8192u/r819xU_phy.c
> @@ -1363,11 +1363,11 @@ static u8 rtl8192_phy_SwChnlStepByStep(struct net_device *dev, u8 channel,
>  			if ((*stage) == 2) {
>  				(*delay) = CurrentCmd->msDelay;
>  				return true;
> -			} else {
> -				(*stage)++;
> -				(*step) = 0;
> -				continue;
>  			}
> +			(*stage)++;
> +			(*step) = 0;
> +			continue;
> +
>  		}
>  
>  		switch (CurrentCmd->CmdID) {
> 

There is one issue with your patch, you add a blank line after continue,
that one needs to be removed.

rtl8192_phy_SwChnlStepByStep() is an utter mess, and given that the
function is huge and have a lot of return points, one either has to take
the approach you have chosen or fix them all up. I just told Aybuke
Ozdemir to use a goto to fix a similar case, so I wanted to point out
that here I think your solution makes sense.

There is another bad case with this function in that they return
true/false in a u8 variable, ie. the variable and the values assigned to
it do not match. This would call for a separate patch though.

Cheers,
Jes

Re: [Outreachy kernel] [PATCH] Staging: rtl8192u: Remove else after return

[Ksenija Stanojević]

On Fri, Feb 20, 2015 at 8:04 PM, Jes Sorensen <jes.sorensen@gmail.com> wrote:
> On 02/20/15 13:31, Ksenija Stanojevic wrote:
>> This patch simplifies the code by removing else and fixes
>> the following checkpatch.pl warning: "else is not useful after
>> break or return".
>>
>> Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
>> ---
>>  drivers/staging/rtl8192u/r819xU_phy.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/staging/rtl8192u/r819xU_phy.c b/drivers/staging/rtl8192u/r819xU_phy.c
>> index e210664..00be1fd 100644
>> --- a/drivers/staging/rtl8192u/r819xU_phy.c
>> +++ b/drivers/staging/rtl8192u/r819xU_phy.c
>> @@ -1363,11 +1363,11 @@ static u8 rtl8192_phy_SwChnlStepByStep(struct net_device *dev, u8 channel,
>>                       if ((*stage) == 2) {
>>                               (*delay) = CurrentCmd->msDelay;
>>                               return true;
>> -                     } else {
>> -                             (*stage)++;
>> -                             (*step) = 0;
>> -                             continue;
>>                       }
>> +                     (*stage)++;
>> +                     (*step) = 0;
>> +                     continue;
>> +
>>               }
>>
>>               switch (CurrentCmd->CmdID) {
>>
>
> There is one issue with your patch, you add a blank line after continue,
> that one needs to be removed.
>
> rtl8192_phy_SwChnlStepByStep() is an utter mess, and given that the
> function is huge and have a lot of return points, one either has to take
> the approach you have chosen or fix them all up. I just told Aybuke
> Ozdemir to use a goto to fix a similar case, so I wanted to point out
> that here I think your solution makes sense.
>
> There is another bad case with this function in that they return
> true/false in a u8 variable, ie. the variable and the values assigned to
> it do not match. This would call for a separate patch though.
>
> Cheers,
> Jes
>
Hi,

I can not find any variable that  uses true/false in this function.
Can you please explain in more detail?