Re: [PATCH] update checkpatch.pl to version 0.08

[Ingo Molnar <mingo@elte.hu>]

* Andy Whitcroft <apw@shadowen.org> wrote:

> > within the last 3 weeks, this script went from *really usable* to *a 
> > big noise maker*.

seconded ...

v0.06 was "almost there". I fixed kernel/sched.c to be completely clean, 
except for 3 false positives. That was a real improvement, and i started 
to like checkpatch.pl.

v0.08 is a clear step backwards: it emits 61 warnings now, 90% of which 
are totally bogus. The only 'fix' for many of those warnings is to make 
the code _worse_. That is unacceptable.

> > Warning on multiple declarations on a line is nice, but IMO really 
> > too verbose (why is "int i, j;" bad? Did C somehow change syntax 
> > today?).
> 
> No the normal response is two fold:
> 
> 1) "what the heck are i and j those are meaningless names"
> 2) "please can we have some comments for those variables"

you really should not even be arguing about this. LOOK AT the many false 
positives in sched.c. This is perfectly readable code:

 void __init sched_init(void)
 {
         u64 now = sched_clock();
         int highest_cpu = 0;
         int i, j;

         for_each_possible_cpu(i) {
                 struct rt_prio_array *array;
                 struct rq *rq;

                 rq = cpu_rq(i);

this warning for "i, j" is clearly bogus. So are many of the other 
warnings. checkpatch.pl went from a useful tool that improved the 
quality of the kernel to a rigid, unflexible policeman. It needs to be 
fixed or needs to be gotten rid of.

> which normally leads to the suggestion that it be the following form:
> 
> 	int source;		/* source clock hand */
> 	int destination;	/* destination clock hand */

what the hell are you thinking? Not every trivial line of code needs to 
be commented. Comments are needed for the _nontrivial_ lines of code, 
and there's no way a tool can decide that. The longer "destination" 
variable suggested by you can _easily_ make a previously readable piece 
of code unreadable.

	Ingo