Re: [Outreachy kernel] [PATCH v3 2/2] Staging: rtl8723au: Use put_unaligned_le16

[Preeti U Murthy <preeti@linux.vnet.ibm.com>]

On 02/20/2015 09:01 AM, Vaishali Thakkar wrote:
> 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. Here, use of variable tim_bitmap_le
> and variable itself is removed. Also, to be compatible with the
> changes header file is added too.
> 
> Coccinelle is used to do this change and semantic patch used for
> this is as follows:
> 
> @a@
> typedef __le16;
> __le16 e16;
> identifier tmp;
> expression ptr;
> expression y,e;
> type T;
> @@
> 
> - tmp = cpu_to_le16(y);
> 
> <+... when != tmp
> (
> - memcpy(ptr, (T)&tmp, \(2\|sizeof(__le16)\|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
> 
> Signed-off-by: Vaishali Thakkar <vthakkar1994@gmail.com>
> ---
> Changes since v1:
> 	-Edit commit log
> 	-Merge this patch in a patch series
> 
>  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;
>  	}
> 
Reviewed-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>