[PATCH] staging: r8188eu: convert three functions to bool

[PATCH] staging: r8188eu: convert three functions to bool

[Michael Straube]

The functions

is_client_associated_to_ap()
is_client_associated_to_ibss()
is_IBSS_empty()

return boolean values. Convert their return type to bool and replace
_FAIL, which is defined as 0, with false. Another step to get rid of
_SUCCESS / _FAIL.

Signed-off-by: Michael Straube <straube.linux@gmail.com>
---
 drivers/staging/r8188eu/core/rtw_wlan_util.c   | 18 +++++++++---------
 drivers/staging/r8188eu/include/rtw_mlme_ext.h |  6 +++---
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_wlan_util.c b/drivers/staging/r8188eu/core/rtw_wlan_util.c
index e50631848cab..c95438a12b59 100644
--- a/drivers/staging/r8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/r8188eu/core/rtw_wlan_util.c
@@ -331,35 +331,35 @@ u16 get_beacon_interval(struct wlan_bssid_ex *bss)
 	return le16_to_cpu(val);
 }
 
-int is_client_associated_to_ap(struct adapter *padapter)
+bool is_client_associated_to_ap(struct adapter *padapter)
 {
 	struct mlme_ext_priv	*pmlmeext;
 	struct mlme_ext_info	*pmlmeinfo;
 
 	if (!padapter)
-		return _FAIL;
+		return false;
 
 	pmlmeext = &padapter->mlmeextpriv;
 	pmlmeinfo = &pmlmeext->mlmext_info;
 
 	if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) && ((pmlmeinfo->state & 0x03) == WIFI_FW_STATION_STATE))
 		return true;
-	else
-		return _FAIL;
+
+	return false;
 }
 
-int is_client_associated_to_ibss(struct adapter *padapter)
+bool is_client_associated_to_ibss(struct adapter *padapter)
 {
 	struct mlme_ext_priv	*pmlmeext = &padapter->mlmeextpriv;
 	struct mlme_ext_info	*pmlmeinfo = &pmlmeext->mlmext_info;
 
 	if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) && ((pmlmeinfo->state & 0x03) == WIFI_FW_ADHOC_STATE))
 		return true;
-	else
-		return _FAIL;
+
+	return false;
 }
 
-int is_IBSS_empty(struct adapter *padapter)
+bool is_IBSS_empty(struct adapter *padapter)
 {
 	unsigned int i;
 	struct mlme_ext_priv	*pmlmeext = &padapter->mlmeextpriv;
@@ -367,7 +367,7 @@ int is_IBSS_empty(struct adapter *padapter)
 
 	for (i = IBSS_START_MAC_ID; i < NUM_STA; i++) {
 		if (pmlmeinfo->FW_sta_info[i].status == 1)
-			return _FAIL;
+			return false;
 	}
 	return true;
 }
