[OPW,kernel,v3,01/28] staging: rtl8723au: core: rtw_ap.c: Replace non-standard return values
diff mbox

Message ID 1414668396-4093-2-git-send-email-roberta.dobrescu@gmail.com
State New, archived
Headers show

Commit Message

Roberta Dobrescu Oct. 30, 2014, 11:26 a.m. UTC
This patch replaces non-standard return values _SUCCESS/_FAIL with 0
or an appropriate err code and modifies the code according to that.

Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>
---
 drivers/staging/rtl8723au/core/rtw_ap.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

Comments

Greg KH Oct. 30, 2014, 8:03 p.m. UTC | #1
On Thu, Oct 30, 2014 at 01:26:09PM +0200, Roberta Dobrescu wrote:
> This patch replaces non-standard return values _SUCCESS/_FAIL with 0
> or an appropriate err code and modifies the code according to that.
> 
> Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>
> ---
>  drivers/staging/rtl8723au/core/rtw_ap.c | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723au/core/rtw_ap.c b/drivers/staging/rtl8723au/core/rtw_ap.c
> index e394d12..c4c8b92 100644
> --- a/drivers/staging/rtl8723au/core/rtw_ap.c
> +++ b/drivers/staging/rtl8723au/core/rtw_ap.c
> @@ -293,8 +293,7 @@ void	expire_timeout_chk23a(struct rtw_adapter *padapter)
>  
>  	/* issue null data to check sta alive*/
>  	for (i = 0; i < chk_alive_num; i++) {
> -
> -		int ret = _FAIL;
> +		int ret = -ETIMEDOUT;
>  
>  		psta = chk_alive_list[i];
>  		if (!(psta->state & _FW_LINKED))
> @@ -306,7 +305,7 @@ void	expire_timeout_chk23a(struct rtw_adapter *padapter)
>  			ret = issue_nulldata23a(padapter, psta->hwaddr, 0, 3, 50);
>  
>  		psta->keep_alive_trycnt++;
> -		if (ret == _SUCCESS) {
> +		if (ret == 0) {

This isn't going to work.

ret will be set to _SUCCESS from the calls above this line, so the code
is now broken in this patch.

This is going to be harder than to just break the changes up into one
file at a time.  You need to fix up one _function_ at a time, and all of
the callers of that function, in order to keep everything working
properly.

So I can't take this as-is, sorry.

greg k-h
Roberta Dobrescu Oct. 30, 2014, 9 p.m. UTC | #2
On 30.10.2014 22:03, Greg KH wrote:
> On Thu, Oct 30, 2014 at 01:26:09PM +0200, Roberta Dobrescu wrote:
>> This patch replaces non-standard return values _SUCCESS/_FAIL with 0
>> or an appropriate err code and modifies the code according to that.
>>
>> Signed-off-by: Roberta Dobrescu <roberta.dobrescu@gmail.com>
>> ---
>>   drivers/staging/rtl8723au/core/rtw_ap.c | 21 ++++++++++-----------
>>   1 file changed, 10 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/staging/rtl8723au/core/rtw_ap.c b/drivers/staging/rtl8723au/core/rtw_ap.c
>> index e394d12..c4c8b92 100644
>> --- a/drivers/staging/rtl8723au/core/rtw_ap.c
>> +++ b/drivers/staging/rtl8723au/core/rtw_ap.c
>> @@ -293,8 +293,7 @@ void	expire_timeout_chk23a(struct rtw_adapter *padapter)
>>
>>   	/* issue null data to check sta alive*/
>>   	for (i = 0; i < chk_alive_num; i++) {
>> -
>> -		int ret = _FAIL;
>> +		int ret = -ETIMEDOUT;
>>
>>   		psta = chk_alive_list[i];
>>   		if (!(psta->state & _FW_LINKED))
>> @@ -306,7 +305,7 @@ void	expire_timeout_chk23a(struct rtw_adapter *padapter)
>>   			ret = issue_nulldata23a(padapter, psta->hwaddr, 0, 3, 50);
>>
>>   		psta->keep_alive_trycnt++;
>> -		if (ret == _SUCCESS) {
>> +		if (ret == 0) {
>
> This isn't going to work.
>
> ret will be set to _SUCCESS from the calls above this line, so the code
> is now broken in this patch.
>
> This is going to be harder than to just break the changes up into one
> file at a time.  You need to fix up one _function_ at a time, and all of
> the callers of that function, in order to keep everything working
> properly.

I understand what you're saying, that's why I did one patch per 
directory the first time, to avoid these kind of problems and even then
I think there were some broken dependencies. I just thought that in
this particular case the patchset will be applied all at once.

This driver isn't small at all, over 50K lines just *.c files and I
find the coding style far from being nice. I know it's not ok how the 
patches are now and on the other side huge patches are hard to follow.

So if you think that the best approach is fixing one function at the 
time, I'm ready to go for it even if that implies searching for all
the dependencies and solving them in order.

Thanks,
Roberta
Greg KH Oct. 30, 2014, 9:16 p.m. UTC | #3
On Thu, Oct 30, 2014 at 11:00:07PM +0200, Roberta Dobrescu wrote:
> So if you think that the best approach is fixing one function at the time,
> I'm ready to go for it even if that implies searching for all
> the dependencies and solving them in order.

Yes, that's what needs to be done to do this correctly.

thanks,

greg k-h
Roberta Dobrescu Oct. 30, 2014, 10:32 p.m. UTC | #4
On 30.10.2014 23:16, Greg KH wrote:
> On Thu, Oct 30, 2014 at 11:00:07PM +0200, Roberta Dobrescu wrote:
>> So if you think that the best approach is fixing one function at the time,
>> I'm ready to go for it even if that implies searching for all
>> the dependencies and solving them in order.
>
> Yes, that's what needs to be done to do this correctly.

Ok then. I'll think the best approach is to start with the static 
functions. If they are not many, I plan to put them all the the same
patch.

After that I'll continue with the rest of the functions.

Thanks,
Roberta

Patch
diff mbox

diff --git a/drivers/staging/rtl8723au/core/rtw_ap.c b/drivers/staging/rtl8723au/core/rtw_ap.c
index e394d12..c4c8b92 100644
--- a/drivers/staging/rtl8723au/core/rtw_ap.c
+++ b/drivers/staging/rtl8723au/core/rtw_ap.c
@@ -293,8 +293,7 @@  void	expire_timeout_chk23a(struct rtw_adapter *padapter)
 
 	/* issue null data to check sta alive*/
 	for (i = 0; i < chk_alive_num; i++) {
-
-		int ret = _FAIL;
+		int ret = -ETIMEDOUT;
 
 		psta = chk_alive_list[i];
 		if (!(psta->state & _FW_LINKED))
@@ -306,7 +305,7 @@  void	expire_timeout_chk23a(struct rtw_adapter *padapter)
 			ret = issue_nulldata23a(padapter, psta->hwaddr, 0, 3, 50);
 
 		psta->keep_alive_trycnt++;
-		if (ret == _SUCCESS) {
+		if (ret == 0) {
 			DBG_8723A("asoc check, sta(" MAC_FMT ") is alive\n", MAC_ARG(psta->hwaddr));
 			psta->expire_to = pstapriv->expire_to;
 			psta->keep_alive_trycnt = 0;
@@ -757,7 +756,7 @@  static void start_bss_network(struct rtw_adapter *padapter, u8 *pbuf)
 		update_beacon23a(padapter, WLAN_EID_TIM, NULL, false);
 
 		/* issue beacon frame */
-		if (send_beacon23a(padapter) == _FAIL)
+		if (send_beacon23a(padapter) < 0)
 			DBG_8723A("issue_beacon23a, fail!\n");
 	}
 
@@ -768,7 +767,7 @@  static void start_bss_network(struct rtw_adapter *padapter, u8 *pbuf)
 int rtw_check_beacon_data23a(struct rtw_adapter *padapter,
 			     struct ieee80211_mgmt *mgmt, unsigned int len)
 {
-	int ret = _SUCCESS;
+	int ret = 0;
 	u8 *p;
 	u8 *pHT_caps_ie = NULL;
 	u8 *pHT_info_ie = NULL;
@@ -800,10 +799,10 @@  int rtw_check_beacon_data23a(struct rtw_adapter *padapter,
 	DBG_8723A("%s, len =%d\n", __func__, len);
 
 	if (!check_fwstate(pmlmepriv, WIFI_AP_STATE))
-		return _FAIL;
+		return -EBUSY;
 
 	if (len > MAX_IE_SZ)
-		return _FAIL;
+		return -EINVAL;
 
 	pbss_network->IELength = len;
 
@@ -813,7 +812,7 @@  int rtw_check_beacon_data23a(struct rtw_adapter *padapter,
 
 	if (pbss_network->ifmode != NL80211_IFTYPE_AP &&
 	    pbss_network->ifmode != NL80211_IFTYPE_P2P_GO)
-		return _FAIL;
+		return -EOPNOTSUPP;
 
 	pbss_network->Rssi = 0;
 
@@ -880,7 +879,7 @@  int rtw_check_beacon_data23a(struct rtw_adapter *padapter,
 			  pbss_network->IELength);
 	if (p && ie_len > 0) {
 		if (rtw_parse_wpa2_ie23a(p, ie_len+2, &group_cipher,
-					 &pairwise_cipher, NULL) == _SUCCESS) {
+					 &pairwise_cipher, NULL) == 0) {
 			psecuritypriv->dot11AuthAlgrthm = dot11AuthAlgrthm_8021X;
 
 			psecuritypriv->dot8021xalg = 1;/* psk,  todo:802.1x */
@@ -902,7 +901,7 @@  int rtw_check_beacon_data23a(struct rtw_adapter *padapter,
 				  pbss_network->IELength - (ie_len + 2));
 		if ((p) && (!memcmp(p+2, RTW_WPA_OUI23A_TYPE, 4))) {
 			if (rtw_parse_wpa_ie23a(p, ie_len+2, &group_cipher,
-						&pairwise_cipher, NULL) == _SUCCESS) {
+						&pairwise_cipher, NULL) == 0) {
 				psecuritypriv->dot11AuthAlgrthm = dot11AuthAlgrthm_8021X;
 
 				/* psk,  todo:802.1x */
@@ -1013,7 +1012,7 @@  int rtw_check_beacon_data23a(struct rtw_adapter *padapter,
 					    pbss_network->MacAddress,
 					    GFP_KERNEL);
 		if (!psta)
-			return _FAIL;
+			return -ENOMEM;
 	}
 	/* fix bug of flush_cam_entry at STOP AP mode */
 	psta->state |= WIFI_AP_STATE;