On Tue, 2008-03-25 at 13:26 +0100, Andi Kleen wrote:I think someone with a consistent code style generates better code than someone who can't even make up his mind on how to write it, let alone what to write. With Linux we have many people, many of us with a preferred style, most of which will differ from the suggested style. Much like natural languages, many of us don't speak English as their native tongue, mine is Dutch, yours is German. Still we communicate with each other using English, because that is the de-facto standard on LKML [ and of course because my German truly sucks ;-) ]. Similarly for coding style, if we all stick to one style its easier to read (and hopefully fix) each others code. All checkpatch.pl attempts is help us do that. Sure it complains about sometimes trivial things - but if truly trivial making the change isn't hard - or might even not be done in favour of 'better' taste. Its a guide, not hard rules, it suggests we inspect a certain piece of code again, to see if we really intended to write it so. Does it get things wrong; Yes I do think so. The change in patch 120/148 is utter nonsense IMHO. Checkpatch will also not complain about larger things that make a bigger difference; for instance it will happily let you write a 200 line function, even though the code would have been so much better in 10 smaller functions. Does it help; Yes, I do think it does. If only people would apply common sense along with it... The trailing and leading whitespace thingies are trivial to fix (and quilt refresh --strip-trailing-whitespace does half of it already, so anybody using quilt doesn't have any excuse in ever seeing that error anyway). There is something to say to send Linus a script that fixes up the whole tree and be done with it. These are petty things indeed. Which is why it is a guide, a human using the tool can use his good taste and discretion by choosing which suggestions to take and which to politely ignore. Sure, and I think that when there is actual work pending its 'actively' maintained and will eventually gravitate towards cleaniness anyway. The good thing is that most of these checkpatch patches could be validated by comparing object code of the affected translation units - they're that trivial. When generating 100% identical code, there is no issue. Building a single allyesconfig for x86_32 and x86_64 before and after and getting identical binaries is pretty strong. Although I think this patch series doesn't manage to accomplish it, if only because it moves __LINE__ statements around. Sure, cleanups like this should never bother real work - good chance that the real work already cleans up most issues anyway. It might be an incentive to touch (and get to know and appreciate and eventually clean up or start maintaining) otherwise dead code. I don't think we should stop such incentives (even if they are very small). --
| david | Re: Dual-Licensing Linux Kernel with GPL V2 and GPL V3 |
| David Woodhouse | [GIT *] Allow request_firmware() to be satisfied from in-kernel, use it in more dr... |
| Philipp Marek | Re: sys_chroot+sys_fchdir Fix |
| Greg Kroah-Hartman | [PATCH 008/196] Chinese: add translation of volatile-considered-harmful.txt |
git: | |
| Krishna Kumar | [PATCH 9/10 REV5] [IPoIB] Implement batching |
| Gerrit Renker | [PATCH 15/37] dccp: Set per-connection CCIDs via socket options |
| David Miller | Re: [PATCH] pkt_sched: Destroy gen estimators under rtnl_lock(). |
| David Miller | [GIT]: Networking |
