[PATCH] Staging: rtl8723au: Use put_unaligned_le16

[PATCH] Staging: rtl8723au: Use put_unaligned_le16

[Vaishali Thakkar]

Using byte ordering functions and then memcpy() is risky
and prone to hide errors which are hard to track down. So,
this patch introduces the use of function put_unaligned_le16
which makes the code clear.

This is done using Coccinelle and semantic patch used is as follows:

@a@
typedef u16, __le16, uint16_t;
{u16,__le16,uint16_t} e16;
identifier tmp;
expression ptr;
expression y,e;
type T;
@@

- tmp = cpu_to_le16(y);

<+... when != tmp
(
- memcpy(ptr, (T)&tmp, \(2\|sizeof(u16)\|sizeof(__le16)\|sizeof(uint16_t)\|sizeof(e16)\));
+ put_unaligned_le16(y,ptr);
|
- memcpy(ptr, (T)&tmp, ...);
+ put_unaligned_le16(y,ptr);
)
...+>
? tmp = e

@@ type T; identifier a.tmp; @@

- T tmp;
...when != tmp

Here, use of variable tim_bitmap_le and variable itself is removed. To be compatible with
changes header file is added too.

Signed-off-by: Vaishali Thakkar <vthakkar1994@gmail.com>
---
 drivers/staging/rtl8723au/core/rtw_ap.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_ap.c b/drivers/staging/rtl8723au/core/rtw_ap.c
index c6327c0..7fa4352 100644
--- a/drivers/staging/rtl8723au/core/rtw_ap.c
+++ b/drivers/staging/rtl8723au/core/rtw_ap.c
@@ -20,6 +20,7 @@
 #include <wifi.h>
 #include <rtl8723a_cmd.h>
 #include <rtl8723a_hal.h>
+#include <asm/unaligned.h>
 
 extern unsigned char WMM_OUI23A[];
 extern unsigned char WPS_OUI23A[];
@@ -72,11 +73,8 @@ static void update_BCNTIM(struct rtw_adapter *padapter)
 	struct wlan_bssid_ex *pnetwork_mlmeext = &pmlmeinfo->network;
 	unsigned char *pie = pnetwork_mlmeext->IEs;
 	u8 *p, *dst_ie, *premainder_ie = NULL, *pbackup_remainder_ie = NULL;
-	__le16 tim_bitmap_le;
 	uint offset, tmp_len, tim_ielen, tim_ie_offset, remainder_ielen;
 
-	tim_bitmap_le = cpu_to_le16(pstapriv->tim_bitmap);
-
 	p = rtw_get_ie23a(pie, WLAN_EID_TIM, &tim_ielen,
 			  pnetwork_mlmeext->IELength);
 	if (p != NULL && tim_ielen > 0) {
@@ -143,9 +141,9 @@ static void update_BCNTIM(struct rtw_adapter *padapter)
 		*dst_ie++ = 0;
 
 	if (tim_ielen == 4) {
-		*dst_ie++ = *(u8 *)&tim_bitmap_le;
+		*dst_ie++ = pstapriv->tim_bitmap & 0xff;
 	} else if (tim_ielen == 5) {
-		memcpy(dst_ie, &tim_bitmap_le, 2);
+		put_unaligned_le16(pstapriv->tim_bitmap, dst_ie);
 		dst_ie += 2;
 	}
 
-- 
1.9.1

Re: [Outreachy kernel] [PATCH] Staging: rtl8723au: Use put_unaligned_le16

[Julia Lawall]

> Here, use of variable tim_bitmap_le and variable itself is removed. To be compatible with
> changes header file is added too.

Your commit message is too long here.

julia

Re: [Outreachy kernel] [PATCH] Staging: rtl8723au: Use put_unaligned_le16

[Jes Sorensen]

On 02/18/15 15:12, Julia Lawall wrote:
>> Here, use of variable tim_bitmap_le and variable itself is removed. To be compatible with
>> changes header file is added too.
> 
> Your commit message is too long here.
> 
> julia
> 

I agree!

In general, as a driver maintainer, I would also prefer a text message
explaining what is fixed in a, rather than automated Coccinelle output.
Adding a comment that the issue was discovered using Coccinelle is great
however.

Cheers,
Jes

Re: [Outreachy kernel] [PATCH] Staging: rtl8723au: Use put_unaligned_le16

[Vaishali Thakkar]

On Thu, Feb 19, 2015 at 2:57 AM, Jes Sorensen <jes.sorensen@gmail.com> wrote:
> On 02/18/15 15:12, Julia Lawall wrote:
>>> Here, use of variable tim_bitmap_le and variable itself is removed. To be compatible with
>>> changes header file is added too.
>>
>> Your commit message is too long here.
>>
>> julia
>>
>
> I agree!
>
> In general, as a driver maintainer, I would also prefer a text message
> explaining what is fixed in a, rather than automated Coccinelle output.
> Adding a comment that the issue was discovered using Coccinelle is great
> however.

Ok. I will split this patch in 2 patches also. And will make the commit log
short and more clear.

> Cheers,
> Jes
>



-- 
Vaishali

Re: [Outreachy kernel] [PATCH] Staging: rtl8723au: Use put_unaligned_le16

[Julia Lawall]

> Ok. I will split this patch in 2 patches also. And will make the commit log
> short and more clear.

Actually, my comment was ambiguous.  I was commenting on the fact that you 
went way over 80 characters.  I think Jes's comment was more that he 
wanted all the English text part of the commit message up at the top, 
where he would see it immediately, not buried after the semantic patch.

julia

Re: [Outreachy kernel] [PATCH] Staging: rtl8723au: Use put_unaligned_le16

[Vaishali Thakkar]

On Thu, Feb 19, 2015 at 12:54 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> Ok. I will split this patch in 2 patches also. And will make the commit log
>> short and more clear.
>
> Actually, my comment was ambiguous.  I was commenting on the fact that you
> went way over 80 characters.  I think Jes's comment was more that he
> wanted all the English text part of the commit message up at the top,
> where he would see it immediately, not buried after the semantic patch.

Yes. I agree. My line went over 80 characters. Ok. I will keep english part
first in a commit log explaining the change and then semantic patch.


> julia



-- 
Vaishali