diff --git a/drivers/staging/r8188eu/include/rtw_mlme_ext.h b/drivers/staging/r8188eu/include/rtw_mlme_ext.h
index e234a3b9af6f..7652e72a03f4 100644
--- a/drivers/staging/r8188eu/include/rtw_mlme_ext.h
+++ b/drivers/staging/r8188eu/include/rtw_mlme_ext.h
@@ -432,9 +432,9 @@ void update_network(struct wlan_bssid_ex *dst, struct wlan_bssid_ex *src,
 u8 *get_my_bssid(struct wlan_bssid_ex *pnetwork);
 u16 get_beacon_interval(struct wlan_bssid_ex *bss);
 
-int is_client_associated_to_ap(struct adapter *padapter);
-int is_client_associated_to_ibss(struct adapter *padapter);
-int is_IBSS_empty(struct adapter *padapter);
+bool is_client_associated_to_ap(struct adapter *padapter);
+bool is_client_associated_to_ibss(struct adapter *padapter);
+bool is_IBSS_empty(struct adapter *padapter);
 
 unsigned char check_assoc_AP(u8 *pframe, uint len);
 
-- 
2.38.0

Re: [PATCH] staging: r8188eu: convert three functions to bool

[Phillip Potter]

On Sat, Nov 05, 2022 at 10:39:16AM +0100, Michael Straube wrote:
> The functions
> 
> is_client_associated_to_ap()
> is_client_associated_to_ibss()
> is_IBSS_empty()
> 
> return boolean values. Convert their return type to bool and replace
> _FAIL, which is defined as 0, with false. Another step to get rid of
> _SUCCESS / _FAIL.
> 
> Signed-off-by: Michael Straube <straube.linux@gmail.com>
> ---
>  drivers/staging/r8188eu/core/rtw_wlan_util.c   | 18 +++++++++---------
>  drivers/staging/r8188eu/include/rtw_mlme_ext.h |  6 +++---
>  2 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/core/rtw_wlan_util.c b/drivers/staging/r8188eu/core/rtw_wlan_util.c
> index e50631848cab..c95438a12b59 100644
> --- a/drivers/staging/r8188eu/core/rtw_wlan_util.c
> +++ b/drivers/staging/r8188eu/core/rtw_wlan_util.c
> @@ -331,35 +331,35 @@ u16 get_beacon_interval(struct wlan_bssid_ex *bss)
>  	return le16_to_cpu(val);
>  }
>  
> -int is_client_associated_to_ap(struct adapter *padapter)
> +bool is_client_associated_to_ap(struct adapter *padapter)
>  {
>  	struct mlme_ext_priv	*pmlmeext;
>  	struct mlme_ext_info	*pmlmeinfo;
>  
>  	if (!padapter)
> -		return _FAIL;
> +		return false;
>  
>  	pmlmeext = &padapter->mlmeextpriv;
>  	pmlmeinfo = &pmlmeext->mlmext_info;
>  
>  	if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) && ((pmlmeinfo->state & 0x03) == WIFI_FW_STATION_STATE))
>  		return true;
> -	else
> -		return _FAIL;
> +
> +	return false;
>  }
>  
> -int is_client_associated_to_ibss(struct adapter *padapter)
> +bool is_client_associated_to_ibss(struct adapter *padapter)
>  {
>  	struct mlme_ext_priv	*pmlmeext = &padapter->mlmeextpriv;
>  	struct mlme_ext_info	*pmlmeinfo = &pmlmeext->mlmext_info;
>  
>  	if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) && ((pmlmeinfo->state & 0x03) == WIFI_FW_ADHOC_STATE))
>  		return true;
> -	else
> -		return _FAIL;
> +
> +	return false;
>  }
>  
> -int is_IBSS_empty(struct adapter *padapter)
> +bool is_IBSS_empty(struct adapter *padapter)
>  {
>  	unsigned int i;
>  	struct mlme_ext_priv	*pmlmeext = &padapter->mlmeextpriv;
> @@ -367,7 +367,7 @@ int is_IBSS_empty(struct adapter *padapter)
>  
>  	for (i = IBSS_START_MAC_ID; i < NUM_STA; i++) {
>  		if (pmlmeinfo->FW_sta_info[i].status == 1)
> -			return _FAIL;
> +			return false;
>  	}
>  	return true;
>  }
> diff --git a/drivers/staging/r8188eu/include/rtw_mlme_ext.h b/drivers/staging/r8188eu/include/rtw_mlme_ext.h
> index e234a3b9af6f..7652e72a03f4 100644
> --- a/drivers/staging/r8188eu/include/rtw_mlme_ext.h
> +++ b/drivers/staging/r8188eu/include/rtw_mlme_ext.h
> @@ -432,9 +432,9 @@ void update_network(struct wlan_bssid_ex *dst, struct wlan_bssid_ex *src,
>  u8 *get_my_bssid(struct wlan_bssid_ex *pnetwork);
>  u16 get_beacon_interval(struct wlan_bssid_ex *bss);
>  
> -int is_client_associated_to_ap(struct adapter *padapter);
> -int is_client_associated_to_ibss(struct adapter *padapter);
> -int is_IBSS_empty(struct adapter *padapter);
> +bool is_client_associated_to_ap(struct adapter *padapter);
> +bool is_client_associated_to_ibss(struct adapter *padapter);
> +bool is_IBSS_empty(struct adapter *padapter);
>  
>  unsigned char check_assoc_AP(u8 *pframe, uint len);
>  
> -- 
> 2.38.0
> 

Hi Michael,

Thanks for the patch. Just my personal opinion, but I would prefer to
keep the return type as int, and have 0 for success and then _FAIL
replaced with an appropriate error code depending on the circumstance,
(e.g. -ENOMEM). This is generally the convention elsewhere in the
kernel. Others may not agree though.

All the best,
Phil

Re: [PATCH] staging: r8188eu: convert three functions to bool

[Greg KH]

On Sat, Nov 05, 2022 at 11:06:56AM +0000, Phillip Potter wrote:
> On Sat, Nov 05, 2022 at 10:39:16AM +0100, Michael Straube wrote:
> > The functions
> > 
> > is_client_associated_to_ap()
> > is_client_associated_to_ibss()
> > is_IBSS_empty()
> > 
> > return boolean values. Convert their return type to bool and replace
> > _FAIL, which is defined as 0, with false. Another step to get rid of
> > _SUCCESS / _FAIL.
> > 
> > Signed-off-by: Michael Straube <straube.linux@gmail.com>
> > ---
> >  drivers/staging/r8188eu/core/rtw_wlan_util.c   | 18 +++++++++---------
> >  drivers/staging/r8188eu/include/rtw_mlme_ext.h |  6 +++---
> >  2 files changed, 12 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/staging/r8188eu/core/rtw_wlan_util.c b/drivers/staging/r8188eu/core/rtw_wlan_util.c
> > index e50631848cab..c95438a12b59 100644
> > --- a/drivers/staging/r8188eu/core/rtw_wlan_util.c
> > +++ b/drivers/staging/r8188eu/core/rtw_wlan_util.c
> > @@ -331,35 +331,35 @@ u16 get_beacon_interval(struct wlan_bssid_ex *bss)
> >  	return le16_to_cpu(val);
> >  }
> >  
> > -int is_client_associated_to_ap(struct adapter *padapter)
> > +bool is_client_associated_to_ap(struct adapter *padapter)
> >  {
> >  	struct mlme_ext_priv	*pmlmeext;
> >  	struct mlme_ext_info	*pmlmeinfo;
> >  
> >  	if (!padapter)
> > -		return _FAIL;
> > +		return false;
> >  
> >  	pmlmeext = &padapter->mlmeextpriv;
> >  	pmlmeinfo = &pmlmeext->mlmext_info;
> >  
> >  	if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) && ((pmlmeinfo->state & 0x03) == WIFI_FW_STATION_STATE))
> >  		return true;
> > -	else
> > -		return _FAIL;
> > +
> > +	return false;
> >  }
> >  
> > -int is_client_associated_to_ibss(struct adapter *padapter)
> > +bool is_client_associated_to_ibss(struct adapter *padapter)
> >  {
> >  	struct mlme_ext_priv	*pmlmeext = &padapter->mlmeextpriv;
> >  	struct mlme_ext_info	*pmlmeinfo = &pmlmeext->mlmext_info;
> >  
> >  	if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) && ((pmlmeinfo->state & 0x03) == WIFI_FW_ADHOC_STATE))
> >  		return true;
> > -	else
> > -		return _FAIL;
> > +
> > +	return false;
> >  }
> >  
> > -int is_IBSS_empty(struct adapter *padapter)
> > +bool is_IBSS_empty(struct adapter *padapter)
> >  {
> >  	unsigned int i;
> >  	struct mlme_ext_priv	*pmlmeext = &padapter->mlmeextpriv;
> > @@ -367,7 +367,7 @@ int is_IBSS_empty(struct adapter *padapter)
> >  
> >  	for (i = IBSS_START_MAC_ID; i < NUM_STA; i++) {
> >  		if (pmlmeinfo->FW_sta_info[i].status == 1)
> > -			return _FAIL;
> > +			return false;
> >  	}
> >  	return true;
> >  }
> > diff --git a/drivers/staging/r8188eu/include/rtw_mlme_ext.h b/drivers/staging/r8188eu/include/rtw_mlme_ext.h
> > index e234a3b9af6f..7652e72a03f4 100644
> > --- a/drivers/staging/r8188eu/include/rtw_mlme_ext.h
> > +++ b/drivers/staging/r8188eu/include/rtw_mlme_ext.h
> > @@ -432,9 +432,9 @@ void update_network(struct wlan_bssid_ex *dst, struct wlan_bssid_ex *src,
> >  u8 *get_my_bssid(struct wlan_bssid_ex *pnetwork);
> >  u16 get_beacon_interval(struct wlan_bssid_ex *bss);
> >  
> > -int is_client_associated_to_ap(struct adapter *padapter);
> > -int is_client_associated_to_ibss(struct adapter *padapter);
> > -int is_IBSS_empty(struct adapter *padapter);
> > +bool is_client_associated_to_ap(struct adapter *padapter);
> > +bool is_client_associated_to_ibss(struct adapter *padapter);
> > +bool is_IBSS_empty(struct adapter *padapter);
> >  
> >  unsigned char check_assoc_AP(u8 *pframe, uint len);
> >  
> > -- 
> > 2.38.0
> > 
> 
> Hi Michael,
> 
> Thanks for the patch. Just my personal opinion, but I would prefer to
> keep the return type as int, and have 0 for success and then _FAIL
> replaced with an appropriate error code depending on the circumstance,
> (e.g. -ENOMEM). This is generally the convention elsewhere in the
> kernel. Others may not agree though.

For functions that are just returning true/false like this (and the
function name is something like "is_foo()"), a boolean type is fine and
makes more sense as you will only check it in an "if ()" statement block
like how these are being used.

However, these function names are horrible for being in the global
namespace, but that's a totally different issue that can be fixed up in
a later patch.

thanks,

greg k-h

Re: [PATCH] staging: r8188eu: convert three functions to bool

[Michael Straube]

On 11/5/22 12:25, Greg KH wrote:
> However, these function names are horrible for being in the global
> namespace, but that's a totally different issue that can be fixed up in
> a later patch.
> 

I'll send a patch to address that after this patch is accepted.

Thanks,
Michael

Re: [PATCH] staging: r8188eu: convert three functions to bool

[Philipp Hortmann]

On 11/5/22 10:39, Michael Straube wrote:
> The functions
> 
> is_client_associated_to_ap()
> is_client_associated_to_ibss()
> is_IBSS_empty()
> 
> return boolean values. Convert their return type to bool and replace
> _FAIL, which is defined as 0, with false. Another step to get rid of
> _SUCCESS / _FAIL.
> 
> Signed-off-by: Michael Straube <straube.linux@gmail.com>
> ---
>   drivers/staging/r8188eu/core/rtw_wlan_util.c   | 18 +++++++++---------
>   drivers/staging/r8188eu/include/rtw_mlme_ext.h |  6 +++---
>   2 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/core/rtw_wlan_util.c b/drivers/staging/r8188eu/core/rtw_wlan_util.c
> index e50631848cab..c95438a12b59 100644
> --- a/drivers/staging/r8188eu/core/rtw_wlan_util.c
> +++ b/drivers/staging/r8188eu/core/rtw_wlan_util.c
> @@ -331,35 +331,35 @@ u16 get_beacon_interval(struct wlan_bssid_ex *bss)
>   	return le16_to_cpu(val);
>   }
>   
> -int is_client_associated_to_ap(struct adapter *padapter)
> +bool is_client_associated_to_ap(struct adapter *padapter)
>   {
>   	struct mlme_ext_priv	*pmlmeext;
>   	struct mlme_ext_info	*pmlmeinfo;
>   
>   	if (!padapter)
> -		return _FAIL;
> +		return false;
>   
>   	pmlmeext = &padapter->mlmeextpriv;
>   	pmlmeinfo = &pmlmeext->mlmext_info;
>   
>   	if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) && ((pmlmeinfo->state & 0x03) == WIFI_FW_STATION_STATE))
>   		return true;
> -	else
> -		return _FAIL;
> +
> +	return false;
>   }
>   
> -int is_client_associated_to_ibss(struct adapter *padapter)
> +bool is_client_associated_to_ibss(struct adapter *padapter)
>   {
>   	struct mlme_ext_priv	*pmlmeext = &padapter->mlmeextpriv;
>   	struct mlme_ext_info	*pmlmeinfo = &pmlmeext->mlmext_info;
>   
>   	if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) && ((pmlmeinfo->state & 0x03) == WIFI_FW_ADHOC_STATE))
>   		return true;
> -	else
> -		return _FAIL;
> +
> +	return false;
>   }
>   
> -int is_IBSS_empty(struct adapter *padapter)
> +bool is_IBSS_empty(struct adapter *padapter)
>   {
>   	unsigned int i;
>   	struct mlme_ext_priv	*pmlmeext = &padapter->mlmeextpriv;
> @@ -367,7 +367,7 @@ int is_IBSS_empty(struct adapter *padapter)
>   
>   	for (i = IBSS_START_MAC_ID; i < NUM_STA; i++) {
>   		if (pmlmeinfo->FW_sta_info[i].status == 1)
> -			return _FAIL;
> +			return false;
>   	}
>   	return true;
>   }
> diff --git a/drivers/staging/r8188eu/include/rtw_mlme_ext.h b/drivers/staging/r8188eu/include/rtw_mlme_ext.h
> index e234a3b9af6f..7652e72a03f4 100644
> --- a/drivers/staging/r8188eu/include/rtw_mlme_ext.h
> +++ b/drivers/staging/r8188eu/include/rtw_mlme_ext.h
> @@ -432,9 +432,9 @@ void update_network(struct wlan_bssid_ex *dst, struct wlan_bssid_ex *src,
>   u8 *get_my_bssid(struct wlan_bssid_ex *pnetwork);
>   u16 get_beacon_interval(struct wlan_bssid_ex *bss);
>   
> -int is_client_associated_to_ap(struct adapter *padapter);
> -int is_client_associated_to_ibss(struct adapter *padapter);
> -int is_IBSS_empty(struct adapter *padapter);
> +bool is_client_associated_to_ap(struct adapter *padapter);
> +bool is_client_associated_to_ibss(struct adapter *padapter);
> +bool is_IBSS_empty(struct adapter *padapter);
>   
>   unsigned char check_assoc_AP(u8 *pframe, uint len);
>   
Tested-by: Philipp Hortmann <philipp.g.hortmann@gmail.com> # Edimax N150

Re: [PATCH] staging: r8188eu: convert three functions to bool

[Joe Perches]

On Sat, 2022-11-05 at 10:39 +0100, Michael Straube wrote:
> The functions
> 
> is_client_associated_to_ap()
> is_client_associated_to_ibss()
> is_IBSS_empty()
> 
> return boolean values. Convert their return type to bool and replace
> _FAIL, which is defined as 0, with false. Another step to get rid of
> _SUCCESS / _FAIL.

yay.

> diff --git a/drivers/staging/r8188eu/core/rtw_wlan_util.c b/drivers/staging/r8188eu/core/rtw_wlan_util.c
[]
> +bool is_client_associated_to_ap(struct adapter *padapter)
>  {
>  	struct mlme_ext_priv	*pmlmeext;
>  	struct mlme_ext_info	*pmlmeinfo;
>  
>  	if (!padapter)
> -		return _FAIL;
> +		return false;
>  
>  	pmlmeext = &padapter->mlmeextpriv;
>  	pmlmeinfo = &pmlmeext->mlmext_info;
>  
>  	if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) && ((pmlmeinfo->state & 0x03) == WIFI_FW_STATION_STATE))
>  		return true;
> -	else
> -		return _FAIL;
> +
> +	return false;

