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

Andy Whitcroft
2007-07-24T11:34:24Z

Kok, Auke wrote:
> Andy Whitcroft wrote:
>> This version brings a number of new checks, and a number of bug
>> fixes. Of note:
>>
>> - warnings for multiple assignments per line
>> - warnings for multiple declarations per line
>> - checks for single statement blocks with braces
>>
>> This patch includes an update for feature-removal-schedule.txt to
>> better target checks.
>>
>> Andy Whitcroft (12):
>> Version: 0.08
>> only apply printk checks where there is a string literal
>> allow suppression of errors for when no patch is found
>> warn about multiple assignments
>> warn on declaration of multiple variables
>> check for kfree() with needless null check
>> check for single statement braced blocks
>> check for aggregate initialisation on the next line
>> handle the => operator
>> check for spaces between function name and open parenthesis
>> move to explicit Check: entries in feature-removal-schedule.txt
>> handle pointer attributes
>
> within the last 3 weeks, this script went from *really usable* to *a big
> noise maker*.

She is developing for sure. I for one don't want it to be worthless. I
also want it to catch the things that Andrew is hottest on. A difficult
path. Always remember that this is a guide to style not definitive.

> I am testing this with new drivers (igb, e1000e, ixgbe) and the amount
> of warnings it throws was in the order of 10 last week, but now I'm at
> hundreds again, and my code has not changed.
>
> The superfluous braces error should definately be fixed.

Yes, that was a misunderstanding my end, and I have loosened that check.
for the next version. Not sure if its much use anymore but it should no
longer winge all over your patch.

> 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"

which normally leads to the suggestion that it be the following form:

int source; /* source clock hand */
int destination; /* destination clock hand */

and all is well. That was the background for the checks. However,
there is much upsetedness over it and push for i, j, k, l being a handy
form.

I am inclined to drop this check completely. Andrew this was one of
your requests?

> Some of the new features are plain broken as I posted before. A lot of
> it now is purely false positives only.
>
> Bottom line: I really wish that I could have the script run in the old
> behaviour before. While this level of verbosity is great for single-line
> patches, it really completely wastes my time when I'm trying to get a
> grasp for a 200k hunk piece of code.

I can only shudder at the thought of a 200k patch, but ok.

> The best way to implement this is that I can somehow select / omit some
> of these checks when running the script. With more and more checks added
> to the script it will be very quick for new driver writers to stop using
> it because of the sheer volume that the script currently outputs.

Yeah I have been feeling that we might want to say "--no-check" etc so
you can only get the more serious errors etc. Will think on that.

-apw