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@xxxxxxxxx>
> ---
>
> 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
