Re: [PATCH] add a trivial patch style checker

Jan Engelhardt
2007-05-31T19:30:18Z

>+sub top_of_kernel_tree {
>+ if ((-f "COPYING") && (-f "CREDITS") && (-f "Kbuild") &&
>+ (-f "MAINTAINERS") && (-f "Makefile") && (-f "README") &&
>+ (-d "Documentation") && (-d "arch") && (-d "include") &&
>+ (-d "drivers") && (-d "fs") && (-d "init") && (-d "ipc") &&
>+ (-d "kernel") && (-d "lib") && (-d "scripts")) {
>+ return 1;
>+ }
>+ return 0;
>+}

Looks like some redundant parentheses.

>+#gotos aren't indented
>+ if($line=~/^\s*[A-Za-z\d_]+:/ and !($line=~/^\s*default:/)){
>+ print "Gotos should not be indented\n";
>+ print "$herecurr";
>+ $clean = 0;
>+ }

I think this was discussed already - some indent (1 space?) should be allowed.

>+#studly caps, commented out until figure out how to distinguish between use
>+#of existing and adding new

Yeah that's a big problem, given that linux's mm/ directory has
quite a lot of camel case function names.

>+# if(($line=~/[\w_][a-z\d]+[A-Z]/) and !($line=~/print/)) {
>+# print ("No studly caps, use _\n");
>+# print "$herecurr";
>+# $clean = 0;
>+# }
>+
>+#no spaces allowed after \ in define
>+ if($line=~/\#define.*\\\s$/){

Usually, #s do _not_ need to be quoted (in contrast to @).
I am at stake to be wrong, anyone know more? :)

>+#if/while/etc brace do not go on next line, unless #defining a do while loop, or if that brace on the next line is for something else
>+ if ($prevline=~/(if|while|for|switch)\s*\(/) {

/(?:if|while|.../

I don't see $1 being captured/used, so..

>+ my @opened = $prevline=~/\(/g;
>+ my @closed = $prevline=~/\)/g;
>+ my $nr_line = $linenr;
>+ my $remaining = $realcnt;
>+ my $next_line = $line;
>+ my $extra_lines = 0;
>+ my $display_segment = $prevline;
>+
>+ while ($remaining > 0 && scalar @opened > scalar @closed) {
>+ $prevline .= $next_line;
>+ $display_segment .= "\n" . $next_line;
>+ $next_line = $lines[$nr_line];
>+ $nr_line++;
>+ $remaining--;
>+
>+ @opened = $prevline=~/\(/g;
>+ @closed = $prevline=~/\)/g;
>+ }
>+
>+ if(($prevline=~/(if|while|for|switch)\s*\(.*\)\s*$/) and ($next_line=~/{/) and
>+ !($next_line=~/(if|while|for)/) and !($next_line=~/\#define.*do.*while/)) {

Same.

>+# don't include <linux/video_decoder.h>

Who does that?

Jan
--