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

!MAILaRCHIVE_VOTE_RePLACE
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: Andy Whitcroft <apw@...>
Cc: Andrew Morton <akpm@...>, Randy Dunlap <rdunlap@...>, Joel Schopp <jschopp@...>, <linux-kernel@...>
Date: Friday, September 28, 2007 - 6:49 am

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


you _still_ fail to understand (let alone address), the fundamental 
issues i raised many times. (see below for details)


the most usable checkpatch.pl version was v6/v7. It went downhill after 
that. Look at the script size versus the number of complaints about 
kernel/sched.c:

  size                          # warnings
  ----------------------------------------
  25383  checkpatch.pl.v6       5
  26038  checkpatch.pl.v7       6
  29603  checkpatch.pl.v8       65
  31160  checkpatch.pl.v9       24
  34950  checkpatch.pl.v10      28

i.e. size did not increase all that significantly, still the number of 
warnings has increased significantly - with little benefit.

and here's the breakdown for v10: out of 28 warnings, only 6 are 
legitimate! So the signal to noise ratio has worsened significantly 
since v6. One warning per 300 lines of sched.c is _not manageable_. It 
translates to _over 25 thousand false positives_ for the whole kernel.

every false warning has additive costs: i will have to ignore it from 
that point on, forever - and i frequently have to re-check the same 
thing again, and again - just to discover that the warning is still 
bogus.

the mails from me in your mbox during the past few months should be 
proof that i'm fully constructive regarding this matter and that i'm not 
striving for 100% level of perfection. I'm simply stating the obvious: 
your tool is less and less useful with every new version and more 
troubling than that is your apparent inability to realize what this 
whole issue is about. (see below for details)


you still dont get the point of this. This isnt about writing a tool 
that 'can' find something to complain about. This whole kernel source 
code quality task is about:

     _making kernel source code quality better_

not more, not less. If your tool outputs way too many false positives 
and way too many unimportant borderline cases, people will ignore the 
tool, and consequently YOU MAKE THE KERNEL'S SOURCE CODE WORSE than it 
could be. How hard is that to understand?? Having zero (or near zero) 
output from a checker mechanism is FUNDAMENTAL.

( and note that i was a goddamn early adopter of your tool, i'm a tester 
  and i gave you feedback, and i'm one of the few kernel hackers who 
  actually use your script as a mandatory component of their 
  patch-toolchain here and today. But your unchanged fundamentalistic
  attitude has turned me from a happy supporter of checkpatch.pl into an
  almost-opponent. If anything, that should be a warning shot for you. )


then remove most of those 22 false positives as a starter. I dont care 
if it's "hard/impossible" or not. If it cannot be coded like that, DONT 
output that particular type of check by default. Let people OPT IN if 
there's a reasonable chance for false positives.

( the same is true for gcc warnings: false positives are a huge PITA and
  they _CAUSE_ bugs because people have learned to ignore gcc warnings 
  and will accidentally ignore real bugs too. )

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

Messages in current thread:
[PATCH] update checkpatch.pl to version 0.10, Andy Whitcroft, (Wed Sep 12, 11:00 am)
Re: [PATCH] update checkpatch.pl to version 0.10, Ingo Molnar, (Fri Sep 28, 4:40 am)
Re: [PATCH] update checkpatch.pl to version 0.10, Andy Whitcroft, (Fri Sep 28, 5:52 am)
Re: [PATCH] update checkpatch.pl to version 0.10, Andrew Morton, (Fri Sep 28, 5:01 am)
Re: [PATCH] update checkpatch.pl to version 0.10, Ingo Molnar, (Fri Sep 28, 5:44 am)
Re: [PATCH] update checkpatch.pl to version 0.10, Andy Whitcroft, (Fri Sep 28, 5:22 am)
Re: [PATCH] update checkpatch.pl to version 0.10, Ingo Molnar, (Fri Sep 28, 5:39 am)
Re: [PATCH] update checkpatch.pl to version 0.10, Andy Whitcroft, (Fri Sep 28, 6:00 am)
Re: [PATCH] update checkpatch.pl to version 0.10, Sam Ravnborg, (Fri Sep 28, 12:51 pm)
Re: [PATCH] update checkpatch.pl to version 0.10, Ingo Molnar, (Fri Sep 28, 6:49 am)
Re: [PATCH] update checkpatch.pl to version 0.10, Ingo Molnar, (Fri Oct 5, 1:56 am)
Re: [PATCH] update checkpatch.pl to version 0.10, Andy Whitcroft, (Fri Sep 28, 9:21 am)
Re: [PATCH] update checkpatch.pl to version 0.10, Andrew Morton, (Fri Sep 28, 1:46 pm)
Re: [PATCH] update checkpatch.pl to version 0.10, Andy Whitcroft, (Sat Sep 29, 5:22 am)
Re: [PATCH] update checkpatch.pl to version 0.10, Randy Dunlap, (Fri Sep 28, 1:26 pm)
Re: [PATCH] update checkpatch.pl to version 0.10, Joel Schopp, (Fri Sep 28, 11:50 am)
Re: [PATCH] update checkpatch.pl to version 0.10, Pekka Enberg, (Fri Sep 28, 9:37 am)
Re: [PATCH] update checkpatch.pl to version 0.10, Andy Whitcroft, (Fri Sep 28, 10:02 am)
Re: [PATCH] update checkpatch.pl to version 0.10, Christian Borntraeger, (Fri Sep 28, 6:46 am)
Re: [PATCH] update checkpatch.pl to version 0.10, WANG Cong, (Fri Sep 28, 7:03 am)
Re: [PATCH] update checkpatch.pl to version 0.10, Jan Engelhardt, (Fri Sep 28, 10:19 am)
Re: [PATCH] update checkpatch.pl to version 0.10, Randy Dunlap, (Fri Sep 28, 12:57 pm)