[PATCH v2] usb: wusbcore: Use put_unaligned_le32

[PATCH v2] usb: wusbcore: Use put_unaligned_le32

[Himanshu Jha]

Use put_unaligned_le32 rather than using byte ordering function and
memcpy which makes code clear.
Also, add the header file where it is declared.

Done using Coccinelle and semantic patch used is :

@ rule1 @
identifier tmp; expression ptr,x; type T;
@@

- tmp = cpu_to_le32(x);

  <+... when != tmp
- memcpy(ptr, (T)&tmp, ...);
+ put_unaligned_le32(x,ptr);
  ...+>

@ depends on rule1 @
type j; identifier tmp;
@@

- j tmp;
  ...when != tmp

Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---
v2:
* added correct header file.

 drivers/usb/wusbcore/security.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/wusbcore/security.c b/drivers/usb/wusbcore/security.c
index 170f2c3..256b56f 100644
--- a/drivers/usb/wusbcore/security.c
+++ b/drivers/usb/wusbcore/security.c
@@ -22,6 +22,7 @@
  *
  * FIXME: docs
  */
+#include <asm/unaligned.h>
 #include <linux/types.h>
 #include <linux/slab.h>
 #include <linux/usb/ch9.h>
@@ -367,7 +368,6 @@ int wusb_dev_4way_handshake(struct wusbhc *wusbhc, struct wusb_dev *wusb_dev,
 	struct usb_device *usb_dev = wusb_dev->usb_dev;
 	struct device *dev = &usb_dev->dev;
 	u32 tkid;
-	__le32 tkid_le;
 	struct usb_handshake *hs;
 	struct aes_ccm_nonce ccm_n;
 	u8 mic[8];
@@ -385,11 +385,10 @@ int wusb_dev_4way_handshake(struct wusbhc *wusbhc, struct wusb_dev *wusb_dev,
 		goto error_dev_set_encryption;
 
 	tkid = wusbhc_next_tkid(wusbhc, wusb_dev);
-	tkid_le = cpu_to_le32(tkid);
 
 	hs[0].bMessageNumber = 1;
 	hs[0].bStatus = 0;
-	memcpy(hs[0].tTKID, &tkid_le, sizeof(hs[0].tTKID));
+	put_unaligned_le32(tkid, hs[0].tTKID);
 	hs[0].bReserved = 0;
 	memcpy(hs[0].CDID, &wusb_dev->cdid, sizeof(hs[0].CDID));
 	get_random_bytes(&hs[0].nonce, sizeof(hs[0].nonce));
@@ -441,7 +440,7 @@ int wusb_dev_4way_handshake(struct wusbhc *wusbhc, struct wusb_dev *wusb_dev,
 
 	/* Setup the CCM nonce */
 	memset(&ccm_n.sfn, 0, sizeof(ccm_n.sfn)); /* Per WUSB1.0[6.5.2] */
-	memcpy(ccm_n.tkid, &tkid_le, sizeof(ccm_n.tkid));
+	put_unaligned_le32(tkid, ccm_n.tkid);
 	ccm_n.src_addr = wusbhc->uwb_rc->uwb_dev.dev_addr;
 	ccm_n.dest_addr.data[0] = wusb_dev->addr;
 	ccm_n.dest_addr.data[1] = 0;
@@ -472,7 +471,7 @@ int wusb_dev_4way_handshake(struct wusbhc *wusbhc, struct wusb_dev *wusb_dev,
 	/* Send Handshake3 */
 	hs[2].bMessageNumber = 3;
 	hs[2].bStatus = 0;
-	memcpy(hs[2].tTKID, &tkid_le, sizeof(hs[2].tTKID));
+	put_unaligned_le32(tkid, hs[2].tTKID);
 	hs[2].bReserved = 0;
 	memcpy(hs[2].CDID, &wusb_dev->cdid, sizeof(hs[2].CDID));
 	memcpy(hs[2].nonce, hs[0].nonce, sizeof(hs[2].nonce));
-- 
2.7.4

Re: [PATCH v2] usb: wusbcore: Use put_unaligned_le32

[Greg KH]

On Fri, Oct 06, 2017 at 08:38:07PM +0530, Himanshu Jha wrote:
> Use put_unaligned_le32 rather than using byte ordering function and
> memcpy which makes code clear.
> Also, add the header file where it is declared.
> 
> Done using Coccinelle and semantic patch used is :
> 
> @ rule1 @
> identifier tmp; expression ptr,x; type T;
> @@
> 
> - tmp = cpu_to_le32(x);
> 
>   <+... when != tmp
> - memcpy(ptr, (T)&tmp, ...);
> + put_unaligned_le32(x,ptr);
>   ...+>
> 
> @ depends on rule1 @
> type j; identifier tmp;
> @@
> 
> - j tmp;
>   ...when != tmp
> 
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
> ---
> v2:
> * added correct header file.
> 
>  drivers/usb/wusbcore/security.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/wusbcore/security.c b/drivers/usb/wusbcore/security.c
> index 170f2c3..256b56f 100644
> --- a/drivers/usb/wusbcore/security.c
> +++ b/drivers/usb/wusbcore/security.c
> @@ -22,6 +22,7 @@
>   *
>   * FIXME: docs
>   */
> +#include <asm/unaligned.h>

Ick, really?  asm/ ?  As the first include?  That feels wrong, at the
very least, it needs to be at the end of the include list please.

thanks,

greg k-h

Re: [PATCH v2] usb: wusbcore: Use put_unaligned_le32

[Himanshu Jha]

On Tue, Oct 17, 2017 at 11:51:14AM +0200, Greg KH wrote:
> On Fri, Oct 06, 2017 at 08:38:07PM +0530, Himanshu Jha wrote:
> > Use put_unaligned_le32 rather than using byte ordering function and
> > memcpy which makes code clear.
> > Also, add the header file where it is declared.
> > 
> > Done using Coccinelle and semantic patch used is :
> > 
> > @ rule1 @
> > identifier tmp; expression ptr,x; type T;
> > @@
> > 
> > - tmp = cpu_to_le32(x);
> > 
> >   <+... when != tmp
> > - memcpy(ptr, (T)&tmp, ...);
> > + put_unaligned_le32(x,ptr);
> >   ...+>
> > 
> > @ depends on rule1 @
> > type j; identifier tmp;
> > @@
> > 
> > - j tmp;
> >   ...when != tmp
> > 
> > Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
> > ---
> > v2:
> > * added correct header file.
> > 
> >  drivers/usb/wusbcore/security.c | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/usb/wusbcore/security.c b/drivers/usb/wusbcore/security.c
> > index 170f2c3..256b56f 100644
> > --- a/drivers/usb/wusbcore/security.c
> > +++ b/drivers/usb/wusbcore/security.c
> > @@ -22,6 +22,7 @@
> >   *
> >   * FIXME: docs
> >   */
> > +#include <asm/unaligned.h>
> 
> Ick, really?  asm/ ?  As the first include?  That feels wrong, at the
> very least, it needs to be at the end of the include list please.

I just sent a v3 patch with the necessary change!
Also, would you mind explaining as to why asm/ should not be the first
include ?

Thanks

> thanks,
> 
> greg k-h

Re: [PATCH v2] usb: wusbcore: Use put_unaligned_le32

[Greg KH]

On Tue, Oct 17, 2017 at 05:20:29PM +0530, Himanshu Jha wrote:
> On Tue, Oct 17, 2017 at 11:51:14AM +0200, Greg KH wrote:
> > On Fri, Oct 06, 2017 at 08:38:07PM +0530, Himanshu Jha wrote:
> > > Use put_unaligned_le32 rather than using byte ordering function and
> > > memcpy which makes code clear.
> > > Also, add the header file where it is declared.
> > > 
> > > Done using Coccinelle and semantic patch used is :
> > > 
> > > @ rule1 @
> > > identifier tmp; expression ptr,x; type T;
> > > @@
> > > 
> > > - tmp = cpu_to_le32(x);
> > > 
> > >   <+... when != tmp
> > > - memcpy(ptr, (T)&tmp, ...);
> > > + put_unaligned_le32(x,ptr);
> > >   ...+>
> > > 
> > > @ depends on rule1 @
> > > type j; identifier tmp;
> > > @@
> > > 
> > > - j tmp;
> > >   ...when != tmp
> > > 
> > > Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
> > > ---
> > > v2:
> > > * added correct header file.
> > > 
> > >  drivers/usb/wusbcore/security.c | 9 ++++-----
> > >  1 file changed, 4 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/usb/wusbcore/security.c b/drivers/usb/wusbcore/security.c
> > > index 170f2c3..256b56f 100644
> > > --- a/drivers/usb/wusbcore/security.c
> > > +++ b/drivers/usb/wusbcore/security.c
> > > @@ -22,6 +22,7 @@
> > >   *
> > >   * FIXME: docs
> > >   */
> > > +#include <asm/unaligned.h>
> > 
> > Ick, really?  asm/ ?  As the first include?  That feels wrong, at the
> > very least, it needs to be at the end of the include list please.
> 
> I just sent a v3 patch with the necessary change!
> Also, would you mind explaining as to why asm/ should not be the first
> include ?

Do you see any other files that have it that way?  It's traditionally
last.

thanks,

greg k-h