linux-kernel - [PATCH RFC V2] checkpatch: flag split arithmetic operations with CHECK

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 for Android: free password hash cracker in your pocket

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

Message-Id: <1431257186-18013-1-git-send-email-hofrat@osadl.org>
Date:	Sun, 10 May 2015 13:26:26 +0200
From:	Nicholas Mc Guire <hofrat@...dl.org>
To:	Andy Whitcroft <apw@...onical.com>
Cc:	Joe Perches <joe@...ches.com>, linux-kernel@...r.kernel.org,
	Nicholas Mc Guire <hofrat@...dl.org>
Subject: [PATCH RFC V2] checkpatch: flag split arithmetic operations with CHECK

Simple arithmetic operations should be on one line, if they can be fit,
rather than splitting at the operator. As this is not in the CodingStyle it
is limited to --strict use of checkpatch.pl and emits a CHECK only.

Signed-off-by: Nicholas Mc Guire <hofrat@...dl.org>
---

V2: re-use the debug type detection as proposed by Andy Whitcroft 
    to distinguish the unary * from the binary * - thanks!

This now flags arithmetic operations split across lines indicated by
either a leading or final binary operator.

This extension should flag such lines as CHECK only, as this is not
mandated by the CodingStyle and in some cases they seem justified 
(e.g. kernel/sched/fair.c:760 and a few other examples in that file)

TODO: Move to "# Check operator spacing" section as Joe Perches
      requested - simply got completely lost in that section...
      Also not clear if it really is the right place to put it - 
      spacing issues and split operators are two quite different 
      problems.
Question: There are many (518) "lines over 80 char" warnings but that
          seems to be accepted in checkpatch.pl - should those be 
          wrapped ?

The 4 rules are left separated for review - most of this could be 
joined but before doing that a review of the individual regex would 
be good.

Test-case:
scripts/checkpatch.pl --strict -f kernel/*.c | grep -A 3 -e "Arithmetic" 
(and a few other directories) - there seem to be no false positive there

Patch is against 4.1-rc2 (localversion-next is -next-20150508)

 scripts/checkpatch.pl |   52 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 89b1df4..a2e3d26 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2577,6 +2577,58 @@ sub process {
 			    "Logical continuations should be on the previous line\n" . $hereprev);
 		}
 
+# check for + or - at the end of a line but ignore --/++
+		if (!($rawline =~ /^\+.*(\+\+|--).*$/) &&
+		    $rawline =~ /^\+.*(\+|-)$/) {
+			my $opline = $line; $opline =~ s/^./ /;
+			my ($curr_values, $types) = annotate_values($opline . "\n", $prev_values);
+			# its type and the end 'B' -> binary * -> fuss
+			if($types =~ m/^.*B$/) {
+			    CHK("ARITHMETIC_CONTINUATIONS",
+				"Arithmetic expressions should be on one line\n" . $hereprev);
+			}
+		}
+
+
+# check for + or - at beginning of a line but exclude ++var/--var
+		if (!($rawline =~ /^\+\s*(\+\+|--).*$/) &&
+		   $rawline =~ /^\+\s*(\+|-).*$/) {
+			# need to check the type here
+			my $opline = $line; $opline =~ s/^./ /;
+			my ($curr_values, $types) = annotate_values($opline . "\n", $prev_values);
+			# first is type 'B' -> binary * -> fuss
+			if($types =~ m/(^_*)(B)/) {
+			    CHK("ARITHMETIC_CONTINUATIONS",
+				"Arithmetic expressions should be on one line\n" . $hereprev);
+			}
+		}
+
+# check for /,% and * at end of a line
+		if (!($rawline =~ /^\+.*\*\/$/) &&
+		     $rawline =~ /^\+.*(\*|\/|%)$/) {
+			# need to check the type here
+			my $opline = $line; $opline =~ s/^./ /;
+			my ($curr_values, $types) = annotate_values($opline . "\n", $prev_values);
+			# its type 'B' -> binary * -> fuss
+			if($types =~ m/(^_*)(B)$/) {
+			    CHK("ARITHMETIC_CONTINUATIONS",
+				"Arithmetic expressions should be on one line\n" . $hereprev);
+			}
+
+		}
+# check for /,% and * at beginning of a line but exclude *var
+		if ($rawline =~ /^\+\s*(\*|\/|%).*$/) {
+			# need to check the type here
+			my $opline = $line; $opline =~ s/^./ /;
+			my ($curr_values, $types) = annotate_values($opline . "\n", $prev_values);
+			# first entry is type 'B' -> binary * -> fuss
+			if($types =~ m/(^_*)(B)/) {
+			    CHK("ARITHMETIC_CONTINUATIONS",
+				"Arithmetic expressions should be on one line\n" . $hereprev);
+			}
+
+		}
+
 # check multi-line statement indentation matches previous line
 		if ($^V && $^V ge 5.10.0 &&
 		    $prevline =~ /^\+([ \t]*)((?:$c90_Keywords(?:\s+if)\s*)|(?:$Declare\s*)?(?:$Ident|\(\s*\*\s*$Ident\s*\))\s*|$Ident\s*=\s*$Ident\s*)\(.*(\&\&|\|\||,)\s*$/) {
-- 
1.7.10.4

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