linux-kernel - Re: [PATCH] staging : rtl8188eu : remove unnecessary else

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1494264198.31950.59.camel@perches.com>
Date:   Mon, 08 May 2017 10:23:18 -0700
From:   Joe Perches <joe@...ches.com>
To:     Surender Polsani <surenderpolsani@...il.com>,
        gregkh@...uxfoundation.org
Cc:     devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging : rtl8188eu : remove unnecessary else

On Mon, 2017-05-08 at 14:13 +0530, Surender Polsani wrote:
> according to coding style else is not generally
> useful after a break or return
> 
> Signed-off-by: Surender Polsani <surenderpolsani@...il.com>
> ---
>  drivers/staging/rtl8188eu/hal/rtl8188e_dm.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c b/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c
> index d04b7fb..7420f55 100644
> --- a/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c
> +++ b/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c
> @@ -212,8 +212,7 @@ u8 rtw_hal_antdiv_before_linked(struct adapter *Adapter)
>  
>  		rtw_antenna_select_cmd(Adapter, dm_swat_tbl->CurAntenna, false);
>  		return true;
> -	} else {
> +	}
>  		dm_swat_tbl->SWAS_NoLink_State = 0;
>  		return false;
> -	}
>  }

It's just fine how else after return is used here.

In the future you should reflow indentation after
removing braces though.

The function might be restructured a bit to return
false before ever returning true.

Something like:
---
 drivers/staging/rtl8188eu/hal/rtl8188e_dm.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c b/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c
index d04b7fbb71e1..1de9bcaa58b8 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c
@@ -198,22 +198,22 @@ u8 rtw_hal_antdiv_before_linked(struct adapter *Adapter)
 	struct sw_ant_switch *dm_swat_tbl = &dm_odm->DM_SWAT_Table;
 	struct mlme_priv *pmlmepriv = &(Adapter->mlmepriv);
 
-	/*  Condition that does not need to use antenna diversity. */
+	/* Condition that does not need to use antenna diversity */
 	if (Adapter->HalData->AntDivCfg == 0)
 		return false;
 
 	if (check_fwstate(pmlmepriv, _FW_LINKED))
 		return false;
 
-	if (dm_swat_tbl->SWAS_NoLink_State == 0) {
-		/* switch channel */
-		dm_swat_tbl->SWAS_NoLink_State = 1;
-		dm_swat_tbl->CurAntenna = (dm_swat_tbl->CurAntenna == Antenna_A) ? Antenna_B : Antenna_A;
-
-		rtw_antenna_select_cmd(Adapter, dm_swat_tbl->CurAntenna, false);
-		return true;
-	} else {
+	if (dm_swat_tbl->SWAS_NoLink_State != 0) {
 		dm_swat_tbl->SWAS_NoLink_State = 0;
 		return false;
 	}
+
+	/* switch channel */
+	dm_swat_tbl->SWAS_NoLink_State = 1;
+	dm_swat_tbl->CurAntenna = dm_swat_tbl->CurAntenna == Antenna_A ? Antenna_B : Antenna_A;
+	rtw_antenna_select_cmd(Adapter, dm_swat_tbl->CurAntenna, false);
+
+	return true;
 }