kernelnewbies.kernelnewbies.org archive mirror
 help / color / mirror / Atom feed
* A quick guide to why stand-alone checkpatch patches suck...
@ 2014-09-17  0:25 Valdis Kletnieks
  2014-09-17  0:37 ` Greg KH
  2014-09-17  1:35 ` Greg Donald
  0 siblings, 2 replies; 35+ messages in thread
From: Valdis Kletnieks @ 2014-09-17  0:25 UTC (permalink / raw)
  To: kernelnewbies

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 

^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2014-09-17 20:45 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-17  0:25 A quick guide to why stand-alone checkpatch patches suck Valdis Kletnieks
2014-09-17  0:37 ` Greg KH
2014-09-17  1:35 ` Greg Donald
2014-09-17  3:42   ` Valdis.Kletnieks at vt.edu
2014-09-17  4:56     ` Greg KH
2014-09-17  5:43       ` Sudip Mukherjee
2014-09-17 10:39       ` Robert P. J. Day
2014-09-17 11:20       ` Robert P. J. Day
2014-09-17 11:38         ` nick
2014-09-17 11:51           ` Sudip Mukherjee
2014-09-17 11:53             ` nick
2014-09-17 12:05               ` Greg Freemyer
2014-09-17 12:09                 ` nick
2014-09-17 12:17                   ` Kai Bojens
2014-09-17 12:23                     ` nick
2014-09-17 12:25                   ` Greg Freemyer
2014-09-17 12:29                     ` Nick Krause
2014-09-17 14:39                   ` Valdis.Kletnieks at vt.edu
2014-09-17 11:53           ` Robert P. J. Day
2014-09-17 11:55             ` nick
2014-09-17 12:17               ` Chris Lee
2014-09-17 12:19                 ` nick
2014-09-17 11:56         ` Greg Freemyer
2014-09-17 12:00           ` Robert P. J. Day
2014-09-17 12:05             ` nick
2014-09-17 12:02           ` nick
2014-09-17 14:39             ` Valdis.Kletnieks at vt.edu
2014-09-17 17:04               ` Nick Krause
2014-09-17 17:13               ` Bruno Guedes Souto
2014-09-17 17:47                 ` Nick Krause
2014-09-17 18:01                   ` Philipp Muhoray
2014-09-17 18:14                     ` Nick Krause
2014-09-17 18:15                   ` Robert P. J. Day
2014-09-17 18:44                     ` Nick Krause
2014-09-17 20:45                 ` John de la Garza

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).