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

[Jes Sorensen <jes.sorensen@gmail.com>]

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