1. Patchew
  2. linux
  3. View series

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

Omer El Idrissi posted 1 patch 1 week, 3 days ago 
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(-)
[PATCH] staging: rtl8723bs: fix error handling in sdio_dvobj_init and it's callees
Posted by Omer El Idrissi 1 week, 3 days ago 
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.

Signed-off-by: Omer El Idrissi <omer.e.idrissi@gmail.com>
---
 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;
@@ -350,8 +351,10 @@ static int rtw_drv_init(
 	struct dvobj_priv *dvobj;
 
 	dvobj = sdio_dvobj_init(func);
-	if (!dvobj)
+	if (IS_ERR(dvobj)) {
+		status = PTR_ERR(dvobj);
 		goto exit;
+	}
 
 	if1 = rtw_sdio_if1_init(dvobj, id);
 	if (!if1)
-- 
2.51.0
Re: [PATCH] staging: rtl8723bs: fix error handling in sdio_dvobj_init and it's callees
Posted by Dan Carpenter 1 week, 3 days ago 
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@gmail.com/

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@gmail.com>
> ---
>  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)

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.