Messages in this thread | | | Date | Tue, 24 Jul 2007 12:33:49 +0100 | From | Andy Whitcroft <> | Subject | Re: [PATCH] update checkpatch.pl to version 0.08 |
| |
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |