* [PATCH] staging: rtl8723bs: fix error handling in sdio_dvobj_init and it's callees
@ 2026-03-23 23:25 Omer El Idrissi
2026-03-24 6:10 ` Dan Carpenter
0 siblings, 1 reply; 2+ messages in thread
From: Omer El Idrissi @ 2026-03-23 23:25 UTC (permalink / raw)
To: gregkh; +Cc: linux-staging, linux-kernel, Omer El Idrissi
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
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH] staging: rtl8723bs: fix error handling in sdio_dvobj_init and it's callees
2026-03-23 23:25 [PATCH] staging: rtl8723bs: fix error handling in sdio_dvobj_init and it's callees Omer El Idrissi
@ 2026-03-24 6:10 ` Dan Carpenter
0 siblings, 0 replies; 2+ messages in thread
From: Dan Carpenter @ 2026-03-24 6:10 UTC (permalink / raw)
To: Omer El Idrissi; +Cc: gregkh, linux-staging, linux-kernel
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)
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-03-24 6:10 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-23 23:25 [PATCH] staging: rtl8723bs: fix error handling in sdio_dvobj_init and it's callees Omer El Idrissi
2026-03-24 6:10 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox