* [PATCH v2] staging: r8188eu: os_dep: remove the goto statement
@ 2021-11-03 15:48 Saurav Girepunje
2021-11-05 9:53 ` Greg KH
0 siblings, 1 reply; 4+ messages in thread
From: Saurav Girepunje @ 2021-11-03 15:48 UTC (permalink / raw)
To: Larry.Finger, phil, gregkh, straube.linux, martin,
saurav.girepunje, linux-staging, linux-kernel
Cc: 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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] staging: r8188eu: os_dep: remove the goto statement
2021-11-03 15:48 [PATCH v2] staging: r8188eu: os_dep: remove the goto statement Saurav Girepunje
@ 2021-11-05 9:53 ` Greg KH
2021-11-05 10:36 ` Pavel Skripkin
0 siblings, 1 reply; 4+ messages in thread
From: Greg KH @ 2021-11-05 9:53 UTC (permalink / raw)
To: Saurav Girepunje
Cc: Larry.Finger, phil, straube.linux, martin, linux-staging,
linux-kernel, saurav.girepunje
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] staging: r8188eu: os_dep: remove the goto statement
2021-11-05 9:53 ` Greg KH
@ 2021-11-05 10:36 ` Pavel Skripkin
2021-11-05 17:18 ` Saurav Girepunje
0 siblings, 1 reply; 4+ messages in thread
From: Pavel Skripkin @ 2021-11-05 10:36 UTC (permalink / raw)
To: Greg KH, Saurav Girepunje
Cc: Larry.Finger, phil, straube.linux, martin, linux-staging,
linux-kernel, saurav.girepunje
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] staging: r8188eu: os_dep: remove the goto statement
2021-11-05 10:36 ` Pavel Skripkin
@ 2021-11-05 17:18 ` Saurav Girepunje
0 siblings, 0 replies; 4+ messages in thread
From: Saurav Girepunje @ 2021-11-05 17:18 UTC (permalink / raw)
To: Pavel Skripkin, Greg KH
Cc: Larry.Finger, phil, straube.linux, martin, linux-staging,
linux-kernel, 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-11-05 17:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-03 15:48 [PATCH v2] staging: r8188eu: os_dep: remove the goto statement Saurav Girepunje
2021-11-05 9:53 ` Greg KH
2021-11-05 10:36 ` Pavel Skripkin
2021-11-05 17:18 ` Saurav Girepunje
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).