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

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

 



On Fri, Mar 14, 2014 at 12:39:11AM +0900, Jérôme Pinot wrote:
> 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.
> 

It is aligned ok, but don't rely on the maintainer to fix your bugs.
Don't send patches which you are not sure about.

Joe, this seems like a very bad warning message from checkpatch.pl
because people will constantly send us patches over and over which
introduce bugs and they rely on the maintainer to catch it every time.
Can we get rid of the warning or move it under --strict or something?

Do we have a mailing list yet for checkpatch issues?

regards,
dan carpenter

_______________________________________________
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