[PATCH 0/2] staging: r8188eu: correct error logic of two functions

[PATCH 0/2] staging: r8188eu: correct error logic of two functions

[Michael Straube]

This series converts two functions away from returning _SUCCESS and
_FAIL. Another tiny step to get rid of _FAIL / _SUCCESS someday.

Tested on x86_64 with Inter-Tech DMG 02.

Michael Straube (2):
  staging: r8188eu: correct error logic of rtl8188eu_init_recv_priv()
  staging: r8188eu: correct error logic of _rtw_init_recv_priv()

 drivers/staging/r8188eu/core/rtw_recv.c   | 30 +++++++++--------------
 drivers/staging/r8188eu/os_dep/os_intfs.c |  2 +-
 2 files changed, 12 insertions(+), 20 deletions(-)

-- 
2.39.1

[PATCH 1/2] staging: r8188eu: correct error logic of rtl8188eu_init_recv_priv()

[Michael Straube]

Convert the function rtl8188eu_init_recv_priv() away from returning
_FAIL and _SUCCESS, which uses inverted error logic. Return 0 for
success and negative values for failure instead.

Signed-off-by: Michael Straube <straube.linux@gmail.com>
---
 drivers/staging/r8188eu/core/rtw_recv.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_recv.c b/drivers/staging/r8188eu/core/rtw_recv.c
index 631c500dda42..70d43c10e53d 100644
--- a/drivers/staging/r8188eu/core/rtw_recv.c
+++ b/drivers/staging/r8188eu/core/rtw_recv.c
@@ -38,7 +38,7 @@ void _rtw_init_sta_recv_priv(struct sta_recv_priv *psta_recvpriv)
 static int rtl8188eu_init_recv_priv(struct adapter *padapter)
 {
 	struct recv_priv *precvpriv = &padapter->recvpriv;
-	int i, res = _SUCCESS;
+	int i, err = 0;
 	struct recv_buf *precvbuf;
 
 	tasklet_init(&precvpriv->recv_tasklet,
@@ -50,10 +50,8 @@ static int rtl8188eu_init_recv_priv(struct adapter *padapter)
 
 	precvpriv->pallocated_recv_buf = kzalloc(NR_RECVBUFF * sizeof(struct recv_buf) + 4,
 						 GFP_KERNEL);
-	if (!precvpriv->pallocated_recv_buf) {
-		res = _FAIL;
-		goto exit;
-	}
+	if (!precvpriv->pallocated_recv_buf)
+		return -ENOMEM;
 
 	precvpriv->precv_buf = (u8 *)ALIGN((size_t)(precvpriv->pallocated_recv_buf), 4);
 
@@ -64,7 +62,7 @@ static int rtl8188eu_init_recv_priv(struct adapter *padapter)
 		precvbuf->reuse = false;
 		precvbuf->purb = usb_alloc_urb(0, GFP_KERNEL);
 		if (!precvbuf->purb) {
-			res = _FAIL;
+			err = -ENOMEM;
 			break;
 		}
 		precvbuf->adapter = padapter;
@@ -94,8 +92,8 @@ static int rtl8188eu_init_recv_priv(struct adapter *padapter)
 			pskb = NULL;
 		}
 	}
-exit:
-	return res;
+
+	return err;
 }
 
 int _rtw_init_recv_priv(struct recv_priv *precvpriv, struct adapter *padapter)
@@ -141,7 +139,8 @@ int _rtw_init_recv_priv(struct recv_priv *precvpriv, struct adapter *padapter)
 	}
 	precvpriv->rx_pending_cnt = 1;
 
-	res = rtl8188eu_init_recv_priv(padapter);
+	if (rtl8188eu_init_recv_priv(padapter))
+		res = _FAIL;
 
 	timer_setup(&precvpriv->signal_stat_timer, rtw_signal_stat_timer_hdl, 0);
 	precvpriv->signal_stat_sampling_interval = 1000; /* ms */
-- 
2.39.1

[PATCH 2/2] staging: r8188eu: correct error logic of _rtw_init_recv_priv()

[Michael Straube]

Convert the function _rtw_init_recv_priv() away from returning _FAIL
and _SUCCESS, which uses inverted error logic. Return 0 for success
and negative values for failure instead.

Signed-off-by: Michael Straube <straube.linux@gmail.com>
---
 drivers/staging/r8188eu/core/rtw_recv.c   | 17 +++++------------
 drivers/staging/r8188eu/os_dep/os_intfs.c |  2 +-
 2 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_recv.c b/drivers/staging/r8188eu/core/rtw_recv.c
index 70d43c10e53d..4c823bbcc22b 100644
--- a/drivers/staging/r8188eu/core/rtw_recv.c
+++ b/drivers/staging/r8188eu/core/rtw_recv.c
@@ -99,10 +99,8 @@ static int rtl8188eu_init_recv_priv(struct adapter *padapter)
 int _rtw_init_recv_priv(struct recv_priv *precvpriv, struct adapter *padapter)
 {
 	int i;
-
 	struct recv_frame *precvframe;
-
-	int	res = _SUCCESS;
+	int err = 0;
 
 	spin_lock_init(&precvpriv->lock);
 
@@ -115,11 +113,8 @@ int _rtw_init_recv_priv(struct recv_priv *precvpriv, struct adapter *padapter)
 	precvpriv->free_recvframe_cnt = NR_RECVFRAME;
 
 	precvpriv->pallocated_frame_buf = vzalloc(NR_RECVFRAME * sizeof(struct recv_frame) + RXFRAME_ALIGN_SZ);
-
-	if (!precvpriv->pallocated_frame_buf) {
-		res = _FAIL;
-		goto exit;
-	}
+	if (!precvpriv->pallocated_frame_buf)
+		return -ENOMEM;
 
 	precvpriv->precv_frame_buf = (u8 *)ALIGN((size_t)(precvpriv->pallocated_frame_buf), RXFRAME_ALIGN_SZ);
 
@@ -139,16 +134,14 @@ int _rtw_init_recv_priv(struct recv_priv *precvpriv, struct adapter *padapter)
 	}
 	precvpriv->rx_pending_cnt = 1;
 
