* [PATCH v3 0/2] Staging: Use put_unaligned_le16 @ 2015-02-20 3:30 Vaishali Thakkar 2015-02-20 3:30 ` [PATCH v3 1/2] Staging: rtl8188eu: " Vaishali Thakkar 2015-02-20 3:31 ` [PATCH v3 2/2] Staging: rtl8723au: " Vaishali Thakkar 0 siblings, 2 replies; 12+ messages in thread From: Vaishali Thakkar @ 2015-02-20 3:30 UTC (permalink / raw) To: outreachy-kernel This patch series introduces the use of function put_unaligned_le16 as Using byte ordering functions and then memcpy() is risky and prone to hide errors which are hard to track down. This change is done using Coccinelle. Changes in v3: -Two separate patches are merged in a patch series Vaishali Thakkar (2): Staging: rtl8188eu: Use put_unaligned_le16 Staging: rtl8723au: Use put_unaligned_le16 drivers/staging/rtl8188eu/core/rtw_ap.c | 8 +++----- drivers/staging/rtl8723au/core/rtw_ap.c | 8 +++----- 2 files changed, 6 insertions(+), 10 deletions(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 1/2] Staging: rtl8188eu: Use put_unaligned_le16 2015-02-20 3:30 [PATCH v3 0/2] Staging: Use put_unaligned_le16 Vaishali Thakkar @ 2015-02-20 3:30 ` Vaishali Thakkar 2015-02-20 7:24 ` [Outreachy kernel] " Preeti U Murthy 2015-02-20 3:31 ` [PATCH v3 2/2] Staging: rtl8723au: " Vaishali Thakkar 1 sibling, 1 reply; 12+ messages in thread From: Vaishali Thakkar @ 2015-02-20 3:30 UTC (permalink / raw) To: outreachy-kernel 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 v2: -Edit commit log -Merge this patch in a patch-series Changes since v1: -Remove use of variable tim_bitmap_le -Remove variable tim_bitmap_le -Add header file to be compatible with the changes -Edit commit log drivers/staging/rtl8188eu/core/rtw_ap.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c index da19145..e65ee6e 100644 --- a/drivers/staging/rtl8188eu/core/rtw_ap.c +++ b/drivers/staging/rtl8188eu/core/rtw_ap.c @@ -23,6 +23,7 @@ #include <drv_types.h> #include <wifi.h> #include <ieee80211.h> +#include <asm/unaligned.h> #ifdef CONFIG_88EU_AP_MODE @@ -78,11 +79,8 @@ static void update_BCNTIM(struct adapter *padapter) if (true) { u8 *p, *dst_ie, *premainder_ie = NULL; u8 *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_ie(pie + _FIXED_IE_LENGTH_, _TIM_IE_, &tim_ielen, pnetwork_mlmeext->IELength - _FIXED_IE_LENGTH_); if (p != NULL && tim_ielen > 0) { tim_ielen += 2; @@ -137,9 +135,9 @@ static void update_BCNTIM(struct 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Outreachy kernel] [PATCH v3 1/2] Staging: rtl8188eu: Use put_unaligned_le16 2015-02-20 3:30 ` [PATCH v3 1/2] Staging: rtl8188eu: " Vaishali Thakkar @ 2015-02-20 7:24 ` Preeti U Murthy 2015-02-20 7:26 ` Vaishali Thakkar 0 siblings, 1 reply; 12+ messages in thread From: Preeti U Murthy @ 2015-02-20 7:24 UTC (permalink / raw) To: outreachy-kernel, Vaishali Thakkar Hi Vaishali, On 02/20/2015 09:00 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. You don't really need to explain the inclusion of header files since it would be understood that it is to link the newly introduced functions in the patch. But you don't need to send out a new version just for correcting this. The patchset looks good to me. Reviewed-by: Preeti U Murthy <preeti@linux.vnet.ibm.com> > > 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 v2: > -Edit commit log > -Merge this patch in a patch-series > > Changes since v1: > -Remove use of variable tim_bitmap_le > -Remove variable tim_bitmap_le > -Add header file to be compatible with the changes > -Edit commit log > > drivers/staging/rtl8188eu/core/rtw_ap.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c > index da19145..e65ee6e 100644 > --- a/drivers/staging/rtl8188eu/core/rtw_ap.c > +++ b/drivers/staging/rtl8188eu/core/rtw_ap.c > @@ -23,6 +23,7 @@ > #include <drv_types.h> > #include <wifi.h> > #include <ieee80211.h> > +#include <asm/unaligned.h> > > #ifdef CONFIG_88EU_AP_MODE > > @@ -78,11 +79,8 @@ static void update_BCNTIM(struct adapter *padapter) > if (true) { > u8 *p, *dst_ie, *premainder_ie = NULL; > u8 *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_ie(pie + _FIXED_IE_LENGTH_, _TIM_IE_, &tim_ielen, pnetwork_mlmeext->IELength - _FIXED_IE_LENGTH_); > if (p != NULL && tim_ielen > 0) { > tim_ielen += 2; > @@ -137,9 +135,9 @@ static void update_BCNTIM(struct 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; > } > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Outreachy kernel] [PATCH v3 1/2] Staging: rtl8188eu: Use put_unaligned_le16 2015-02-20 7:24 ` [Outreachy kernel] " Preeti U Murthy @ 2015-02-20 7:26 ` Vaishali Thakkar 0 siblings, 0 replies; 12+ messages in thread From: Vaishali Thakkar @ 2015-02-20 7:26 UTC (permalink / raw) To: Preeti U Murthy; +Cc: outreachy-kernel On Fri, Feb 20, 2015 at 12:54 PM, Preeti U Murthy <preeti@linux.vnet.ibm.com> wrote: > Hi Vaishali, Hello Preeti > On 02/20/2015 09:00 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. > > You don't really need to explain the inclusion of header files since it > would be understood that it is to link the newly introduced functions in > the patch. But you don't need to send out a new version just for > correcting this. The patchset looks good to me. Ok. I will keep this in mind for next patches. Thank You > Reviewed-by: Preeti U Murthy <preeti@linux.vnet.ibm.com> >> >> 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 v2: >> -Edit commit log >> -Merge this patch in a patch-series >> >> Changes since v1: >> -Remove use of variable tim_bitmap_le >> -Remove variable tim_bitmap_le >> -Add header file to be compatible with the changes >> -Edit commit log >> >> drivers/staging/rtl8188eu/core/rtw_ap.c | 8 +++----- >> 1 file changed, 3 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c >> index da19145..e65ee6e 100644 >> --- a/drivers/staging/rtl8188eu/core/rtw_ap.c >> +++ b/drivers/staging/rtl8188eu/core/rtw_ap.c >> @@ -23,6 +23,7 @@ >> #include <drv_types.h> >> #include <wifi.h> >> #include <ieee80211.h> >> +#include <asm/unaligned.h> >> >> #ifdef CONFIG_88EU_AP_MODE >> >> @@ -78,11 +79,8 @@ static void update_BCNTIM(struct adapter *padapter) >> if (true) { >> u8 *p, *dst_ie, *premainder_ie = NULL; >> u8 *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_ie(pie + _FIXED_IE_LENGTH_, _TIM_IE_, &tim_ielen, pnetwork_mlmeext->IELength - _FIXED_IE_LENGTH_); >> if (p != NULL && tim_ielen > 0) { >> tim_ielen += 2; >> @@ -137,9 +135,9 @@ static void update_BCNTIM(struct 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; >> } >> > -- Vaishali ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 2/2] Staging: rtl8723au: Use put_unaligned_le16 2015-02-20 3:30 [PATCH v3 0/2] Staging: Use put_unaligned_le16 Vaishali Thakkar 2015-02-20 3:30 ` [PATCH v3 1/2] Staging: rtl8188eu: " Vaishali Thakkar @ 2015-02-20 3:31 ` Vaishali Thakkar 2015-02-20 7:17 ` [Outreachy kernel] " Julia Lawall 2015-02-20 7:25 ` Preeti U Murthy 1 sibling, 2 replies; 12+ messages in thread From: Vaishali Thakkar @ 2015-02-20 3:31 UTC (permalink / raw) To: outreachy-kernel 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; } -- 1.9.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Outreachy kernel] [PATCH v3 2/2] Staging: rtl8723au: Use put_unaligned_le16 2015-02-20 3:31 ` [PATCH v3 2/2] Staging: rtl8723au: " Vaishali Thakkar @ 2015-02-20 7:17 ` Julia Lawall 2015-02-20 7:24 ` Vaishali Thakkar 2015-02-20 7:25 ` Preeti U Murthy 1 sibling, 1 reply; 12+ messages in thread From: Julia Lawall @ 2015-02-20 7:17 UTC (permalink / raw) To: Vaishali Thakkar; +Cc: outreachy-kernel On Fri, 20 Feb 2015, 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 So in this case there is no change since v2? Maybe it would be better to make that explicit. julia > 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 > > -- > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. > To post to this group, send email to outreachy-kernel@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/9defe79e0f212c883f260e27a67bff9f9ba013ba.1424402536.git.vthakkar1994%40gmail.com. > For more options, visit https://groups.google.com/d/optout. > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Outreachy kernel] [PATCH v3 2/2] Staging: rtl8723au: Use put_unaligned_le16 2015-02-20 7:17 ` [Outreachy kernel] " Julia Lawall @ 2015-02-20 7:24 ` Vaishali Thakkar 2015-02-20 7:38 ` Preeti U Murthy 0 siblings, 1 reply; 12+ messages in thread From: Vaishali Thakkar @ 2015-02-20 7:24 UTC (permalink / raw) To: Julia Lawall; +Cc: outreachy-kernel On Fri, Feb 20, 2015 at 12:47 PM, Julia Lawall <julia.lawall@lip6.fr> wrote: > On Fri, 20 Feb 2015, 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 > > So in this case there is no change since v2? Maybe it would be better to > make that explicit. Actually there is no v2 of this patch but as this patch is merged in a series with v3 of patch Staging: rtl8188eu: Use put_unaligned_le16 it comes as a v3. Should I clear this in a cover letter and then resend the patch series? > julia > > >> 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 >> >> -- >> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. >> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. >> To post to this group, send email to outreachy-kernel@googlegroups.com. >> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/9defe79e0f212c883f260e27a67bff9f9ba013ba.1424402536.git.vthakkar1994%40gmail.com. >> For more options, visit https://groups.google.com/d/optout. >> -- Vaishali ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Outreachy kernel] [PATCH v3 2/2] Staging: rtl8723au: Use put_unaligned_le16 2015-02-20 7:24 ` Vaishali Thakkar @ 2015-02-20 7:38 ` Preeti U Murthy 2015-02-20 7:50 ` Vaishali Thakkar 0 siblings, 1 reply; 12+ messages in thread From: Preeti U Murthy @ 2015-02-20 7:38 UTC (permalink / raw) To: Vaishali Thakkar, Julia Lawall; +Cc: outreachy-kernel On 02/20/2015 12:54 PM, Vaishali Thakkar wrote: > On Fri, Feb 20, 2015 at 12:47 PM, Julia Lawall <julia.lawall@lip6.fr> wrote: >> On Fri, 20 Feb 2015, 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 >> >> So in this case there is no change since v2? Maybe it would be better to >> make that explicit. > > Actually there is no v2 of this patch but as this patch is merged in a > series with > v3 of patch Staging: rtl8188eu: Use put_unaligned_le16 it comes as a v3. > > Should I clear this in a cover letter and then resend the patch series? Yes I would suggest that. I missed this. Additionally I would recommend in this case to not prefix any version number to the cover letter. Instead prefix the appropriate version numbers to the individual patches mentioning the changes from the previous versions. Regards Preeti U Murthy > >> julia >> >> >>> 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 >>> >>> -- >>> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. >>> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. >>> To post to this group, send email to outreachy-kernel@googlegroups.com. >>> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/9defe79e0f212c883f260e27a67bff9f9ba013ba.1424402536.git.vthakkar1994%40gmail.com. >>> For more options, visit https://groups.google.com/d/optout. >>> > > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Outreachy kernel] [PATCH v3 2/2] Staging: rtl8723au: Use put_unaligned_le16 2015-02-20 7:38 ` Preeti U Murthy @ 2015-02-20 7:50 ` Vaishali Thakkar 2015-02-20 8:02 ` Preeti U Murthy 0 siblings, 1 reply; 12+ messages in thread From: Vaishali Thakkar @ 2015-02-20 7:50 UTC (permalink / raw) To: Preeti U Murthy; +Cc: Julia Lawall, outreachy-kernel On Fri, Feb 20, 2015 at 1:08 PM, Preeti U Murthy <preeti@linux.vnet.ibm.com> wrote: > > On 02/20/2015 12:54 PM, Vaishali Thakkar wrote: >> On Fri, Feb 20, 2015 at 12:47 PM, Julia Lawall <julia.lawall@lip6.fr> wrote: >>> On Fri, 20 Feb 2015, 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 >>> >>> So in this case there is no change since v2? Maybe it would be better to >>> make that explicit. >> >> Actually there is no v2 of this patch but as this patch is merged in a >> series with >> v3 of patch Staging: rtl8188eu: Use put_unaligned_le16 it comes as a v3. >> >> Should I clear this in a cover letter and then resend the patch series? > > Yes I would suggest that. I missed this. Additionally I would recommend > in this case to not prefix any version number to the cover letter. > Instead prefix the appropriate version numbers to the individual patches > mentioning the changes from the previous versions. Ok. So, in next patch-series cover letter will not have any version number and both of these patches will have v4 as I already prefixed them as v3. And for this particular patch I should write that there is no v2 under signed off by line. Right?? > Regards > Preeti U Murthy >> >>> julia >>> >>> >>>> 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 >>>> >>>> -- >>>> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. >>>> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. >>>> To post to this group, send email to outreachy-kernel@googlegroups.com. >>>> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/9defe79e0f212c883f260e27a67bff9f9ba013ba.1424402536.git.vthakkar1994%40gmail.com. >>>> For more options, visit https://groups.google.com/d/optout. >>>> >> >> >> > -- Vaishali ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Outreachy kernel] [PATCH v3 2/2] Staging: rtl8723au: Use put_unaligned_le16 2015-02-20 7:50 ` Vaishali Thakkar @ 2015-02-20 8:02 ` Preeti U Murthy 2015-02-20 8:27 ` Vaishali Thakkar 0 siblings, 1 reply; 12+ messages in thread From: Preeti U Murthy @ 2015-02-20 8:02 UTC (permalink / raw) To: Vaishali Thakkar; +Cc: Julia Lawall, outreachy-kernel On 02/20/2015 01:20 PM, Vaishali Thakkar wrote: >> >> Yes I would suggest that. I missed this. Additionally I would recommend >> in this case to not prefix any version number to the cover letter. >> Instead prefix the appropriate version numbers to the individual patches >> mentioning the changes from the previous versions. > > Ok. So, in next patch-series cover letter will not have any version number and > both of these patches will have v4 as I already prefixed them as v3. > And for this > particular patch I should write that there is no v2 under signed off > by line. Right?? 1. No version number for the cover letter. 2. Retain [PATCH v3 1/2] Staging: rtl8188eu: Use put_unaligned_le16 as v3 itself. 3. Change [PATCH v3 2/2] Staging: rtl8723au: Use put_unaligned_le16: to v2. So it would be: [PATCH v2 2/2] Staging: rtl8723au: Use put_unaligned_le16, since it has undergone just one revision. The reason I ask not to change the version numbers again is because there is nothing that has significantly changed from the previous versions. There will be a change only in the manner of sending patches. So now the question is about how you indicate to the maintainer that the fresh series that you will post now is the one to be picked up. Usually under such circumstances we append RESEND to the patches. So now you can resend the next version as mentioned in the points above but with a RESEND tag to the cover letter and patches. Something like: [RESEND PATCH 0/2] Staging: Use put_unaligned_le16. The maintainer will thus know that this is the one to be picked up. Regards Preeti U Murthy > >> Regards >> Preeti U Murthy ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Outreachy kernel] [PATCH v3 2/2] Staging: rtl8723au: Use put_unaligned_le16 2015-02-20 8:02 ` Preeti U Murthy @ 2015-02-20 8:27 ` Vaishali Thakkar 0 siblings, 0 replies; 12+ messages in thread From: Vaishali Thakkar @ 2015-02-20 8:27 UTC (permalink / raw) To: Preeti U Murthy; +Cc: Julia Lawall, outreachy-kernel On Fri, Feb 20, 2015 at 1:32 PM, Preeti U Murthy <preeti@linux.vnet.ibm.com> wrote: > On 02/20/2015 01:20 PM, Vaishali Thakkar wrote: >>> >>> Yes I would suggest that. I missed this. Additionally I would recommend >>> in this case to not prefix any version number to the cover letter. >>> Instead prefix the appropriate version numbers to the individual patches >>> mentioning the changes from the previous versions. >> >> Ok. So, in next patch-series cover letter will not have any version number and >> both of these patches will have v4 as I already prefixed them as v3. >> And for this >> particular patch I should write that there is no v2 under signed off >> by line. Right?? > > 1. No version number for the cover letter. > 2. Retain [PATCH v3 1/2] Staging: rtl8188eu: Use put_unaligned_le16 as > v3 itself. > 3. Change [PATCH v3 2/2] Staging: rtl8723au: Use put_unaligned_le16: to > v2. So it would be: > [PATCH v2 2/2] Staging: rtl8723au: Use put_unaligned_le16, since it has > undergone just one revision. > > The reason I ask not to change the version numbers again is because > there is nothing that has significantly changed from the previous > versions. There will be a change only in the manner of sending patches. > > So now the question is about how you indicate to the maintainer that the > fresh series that you will post now is the one to be picked up. Usually > under such circumstances we append RESEND to the patches. > So now you can resend the next version as mentioned in the points above > but with a RESEND tag to the cover letter and patches. > > Something like: [RESEND PATCH 0/2] Staging: Use put_unaligned_le16. > The maintainer will thus know that this is the one to be picked up. Yes. This makes sense. I was confused about how maintainer will know what to pick in these kind of situations. Thanks for clearing this. :) > Regards > Preeti U Murthy > > >> >>> Regards >>> Preeti U Murthy > -- Vaishali ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Outreachy kernel] [PATCH v3 2/2] Staging: rtl8723au: Use put_unaligned_le16 2015-02-20 3:31 ` [PATCH v3 2/2] Staging: rtl8723au: " Vaishali Thakkar 2015-02-20 7:17 ` [Outreachy kernel] " Julia Lawall @ 2015-02-20 7:25 ` Preeti U Murthy 1 sibling, 0 replies; 12+ messages in thread From: Preeti U Murthy @ 2015-02-20 7:25 UTC (permalink / raw) To: Vaishali Thakkar, outreachy-kernel 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> ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-02-20 8:27 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-02-20 3:30 [PATCH v3 0/2] Staging: Use put_unaligned_le16 Vaishali Thakkar 2015-02-20 3:30 ` [PATCH v3 1/2] Staging: rtl8188eu: " Vaishali Thakkar 2015-02-20 7:24 ` [Outreachy kernel] " Preeti U Murthy 2015-02-20 7:26 ` Vaishali Thakkar 2015-02-20 3:31 ` [PATCH v3 2/2] Staging: rtl8723au: " Vaishali Thakkar 2015-02-20 7:17 ` [Outreachy kernel] " Julia Lawall 2015-02-20 7:24 ` Vaishali Thakkar 2015-02-20 7:38 ` Preeti U Murthy 2015-02-20 7:50 ` Vaishali Thakkar 2015-02-20 8:02 ` Preeti U Murthy 2015-02-20 8:27 ` Vaishali Thakkar 2015-02-20 7:25 ` Preeti U Murthy
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.