Re: [PATCH 3/3] staging/ozwpan: coding style ether_addr_copy

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/13/14 02:28, Greg Kroah-Hartman wrote:
> On Thu, Mar 13, 2014 at 10:21:44AM +0900, Jérôme Pinot wrote:
[...]
> > diff --git a/drivers/staging/ozwpan/ozcdev.c b/drivers/staging/ozwpan/ozcdev.c
> > index 5de5981..10c0a96 100644
> > --- a/drivers/staging/ozwpan/ozcdev.c
> > +++ b/drivers/staging/ozwpan/ozcdev.c
> > @@ -217,7 +217,7 @@ static int oz_set_active_pd(const u8 *addr)
> >  	pd = oz_pd_find(addr);
> >  	if (pd) {
> >  		spin_lock_bh(&g_cdev.lock);
> > -		memcpy(g_cdev.active_addr, addr, ETH_ALEN);
> > +		ether_addr_copy(g_cdev.active_addr, addr);
> 
> Are you sure this will work?

No. But the ozwpan driver uses already ether_addr_equal which is not
alignment-safe. As
https://www.kernel.org/doc/Documentation/unaligned-memory-access.txt 
said:

"This alignment-unsafe function is still useful as it is a decent
optimization for the cases when you can ensure alignment, which is
true almost all of the time in ethernet networking context."

I expected the maintainer to confirm/infirm this. I'm just seeing that's
actually Chris Kelly who did write this part of code, so I'm CC'ing him
too.

> You have to check the alignment of the variable.
> 
> Also, this breaks the build, you need to include some kind of .h file to
> get this to work, please ALWAYS test your patches to make sure they
> don't break things.

My bad, sorry. I'll send a better patch. Thanks for your time.

> greg k-h

-- 
Jérôme Pinot
http://ngc891.blogdns.net/
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel





[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux