Re: [PATCH] staging: rtl8723bs: fix error handling in sdio_dvobj_init and it's callees

[Dan Carpenter]

The subject is a bit long.  It's not really a "fix" it's a cleanup.

Subject: [PATCH] staging: rtl8723bs: cleanup return in sdio_dvobj_init()

On Tue, Mar 24, 2026 at 12:25:26AM +0100, Omer El Idrissi wrote:
> Return proper errno values instead of vendor-defined non-descriptive
> _SUCCESS/_FAIL macros
> Callers only check for non-zero return values, so this does not change
> behaviour while improving correctness.
>

This is how your email looks like to me:
https://lore.kernel.org/all/20260323232526.25288-1-omer.e.idrissi@xxxxxxxxx/

I can't find the subject so I only read the commit message.  The commit
message should mention sdio_dvobj_init().  But really sdio_dvobj_init()
is not a leaf function, it's a branch.  sdio_init() is the leaf at the
end of the call tree.  The subject should really say:

[PATCH] staging: rtl8723bs: cleanup return in sdio_init()







> Signed-off-by: Omer El Idrissi <omer.e.idrissi@xxxxxxxxx>
> ---
>  drivers/staging/rtl8723bs/os_dep/os_intfs.c  |  7 ++++---
>  drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 19 +++++++++++--------
>  2 files changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/os_dep/os_intfs.c b/drivers/staging/rtl8723bs/os_dep/os_intfs.c
> index 7ba689f2dfc8..80ff3154f6e7 100644
> --- a/drivers/staging/rtl8723bs/os_dep/os_intfs.c
> +++ b/drivers/staging/rtl8723bs/os_dep/os_intfs.c
> @@ -1136,9 +1136,10 @@ static int rtw_resume_process_normal(struct adapter *padapter)
>  	pmlmepriv = &padapter->mlmepriv;
>  	/*  interface init */
>  	/* if (sdio_init(adapter_to_dvobj(padapter)) != _SUCCESS) */
> -	if ((padapter->intf_init) && (padapter->intf_init(adapter_to_dvobj(padapter)) != _SUCCESS)) {
> -		ret = -1;
> -		goto exit;
> +	if (padapter->intf_init) {
> +		ret = padapter->intf_init(adapter_to_dvobj(padapter));
> +		if (ret)
> +			goto exit;
>  	}
>  	rtw_hal_disable_interrupt(padapter);
>  	/* if (sdio_alloc_irq(adapter_to_dvobj(padapter)) != _SUCCESS) */
> diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> index d664e254912c..0d6475bfbaba 100644
> --- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> +++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> @@ -131,9 +131,7 @@ static u32 sdio_init(struct dvobj_priv *dvobj)
>  release:
>  	sdio_release_host(func);
>  
> -	if (err)
> -		return _FAIL;
> -	return _SUCCESS;
> +	return err;
>  }
>  
>  static void sdio_deinit(struct dvobj_priv *dvobj)
> @@ -159,16 +157,19 @@ static struct dvobj_priv *sdio_dvobj_init(struct sdio_func *func)
>  	struct dvobj_priv *dvobj = NULL;
>  	struct sdio_data *psdio;
>  
> -	dvobj = devobj_init();
> -	if (!dvobj)
> +	dvobj = devobj_init();
> +	if (!dvobj) {
> +		dvobj = ERR_PTR(-ENOMEM);
>  		goto exit;
> +	}
>  
>  	sdio_set_drvdata(func, dvobj);
>  
>  	psdio = &dvobj->intf_data;
>  	psdio->func = func;
>  
> -	if (sdio_init(dvobj) != _SUCCESS)
> +	status = sdio_init(dvobj);
> +	if (status)
>  		goto free_dvobj;
>  
>  	rtw_reset_continual_io_error(dvobj);
> @@ -180,7 +181,7 @@ static struct dvobj_priv *sdio_dvobj_init(struct sdio_func *func)
>  
>  		devobj_deinit(dvobj);
>  
> -		dvobj = NULL;
> +		dvobj = ERR_PTR(status);
>  	}
>  exit:
>  	return dvobj;

This function does some nonsense stuff.  First convert it to
direct returns and then your other patch will be easier.  Actually,
I would leave devobj_init() as returning NULL.  Your conversion to
error pointers isn't wrong but it's isn't necessary either.

[PATCH 1] staging: rtl8723bs: use direct returns in sdio_dvobj_init()
[PATCH 2] staging: rtl8723bs: cleanup return in sdio_init()

regards,
dan carpenter

diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
index d664e254912c..5e093324bbd6 100644
--- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
+++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
@@ -161,7 +161,7 @@ static struct dvobj_priv *sdio_dvobj_init(struct sdio_func *func)
 
 	dvobj = devobj_init();
 	if (!dvobj)
-		goto exit;
+		return NULL;
 
 	sdio_set_drvdata(func, dvobj);
 
@@ -172,18 +172,13 @@ static struct dvobj_priv *sdio_dvobj_init(struct sdio_func *func)
 		goto free_dvobj;
 
 	rtw_reset_continual_io_error(dvobj);
-	status = _SUCCESS;
+	return dvobj;
 
 free_dvobj:
-	if (status != _SUCCESS && dvobj) {
-		sdio_set_drvdata(func, NULL);
-
-		devobj_deinit(dvobj);
+	sdio_set_drvdata(func, NULL);
+	devobj_deinit(dvobj);
 
-		dvobj = NULL;
-	}
-exit:
-	return dvobj;
+	return NULL;
 }
 
 static void sdio_dvobj_deinit(struct sdio_func *func)