instead of

	if (foo)
		return true;
	return false;

These could be:

	return (pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) &&
	       ((pmlmeinfo->state & 0x03) == WIFI_FW_STATION_STATE);

> +bool is_client_associated_to_ibss(struct adapter *padapter)
>  {
>  	struct mlme_ext_priv	*pmlmeext = &padapter->mlmeextpriv;
>  	struct mlme_ext_info	*pmlmeinfo = &pmlmeext->mlmext_info;
>  
>  	if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) && ((pmlmeinfo->state & 0x03) == WIFI_FW_ADHOC_STATE))
>  		return true;
> -	else
> -		return _FAIL;
> +
> +	return false;

and

	return (pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) &&
	       ((pmlmeinfo->state & 0x03) == WIFI_FW_ADHOC_STATE);

Re: [PATCH] staging: r8188eu: convert three functions to bool

[Michael Straube]

On 11/6/22 09:58, Joe Perches wrote:
> On Sat, 2022-11-05 at 10:39 +0100, Michael Straube wrote:
>> The functions
>>
>> is_client_associated_to_ap()
>> is_client_associated_to_ibss()
>> is_IBSS_empty()
>>
>> return boolean values. Convert their return type to bool and replace
>> _FAIL, which is defined as 0, with false. Another step to get rid of
>> _SUCCESS / _FAIL.
> 
> yay.
> 
>> diff --git a/drivers/staging/r8188eu/core/rtw_wlan_util.c b/drivers/staging/r8188eu/core/rtw_wlan_util.c
> []
>> +bool is_client_associated_to_ap(struct adapter *padapter)
>>   {
>>   	struct mlme_ext_priv	*pmlmeext;
>>   	struct mlme_ext_info	*pmlmeinfo;
>>   
>>   	if (!padapter)
>> -		return _FAIL;
>> +		return false;
>>   
>>   	pmlmeext = &padapter->mlmeextpriv;
>>   	pmlmeinfo = &pmlmeext->mlmext_info;
>>   
>>   	if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) && ((pmlmeinfo->state & 0x03) == WIFI_FW_STATION_STATE))
>>   		return true;
>> -	else
>> -		return _FAIL;
>> +
>> +	return false;
> 
> instead of
> 
> 	if (foo)
> 		return true;
> 	return false;
> 
> These could be:
> 
> 	return (pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) &&
> 	       ((pmlmeinfo->state & 0x03) == WIFI_FW_STATION_STATE);
> 
>> +bool is_client_associated_to_ibss(struct adapter *padapter)
>>   {
>>   	struct mlme_ext_priv	*pmlmeext = &padapter->mlmeextpriv;
>>   	struct mlme_ext_info	*pmlmeinfo = &pmlmeext->mlmext_info;
>>   
>>   	if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) && ((pmlmeinfo->state & 0x03) == WIFI_FW_ADHOC_STATE))
>>   		return true;
>> -	else
>> -		return _FAIL;
>> +
>> +	return false;
> 
> and
> 
> 	return (pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) &&
> 	       ((pmlmeinfo->state & 0x03) == WIFI_FW_ADHOC_STATE);
> 
> 

I'll keep this in mind for a future patch.

Thanks,
Michael