Re: [patch] checkpatch: remove the ether_addr_copy warning

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

 



On Fri, 2014-10-03 at 17:47 +0300, Dan Carpenter wrote:
> On Fri, Oct 03, 2014 at 07:22:27AM -0700, Joe Perches wrote:
> > On Fri, 2014-10-03 at 12:35 +0300, Dan Carpenter wrote:
> > > Most people sending checkpatch.pl fixes don't know how to verify the
> > > alignment.  This checkpatch warning just encourages newbies to try
> > > introduce bugs.  Patch submitters tell us that they just sed the code
> > > and it's the job for the maintainer to check that it's correct.

And that's where it's the maintainer's job to educate,
inform, reject 
 
> > I haven't seen many instances of bad patch submittals
> > on netdev.  Is this mostly an issue for staging?
> 
> I don't follow netdev so I can't say.
> 
> Most of the time data is aligned at a 4 byte mark so probably you are
> just getting lucky.

All typical Ethernet frames have 1 of the 2 addresses on an
even byte boundary but not 4 byte aligned.

Most all of the is_<foo>_ether_addr tests assume __aligned(2)

> I really doubt that netdev checkpatch newbies know
> about alignment...

I think that's a learning opportunity...

> > Maybe a downgrade to CHK requiring --strict is OK.
> 
> I would actually like to turn --strict by default in staging.

I recall a suggested patch for that.

> Checkpatch is a good concept, but it should only do safe things instead
> of telling newbies to send buggy patches.

checkpatch isn't just for the inexperienced.
It's an oversight tester and a style tool.

Degrading it doesn't make it better.

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux