linux-kernel - Re: [PATCH] add a trivial patch style checker

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.61.0705312120320.11140@yvahk01.tjqt.qr>
Date:	Thu, 31 May 2007 21:26:50 +0200 (MEST)
From:	Jan Engelhardt <jengelh@...ux01.gwdg.de>
To:	Andy Whitcroft <apw@...dowen.org>
cc:	Andrew Morton <akpm@...l.org>, Randy Dunlap <rdunlap@...otime.net>,
	Joel Schopp <jschopp@...tin.ibm.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] add a trivial patch style checker


>+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
-- 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/