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

[Jiri Slaby <jirislaby@gmail.com>]

On 03/23/2008 06:36 PM, Andi Kleen wrote:
> Jiri Slaby <jirislaby@gmail.com> writes:
>> It just spits out warnings/errors like compiler or some static
>> analyzer, maybe I'm terribly missing something, what exactly do you
>> mind on the output?
> 
> A good example are the trailing white space warnings in there. They
> are just useless and a waste of time. That is something that really
> should just be done automatically by maintainer scripts somewhere on
> merge (and a lot of maintainers do it automatically), not waste the
> valuable time of a human.

Agreed.

> There are also a couple of other pointless warnings. e.g. I dislike
> some of the style warnings -- they are far too broad. Or there 
> can be good reasons to violate them occasionally. e.g. I would
> rather put not a space around an operator occasionally instead
> of splitting an expression to avoid going over 80 characers. 

Me too, there will ever be certain circumstances where the rules just doesn't
apply and you can't teach the scripts in that way (turning off doubtful warnings
sounds reasonable anyway).

> Or the EXPORT_SYMBOL warning. WTF is that good for? Or 
> the warning about if () ... else { ... }.

Understand you points, but most of it is strict implementation of CodingStyle. 
People, who don't prefer coding style as described in that document, won't like 
checkpatch either.

> Also checkpatch.pl --file seems to be a good candidate for one
> of the worst ideas ever merged (to be fair it was difficult
> to see it would become that bad in advance -- but it got) 

Definitely agree.