-	if (rtl8188eu_init_recv_priv(padapter))
-		res = _FAIL;
+	err = rtl8188eu_init_recv_priv(padapter);
 
 	timer_setup(&precvpriv->signal_stat_timer, rtw_signal_stat_timer_hdl, 0);
 	precvpriv->signal_stat_sampling_interval = 1000; /* ms */
 
 	rtw_set_signal_stat_timer(precvpriv);
-exit:
 
-	return res;
+	return err;
 }
 
 static void rtl8188eu_free_recv_priv(struct adapter *padapter)
diff --git a/drivers/staging/r8188eu/os_dep/os_intfs.c b/drivers/staging/r8188eu/os_dep/os_intfs.c
index cfc24420e70c..4130e8fe2952 100644
--- a/drivers/staging/r8188eu/os_dep/os_intfs.c
+++ b/drivers/staging/r8188eu/os_dep/os_intfs.c
@@ -482,7 +482,7 @@ u8 rtw_init_drv_sw(struct adapter *padapter)
 		goto free_mlme_ext;
 	}
 
-	if (_rtw_init_recv_priv(&padapter->recvpriv, padapter) == _FAIL) {
+	if (_rtw_init_recv_priv(&padapter->recvpriv, padapter)) {
 		dev_err(dvobj_to_dev(padapter->dvobj), "_rtw_init_recv_priv failed\n");
 		goto free_xmit_priv;
 	}
-- 
2.39.1

Re: [PATCH 0/2] staging: r8188eu: correct error logic of two functions

[Philipp Hortmann]

On 2/4/23 11:16, Michael Straube wrote:
> This series converts two functions away from returning _SUCCESS and
> _FAIL. Another tiny step to get rid of _FAIL / _SUCCESS someday.
> 
> Tested on x86_64 with Inter-Tech DMG 02.
> 
> Michael Straube (2):
>    staging: r8188eu: correct error logic of rtl8188eu_init_recv_priv()
>    staging: r8188eu: correct error logic of _rtw_init_recv_priv()
> 
>   drivers/staging/r8188eu/core/rtw_recv.c   | 30 +++++++++--------------
>   drivers/staging/r8188eu/os_dep/os_intfs.c |  2 +-
>   2 files changed, 12 insertions(+), 20 deletions(-)
> 
Tested-by: Philipp Hortmann <philipp.g.hortmann@gmail.com> # Edimax N150

Re: [PATCH 2/2] staging: r8188eu: correct error logic of _rtw_init_recv_priv()

[Greg KH]

On Sat, Feb 04, 2023 at 11:16:54AM +0100, Michael Straube wrote:
> Convert the function _rtw_init_recv_priv() away from returning _FAIL
> and _SUCCESS, which uses inverted error logic. Return 0 for success
> and negative values for failure instead.
> 
> Signed-off-by: Michael Straube <straube.linux@gmail.com>
> ---
>  drivers/staging/r8188eu/core/rtw_recv.c   | 17 +++++------------
>  drivers/staging/r8188eu/os_dep/os_intfs.c |  2 +-
>  2 files changed, 6 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/core/rtw_recv.c b/drivers/staging/r8188eu/core/rtw_recv.c
> index 70d43c10e53d..4c823bbcc22b 100644
> --- a/drivers/staging/r8188eu/core/rtw_recv.c
> +++ b/drivers/staging/r8188eu/core/rtw_recv.c
> @@ -99,10 +99,8 @@ static int rtl8188eu_init_recv_priv(struct adapter *padapter)
>  int _rtw_init_recv_priv(struct recv_priv *precvpriv, struct adapter *padapter)
>  {
>  	int i;
> -
>  	struct recv_frame *precvframe;
> -
> -	int	res = _SUCCESS;
> +	int err = 0;
>  
>  	spin_lock_init(&precvpriv->lock);
>  
> @@ -115,11 +113,8 @@ int _rtw_init_recv_priv(struct recv_priv *precvpriv, struct adapter *padapter)
>  	precvpriv->free_recvframe_cnt = NR_RECVFRAME;
>  
>  	precvpriv->pallocated_frame_buf = vzalloc(NR_RECVFRAME * sizeof(struct recv_frame) + RXFRAME_ALIGN_SZ);
> -
> -	if (!precvpriv->pallocated_frame_buf) {
> -		res = _FAIL;
> -		goto exit;
> -	}
> +	if (!precvpriv->pallocated_frame_buf)
> +		return -ENOMEM;
>  
>  	precvpriv->precv_frame_buf = (u8 *)ALIGN((size_t)(precvpriv->pallocated_frame_buf), RXFRAME_ALIGN_SZ);
>  
> @@ -139,16 +134,14 @@ int _rtw_init_recv_priv(struct recv_priv *precvpriv, struct adapter *padapter)
>  	}
>  	precvpriv->rx_pending_cnt = 1;
>  
> -	if (rtl8188eu_init_recv_priv(padapter))
> -		res = _FAIL;
> +	err = rtl8188eu_init_recv_priv(padapter);

You are keeping the original logic here, but this is odd, nothing
actually changes if this is an error except you return it?  That seems
wrong, and you might want to fix that up in further patches.

thanks,

greg k-h