A quick guide to why stand-alone checkpatch patches suck...

Valdis Kletnieks Valdis.Kletnieks at vt.edu
Tue Sep 16 20:25:51 EDT 2014


In general, stand-alone patches to "fix" checkpatch whining are a Bad Idea(TM).

Here's why...

First off, the type of programmer who is tempted to do checkpatch cleanup
as "My First Kernel Patch" are, by and large, novices.

The code in the kernel falls into one of several states of use and stability.
Some code is old and heavily used, some is old and not used, and some of
it is under active development.  Let's look at each case.

There's parts of the kernel that have been around for *years* and are beat on
constantly - the VFS, the driver core code, large parts of the network stack
for example.  Stand-alone checkpatch fixes here aren't a good idea, because
they're potentially destabilizing if somebody gets the fix wrong. And yes, this
has happened, and checkpatch "fixes" have actually introduced bugs - it's part
of why there's a "one thing, one patch" rule, to make it easier to audit
a patch and ensure it doesn't introduce regressions.  Oh, and most of this sort of
code is already *really* clean, because professionals have cleaned it up.

There's parts of the kernel that are digital archaeology waiting to happen
(I'm looking at you, floppy.c :).  Again, fixing style is probably a Bad Idea,
because (a) you might introduce a bug and (b) nobody is even looking at this
code anymore, so nobody *cares* about the style. And if it's an abandoned
part of the code, there may be nobody who cares/understands it well enough to
notice if a subtle bug is introduced.

So that leaves us code that's under active development.  And here, checkpatch
fixes are actually a *detriment*, and they tick the subsystem maintainer off.
That's because they have a high probability of causing merge conflicts with
somebody else's patches that are doing *actual code improvement*.

The end result?  Unless you *are* that "somebody else" who's doing other
work on the code, you shouldn't submit checkpatch cleanups.  So we shouldn't
see these from anybody except as prep work for actual coding.  (But by
all means, if you're about to apply a can-opener to a .c file to do some
Real Work, feel free to spin a preparatory set of cleanup patches so you're
performing code surgery on a properly scrubbed, prepped, and anesthetized
patient :)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 848 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140916/567f4618/attachment.bin 


More information about the Kernelnewbies mailing list