[PATCH v2] staging: r8188eu: os_dep: remove the goto statement

[PATCH v2] staging: r8188eu: os_dep: remove the goto statement

[Saurav Girepunje]

Remove the goto statement from rtw_init_drv_sw(). In this function goto
can be replace by return statement. As on goto label exit, function
only return it is not performing any cleanup. Avoiding goto will
improve the function readability.

Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>
---

ChangelogV2:

	-On V1 combined the if condition check for functions as below

	 if (!rtw_init_evt_priv(&padapter->evtpriv) || !rtw_init_mlme_priv(padapter))
		return _FAIL;
	reverting these changes and keeping them as-is. It will make more clear on
	individual function call if something need to handle for error case.

ChangelogV1:

	-Remove the goto statement from rtw_init_drv_sw(). In this function goto
	 can be replace by return statement. As on goto label exit, function
	 only return it is not performing any cleanup. Avoiding goto will
	 improve the function readability.

 drivers/staging/r8188eu/os_dep/os_intfs.c | 34 ++++++++---------------
 1 file changed, 11 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/r8188eu/os_dep/os_intfs.c b/drivers/staging/r8188eu/os_dep/os_intfs.c
index 1418c9c4916c..ec96e837ab88 100644
--- a/drivers/staging/r8188eu/os_dep/os_intfs.c
+++ b/drivers/staging/r8188eu/os_dep/os_intfs.c
@@ -480,48 +480,37 @@ u8 rtw_init_drv_sw(struct adapter *padapter)
 {
 	u8	ret8 = _SUCCESS;

-	if ((rtw_init_cmd_priv(&padapter->cmdpriv)) == _FAIL) {
-		ret8 = _FAIL;
-		goto exit;
-	}
+	if ((rtw_init_cmd_priv(&padapter->cmdpriv)) == _FAIL)
+		return _FAIL;

 	padapter->cmdpriv.padapter = padapter;

-	if ((rtw_init_evt_priv(&padapter->evtpriv)) == _FAIL) {
-		ret8 = _FAIL;
-		goto exit;
-	}
+	if ((rtw_init_evt_priv(&padapter->evtpriv)) == _FAIL)
+		return _FAIL;

-	if (rtw_init_mlme_priv(padapter) == _FAIL) {
-		ret8 = _FAIL;
-		goto exit;
-	}
+	if (rtw_init_mlme_priv(padapter) == _FAIL)
+		return _FAIL;

 	rtw_init_wifidirect_timers(padapter);
 	init_wifidirect_info(padapter, P2P_ROLE_DISABLE);
 	reset_global_wifidirect_info(padapter);

-	if (init_mlme_ext_priv(padapter) == _FAIL) {
-		ret8 = _FAIL;
-		goto exit;
-	}
+	if (init_mlme_ext_priv(padapter) == _FAIL)
+		return _FAIL;

 	if (_rtw_init_xmit_priv(&padapter->xmitpriv, padapter) == _FAIL) {
 		DBG_88E("Can't _rtw_init_xmit_priv\n");
-		ret8 = _FAIL;
-		goto exit;
+		return _FAIL;
 	}

 	if (_rtw_init_recv_priv(&padapter->recvpriv, padapter) == _FAIL) {
 		DBG_88E("Can't _rtw_init_recv_priv\n");
-		ret8 = _FAIL;
-		goto exit;
+		return _FAIL;
 	}

 	if (_rtw_init_sta_priv(&padapter->stapriv) == _FAIL) {
 		DBG_88E("Can't _rtw_init_sta_priv\n");
-		ret8 = _FAIL;
-		goto exit;
+		return _FAIL;
 	}

 	padapter->stapriv.padapter = padapter;
@@ -539,7 +528,6 @@ u8 rtw_init_drv_sw(struct adapter *padapter)

 	spin_lock_init(&padapter->br_ext_lock);

-exit:
 	return ret8;
 }

--
2.33.0

Re: [PATCH v2] staging: r8188eu: os_dep: remove the goto statement

[Greg KH]

