[PATCH 1/1] drivers: staging: rtl8192u: ieee80211: ieee80211_softmac.c: auth parse error code byte order fix

[PATCH 1/1] drivers: staging: rtl8192u: ieee80211: ieee80211_softmac.c: auth parse error code byte order fix

[Tolga Ceylan]

Error code returned from auth_parse() should in cpu byte order.

Signed-off-by: Tolga Ceylan <tolga.ceylan@gmail.com>
---
 drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
index d2e8b12..0cf0e89 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
@@ -1558,7 +1558,7 @@ static inline u16 auth_parse(struct sk_buff *skb, u8 **challenge, int *chlen)
 		}
 	}
 
-	return cpu_to_le16(a->status);
+	return le16_to_cpu(a->status);
 
 }
 
-- 
2.4.0

Re: [PATCH 1/1] drivers: staging: rtl8192u: ieee80211: ieee80211_softmac.c: auth parse error code byte order fix

[tolga ceylan]

On 05/12/2015 08:55 PM, Tolga Ceylan wrote:
> Error code returned from auth_parse() should in cpu byte order.
>
> Signed-off-by: Tolga Ceylan <tolga.ceylan@gmail.com>
> ---
>   drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> index d2e8b12..0cf0e89 100644
> --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> @@ -1558,7 +1558,7 @@ static inline u16 auth_parse(struct sk_buff *skb, u8 **challenge, int *chlen)
>   		}
>   	}
>
> -	return cpu_to_le16(a->status);
> +	return le16_to_cpu(a->status);
>
>   }
>
>

I haven't gotten a response for this patch. Just checking back.

Regards,
Tolga Ceylan

Re: [PATCH 1/1] drivers: staging: rtl8192u: ieee80211: ieee80211_softmac.c: auth parse error code byte order fix

[Sudip Mukherjee]

On Sat, May 23, 2015 at 07:53:28PM -0700, tolga ceylan wrote:
> On 05/12/2015 08:55 PM, Tolga Ceylan wrote:
<snip>
> >
> >
> 
> I haven't gotten a response for this patch. Just checking back.
no response means no one is having anything to say about it. Final say
will be Greg's when he reviews that when he gets to it.

regards
sudip
> 

Re: [PATCH 1/1] drivers: staging: rtl8192u: ieee80211: ieee80211_softmac.c: auth parse error code byte order fix

[Paul Gortmaker]

On 15-05-23 10:53 PM, tolga ceylan wrote:
> On 05/12/2015 08:55 PM, Tolga Ceylan wrote:
>> Error code returned from auth_parse() should in cpu byte order.
>>
>> Signed-off-by: Tolga Ceylan <tolga.ceylan@gmail.com>
>> ---
>>   drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
>> index d2e8b12..0cf0e89 100644
>> --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
>> +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
>> @@ -1558,7 +1558,7 @@ static inline u16 auth_parse(struct sk_buff *skb, u8 **challenge, int *chlen)
>>   		}
>>   	}
>>
>> -	return cpu_to_le16(a->status);
>> +	return le16_to_cpu(a->status);
>>
>>   }
>>
>>
> 
> I haven't gotten a response for this patch. Just checking back.

In the future, you can help speed up the review process by helping
the reviewer/maintainer.  How?  In your commit log.  Describe the 
end user symptoms and how the problem was found, why it happens
and what is the best fix for the problem (assuming it isn't trivially
obvious).  So, instead of just translating the C change into words
for a commit log like you had above, you could have instead had...

	When trying to use an RL8192 with auth mode foo and module
	parameter blah blah, the driver fails to connect, only showing
	blah blah in the logs.

	This happens because auth_parse blah blah and returns blah
	which is incorrectly interpreted by foo_bar() since it is in
	the wrong endian as blah is in native byte order and ...

	We fix it by noting the error value should be ... since ...


Also, minor nit ; if you are going to put a path fragment in the subject,
then don't replace all the / with ": " --- i.e. use
	
 [PATCH] staging: rtl8192u/ieee80211/ieee80211_softmac.c: auth parse error code byte order fix

or 

 [PATCH] staging/rtl8192u: ieee80211_softmac.c - auth parse error code byte order fix

Paul.
--

> 
> Regards,
> Tolga Ceylan
> 

Re: [PATCH 1/1] drivers: staging: rtl8192u: ieee80211: ieee80211_softmac.c: auth parse error code byte order fix

[Dan Carpenter]

Good advice.  This was actually found by Sparse and the changelog should
say:

Sparse complains about this:

ieee80211_softmac.c:1561:16: warning: cast from restricted __le16
ieee80211_softmac.c:1561:16: warning: incorrect type in return expression (different base types)
ieee80211_softmac.c:1561:16:    expected unsigned short
ieee80211_softmac.c:1561:16:    got restricted __le16 [usertype] <noident>

The caller basically only cares about zero and non-zero so it doesn't
affect runtime (except for making an debug message more accurate).

regards,
dan carpenter

Re: [PATCH 1/1] drivers: staging: rtl8192u: ieee80211: ieee80211_softmac.c: auth parse error code byte order fix

[Tolga Ceylan]

I appreciate the suggestions.

I simply grepped this piece of code by chance while looking at an
actual sparse warning somewhere else. There's no
symptoms and I'm not sure under what conditions this debug statement
is printed nor why/when. But this is an obviously buggy
code only impacting debug/trace statements, but it could become more
serious if/when someone wants to rely on this return code
in the future. I'll expand the terse description and resubmit.

Cheers,
Tolga