linux-kernel - Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080325140314.GA10569@elte.hu>
Date:	Tue, 25 Mar 2008 15:03:14 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	Jörn Engel <joern@...fs.org>
Cc:	David Miller <davem@...emloft.net>, jirislaby@...il.com,
	viro@...IV.linux.org.uk, joe@...ches.com, tglx@...utronix.de,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups
	- formatting only

* Jörn Engel <joern@...fs.org> wrote:

> > So to turn around the argument: could you give me any reason why 
> > differing coding style between subsystems, _often in blatant 
> > violation of Documentation/CodingStyle_, is somehow "good" for Linux 
> > in the long run? I listed numerous first-hand advantages that style 
> > consistency brings and i listed numerous disadvantages created by 
> > inconsistency. So i'm waiting for the list of counter-arguments - 
> > there _must_ be some objective ones, besides the obvious "kernel 
> > old-timers are lazy to change their ways" argument =B-)
> 
> When you reject useful patches based on "this is not our preferred 
> style", you piss people off. [...]

we dont actually do that for newbies and newbies are in fact happy to 
write cleaner code - so the rest of your argument which depends on this 
premise fails. (Most of the time i fix it up silently myself or if a 
style error comes in a pattern, i ask the person to send future patches 
with that small detail fixed.)

my experience with checkpatch.pl is the exact opposite of what you fear: 
it _widened_ the contributor base: a good number of newbies felt 
encouraged that an objective piece of tool reports an "error" in a file 
that was written by otherwise "much more knowledgable" kernel hackers. 
checkpatch.pl is basically the "yes, really, you are right, this piece 
of code in the Linux kernel is indeed crap" review tool that reinforces 
newbies. It lowers the bar of entry to kernel hacking, and it does so 
for exactly those pieces of code that we want newbies to be active on: 
barely maintained source code.

Whoever is afraid of an "army of checkpatch wielding newbies" who'll 
never rise above their newbie status fails to consider two important 
factors: 1) checkpatch errors are a finite resource to feed on and they 
dont re-grow in a well-maintained subsystem 2) they need to look back a 
few years when they themselves were newbies and were in need of some 
easy kernel projects just to familiarize themselves with the kernel and 
its contribution environment. (My first-ever contribution to the Linux 
kernel was a trivial patch.)

( i only remember one patch ever being rejected due to checkpatch 
  failures, it came from a kernel old-timer who sent absolutely horrible 
  patches and who _should have known better_. Kernel old-timers are 
  "multipliers", they write more code and influence more people's code, 
  so it's expected of them to write absolutely squeaky-clean code. )

so at least for the scheduler and for arch/x86 there's absolutely zero 
friction between checkpatch.pl use and newbies - and if you look at the 
nicely evolving arch/x86 contributor statistics you'll have to come to 
the same conclusion i believe.

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/