On Wed, Nov 03, 2021 at 09:18:41PM +0530, Saurav Girepunje wrote:
> Remove the goto statement from rtw_init_drv_sw(). In this function goto
> can be replace by return statement. As on goto label exit, function
> only return it is not performing any cleanup. Avoiding goto will
> improve the function readability.
> 
> Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>
> ---
> 
> ChangelogV2:
> 
> 	-On V1 combined the if condition check for functions as below
> 
> 	 if (!rtw_init_evt_priv(&padapter->evtpriv) || !rtw_init_mlme_priv(padapter))
> 		return _FAIL;
> 	reverting these changes and keeping them as-is. It will make more clear on
> 	individual function call if something need to handle for error case.
> 
> ChangelogV1:
> 
> 	-Remove the goto statement from rtw_init_drv_sw(). In this function goto
> 	 can be replace by return statement. As on goto label exit, function
> 	 only return it is not performing any cleanup. Avoiding goto will
> 	 improve the function readability.
> 
>  drivers/staging/r8188eu/os_dep/os_intfs.c | 34 ++++++++---------------
>  1 file changed, 11 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/os_dep/os_intfs.c b/drivers/staging/r8188eu/os_dep/os_intfs.c
> index 1418c9c4916c..ec96e837ab88 100644
> --- a/drivers/staging/r8188eu/os_dep/os_intfs.c
> +++ b/drivers/staging/r8188eu/os_dep/os_intfs.c
> @@ -480,48 +480,37 @@ u8 rtw_init_drv_sw(struct adapter *padapter)
>  {
>  	u8	ret8 = _SUCCESS;
> 
> -	if ((rtw_init_cmd_priv(&padapter->cmdpriv)) == _FAIL) {
> -		ret8 = _FAIL;
> -		goto exit;
> -	}
> +	if ((rtw_init_cmd_priv(&padapter->cmdpriv)) == _FAIL)
> +		return _FAIL;
> 
>  	padapter->cmdpriv.padapter = padapter;
> 
> -	if ((rtw_init_evt_priv(&padapter->evtpriv)) == _FAIL) {
> -		ret8 = _FAIL;
> -		goto exit;
> -	}
> +	if ((rtw_init_evt_priv(&padapter->evtpriv)) == _FAIL)
> +		return _FAIL;
> 
> -	if (rtw_init_mlme_priv(padapter) == _FAIL) {
> -		ret8 = _FAIL;
> -		goto exit;
> -	}
> +	if (rtw_init_mlme_priv(padapter) == _FAIL)
> +		return _FAIL;
> 
>  	rtw_init_wifidirect_timers(padapter);
>  	init_wifidirect_info(padapter, P2P_ROLE_DISABLE);
>  	reset_global_wifidirect_info(padapter);
> 
> -	if (init_mlme_ext_priv(padapter) == _FAIL) {
> -		ret8 = _FAIL;
> -		goto exit;
> -	}
> +	if (init_mlme_ext_priv(padapter) == _FAIL)
> +		return _FAIL;
> 
>  	if (_rtw_init_xmit_priv(&padapter->xmitpriv, padapter) == _FAIL) {
>  		DBG_88E("Can't _rtw_init_xmit_priv\n");
> -		ret8 = _FAIL;
> -		goto exit;
> +		return _FAIL;
>  	}
> 
>  	if (_rtw_init_recv_priv(&padapter->recvpriv, padapter) == _FAIL) {
>  		DBG_88E("Can't _rtw_init_recv_priv\n");
> -		ret8 = _FAIL;
> -		goto exit;
> +		return _FAIL;
>  	}
> 
>  	if (_rtw_init_sta_priv(&padapter->stapriv) == _FAIL) {
>  		DBG_88E("Can't _rtw_init_sta_priv\n");
> -		ret8 = _FAIL;
> -		goto exit;
> +		return _FAIL;
>  	}
> 
>  	padapter->stapriv.padapter = padapter;

Right after this, ret8 is set, but never checked, which looks like a bug
to me.  Can you work on fixing that after I take this patch?

thanks,

greg k-h

Re: [PATCH v2] staging: r8188eu: os_dep: remove the goto statement

[Pavel Skripkin]

On 11/5/21 12:53, Greg KH wrote:
> Right after this, ret8 is set, but never checked, which looks like a bug
> to me.  Can you work on fixing that after I take this patch?
> 
> thanks,
> 
> greg k-h
> 

ret8 is returned from this function, but as I said [1] it can be just 
removed. It will be always set to _SUCCESS.



[1] 
https://lore.kernel.org/linux-staging/f26b4aec-c0a1-8c93-b34e-8b1a36ac81b3@gmail.com/


With regards,
Pavel Skripkin

Re: [PATCH v2] staging: r8188eu: os_dep: remove the goto statement

[Saurav Girepunje]

On 05/11/21 4:06 pm, Pavel Skripkin wrote:
> On 11/5/21 12:53, Greg KH wrote:
>> Right after this, ret8 is set, but never checked, which looks like a bug
>> to me.  Can you work on fixing that after I take this patch?
Yes, I will send a patch removing a local variable for return value and changing 
the return type of function rtw_init_default_value . As rtw_init_default_value always return 
success and the return value of this function is not checked.
>>
>> thanks,
>>
>> greg k-h
>>
> 
> ret8 is returned from this function, but as I said [1] it can be just removed. It will be always set to _SUCCESS.
> 
> 
> 
> [1] https://lore.kernel.org/linux-staging/f26b4aec-c0a1-8c93-b34e-8b1a36ac81b3@gmail.com/
> 
> 
Pavel also mentioned to remove the local variable ret8.
> With regards,
> Pavel Skripkin

Thanks,
Saurav