login
Header Space

 
 

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

Score:
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: Andi Kleen <andi@...>
Cc: Ingo Molnar <mingo@...>, David Miller <davem@...>, <jirislaby@...>, <viro@...>, <joe@...>, <tglx@...>, <linux-kernel@...>
Date: Tuesday, March 25, 2008 - 1:19 pm

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).

--
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cle..., Peter Zijlstra, (Tue Mar 25, 1:19 pm)
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cle..., Christoph Hellwig, (Wed Mar 26, 7:09 am)
[patch] bkl2mtd: cleanup, Ingo Molnar, (Wed Mar 26, 6:14 am)
Re: [patch] bkl2mtd: cleanup, Al Viro, (Wed Mar 26, 6:48 am)
Re: [patch] bkl2mtd: cleanup, Ingo Molnar, (Wed Mar 26, 7:10 am)
Re: [patch] bkl2mtd: cleanup, Joe Perches, (Wed Mar 26, 12:30 pm)
Re: [patch] bkl2mtd: cleanup, Jiri Slaby, (Wed Mar 26, 7:14 am)
Re: [patch] bkl2mtd: cleanup, Ingo Molnar, (Wed Mar 26, 7:02 am)
Re: [patch] bkl2mtd: cleanup, Ingo Molnar, (Wed Mar 26, 7:00 am)
Re: [patch] bkl2mtd: cleanup, Jörn, (Wed Mar 26, 6:57 am)
speck-geostationary