scripts:checkpatch - correct the error message during check

PATCH In review [email protected] 3 participants 4 messages Oct 30, 2015 1 message not in the archive Checkpatch

lore

Share & tools

lore

[email protected]

author

2015-10-30 11:25 UTC #1 lore raw

PATCH 1/1 scripts:checkpatch - correct the error message during check

In reply to a message not in the archive

From: Shailendra Verma <[email protected]>

Signed-off-by Shailendra Verma <[email protected]>

---
 scripts/checkpatch.pl | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

scripts/checkpatch.pl +6 -6

index c5ec977..b2436d3 100755
@@ -3169,21 +3169,21 @@ sub process {
 		}
 
 # check for global initialisers.
-		if ($line =~ /^\+$Type\s*$Ident(?:\s+$Modifier)*\s*=\s*(?:0|NULL|false)\s*;/) {
+		if ($line =~ /^\+$Type\s*$Ident(?:\s+$Modifier)*\s*=\s*(0x0|0|NULL|false)\s*;/) {
 			if (ERROR("GLOBAL_INITIALISERS",
-				  "do not initialise globals to 0 or NULL\n" .
+				  "do not initialise globals to $1\n" .
 				      $herecurr) &&
 			    $fix) {
-				$fixed[$fixlinenr] =~ s/(^.$Type\s*$Ident(?:\s+$Modifier)*)\s*=\s*(0|NULL|false)\s*;/$1;/;
+				$fixed[$fixlinenr] =~ s/(^.$Type\s*$Ident(?:\s+$Modifier)*)\s*=\s*(0x0|0|NULL|false)\s*;/$1;/;
 			}
 		}
 # check for static initialisers.
-		if ($line =~ /^\+.*\bstatic\s.*=\s*(0|NULL|false)\s*;/) {
+		if ($line =~ /^\+.*\bstatic\s.*=\s*(0x0|0|NULL|false)\s*;/) {
 			if (ERROR("INITIALISED_STATIC",
-				  "do not initialise statics to 0 or NULL\n" .
+				  "do not initialise statics to $1\n" .
 				      $herecurr) &&
 			    $fix) {
-				$fixed[$fixlinenr] =~ s/(\bstatic\s.*?)\s*=\s*(0|NULL|false)\s*;/$1;/;
+				$fixed[$fixlinenr] =~ s/(\bstatic\s.*?)\s*=\s*(0x0|0|NULL|false)\s*;/$1;/;
 			}
 		}

Signature · 1 line

1.9.1

Andy Whitcroft

maintainer

2015-10-30 12:56 UTC #2 lore raw

Quoted 28 lines · On Fri, Oct 30, 2015 at 04:55:04PM +0530, [email protected] wrote:

From: Shailendra Verma <[email protected]>

Signed-off-by Shailendra Verma <[email protected]>

---
 scripts/checkpatch.pl | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

scripts/checkpatch.pl +3 -4

index c5ec977..b2436d3 100755
@@ -3169,21 +3169,21 @@ sub process {
 		}
 
 # check for global initialisers.
-		if ($line =~ /^\+$Type\s*$Ident(?:\s+$Modifier)*\s*=\s*(?:0|NULL|false)\s*;/) {
+		if ($line =~ /^\+$Type\s*$Ident(?:\s+$Modifier)*\s*=\s*(0x0|0|NULL|false)\s*;/) {
 			if (ERROR("GLOBAL_INITIALISERS",
-				  "do not initialise globals to 0 or NULL\n" .
+				  "do not initialise globals to $1\n" .
 				      $herecurr) &&
 			    $fix) {
-				$fixed[$fixlinenr] =~ s/(^.$Type\s*$Ident(?:\s+$Modifier)*)\s*=\s*(0|NULL|false)\s*;/$1;/;
+				$fixed[$fixlinenr] =~ s/(^.$Type\s*$Ident(?:\s+$Modifier)*)\s*=\s*(0x0|0|NULL|false)\s*;/$1;/;
 			}
 		}
 # check for static initialisers.
-		if ($line =~ /^\+.*\bstatic\s.*=\s*(0|NULL|false)\s*;/) {

scripts/checkpatch.pl sub process { · line 3188

Quoted 10 lines

scripts/checkpatch.pl +3 -2

+		if ($line =~ /^\+.*\bstatic\s.*=\s*(0x0|0|NULL|false)\s*;/) {
 			if (ERROR("INITIALISED_STATIC",
-				  "do not initialise statics to 0 or NULL\n" .
+				  "do not initialise statics to $1\n" .
 				      $herecurr) &&
 			    $fix) {
-				$fixed[$fixlinenr] =~ s/(\bstatic\s.*?)\s*=\s*(0|NULL|false)\s*;/$1;/;
+				$fixed[$fixlinenr] =~ s/(\bstatic\s.*?)\s*=\s*(0x0|0|NULL|false)\s*;/$1;/;
 			}
 		}

I guess that looks ok. Joe you don't know of anyone relying on the form of the error do you (I do not)?

-apw

Joe Perches

maintainer

2015-10-30 15:29 UTC #3 lore raw

Quoted 2 lines · Andy Whitcroft

Quoted 1 line · On Fri, Oct 30, 2015 at 04:55:04PM +0530, [email protected] wrote:
From: Shailendra Verma <[email protected]>

[]

Quoted 1 line

scripts/checkpatch.pl
Quoted 1 line
scripts/checkpatch.pl +0 -0

[]

Quoted 22 lines

scripts/checkpatch.pl +5 -5

@@ -3169,21 +3169,21 @@ sub process {
 		}
 
 # check for global initialisers.
-		if ($line =~ /^\+$Type\s*$Ident(?:\s+$Modifier)*\s*=\s*(?:0|NULL|false)\s*;/) {
+		if ($line =~ /^\+$Type\s*$Ident(?:\s+$Modifier)*\s*=\s*(0x0|0|NULL|false)\s*;/) {
 			if (ERROR("GLOBAL_INITIALISERS",
-				  "do not initialise globals to 0 or NULL\n" .
+				  "do not initialise globals to $1\n" .
 				      $herecurr) &&
 			    $fix) {
-				$fixed[$fixlinenr] =~ s/(^.$Type\s*$Ident(?:\s+$Modifier)*)\s*=\s*(0|NULL|false)\s*;/$1;/;
+				$fixed[$fixlinenr] =~ s/(^.$Type\s*$Ident(?:\s+$Modifier)*)\s*=\s*(0x0|0|NULL|false)\s*;/$1;/;
 			}
 		}
 # check for static initialisers.
-		if ($line =~ /^\+.*\bstatic\s.*=\s*(0|NULL|false)\s*;/) {
+		if ($line =~ /^\+.*\bstatic\s.*=\s*(0x0|0|NULL|false)\s*;/) {
 			if (ERROR("INITIALISED_STATIC",
-				  "do not initialise statics to 0 or NULL\n" .
+				  "do not initialise statics to $1\n" .
 				      $herecurr) &&

Quoted 8 lines

scripts/checkpatch.pl sub process { · line 3188
Quoted 5 lines
scripts/checkpatch.pl +1 -1
 			    $fix) {
-				$fixed[$fixlinenr] =~ s/(\bstatic\s.*?)\s*=\s*(0|NULL|false)\s*;/$1;/;
+				$fixed[$fixlinenr] =~ s/(\bstatic\s.*?)\s*=\s*(0x0|0|NULL|false)\s*;/$1;/;
 			}
 		}
I guess that looks ok. Joe you don't know of anyone relying on the form of the error do you (I do not)?

I do not.

If these are changed, perhaps making the tests

	"((?:0[xX]|)0+$Int_type?|NULL|false)"

might catch a couple more of these;

Joe Perches

maintainer

2015-10-30 19:05 UTC #4 lore raw

PATCH 1/1 checkpatch: Improve the unnecessary initialisers tests

In reply to Andy Whitcroft

Global and static variables don't need to be initialized to 0.

There is already a test for this but the output message doesn't mention booleans initialized to false.

Improve the output message and the test by adding various forms with possible specific integer types and possible multiple zeros.

Miscellanea:

o Use a variable to hold the possible 0 test

Signed-off-by Shailendra Verma <[email protected]>

Signed-off-by Joe Perches <[email protected]>

---
 scripts/checkpatch.pl | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

scripts/checkpatch.pl +8 -7

index 2d88cbf9..2b3c228 100755
@@ -370,6 +370,8 @@ our $typeTypedefs = qr{(?x:
 	$typeKernelTypedefs\b
 )};
 
+our $zero_initializer = qr{(?:(?:0[xX])?0+$Int_type?|NULL|false)\b};
+
 our $logFunctions = qr{(?x:
 	printk(?:_ratelimited|_once|)|
 	(?:[a-z0-9]+_){1,2}(?:printk|emerg|alert|crit|err|warning|warn|notice|info|debug|dbg|vdbg|devel|cont|WARN)(?:_ratelimited|_once|)|

@@ -3334,21 +3336,20 @@ sub process {
 		}
 
 # check for global initialisers.
-		if ($line =~ /^\+$Type\s*$Ident(?:\s+$Modifier)*\s*=\s*(?:0|NULL|false)\s*;/) {
+		if ($line =~ /^\+$Type\s*$Ident(?:\s+$Modifier)*\s*=\s*($zero_initializer)\s*;/) {
 			if (ERROR("GLOBAL_INITIALISERS",
-				  "do not initialise globals to 0 or NULL\n" .
-				      $herecurr) &&
+				  "do not initialise globals to $1\n" . $herecurr) &&
 			    $fix) {
-				$fixed[$fixlinenr] =~ s/(^.$Type\s*$Ident(?:\s+$Modifier)*)\s*=\s*(0|NULL|false)\s*;/$1;/;
+				$fixed[$fixlinenr] =~ s/(^.$Type\s*$Ident(?:\s+$Modifier)*)\s*=\s*$zero_initializer\s*;/$1;/;
 			}
 		}
 # check for static initialisers.
-		if ($line =~ /^\+.*\bstatic\s.*=\s*(0|NULL|false)\s*;/) {
+		if ($line =~ /^\+.*\bstatic\s.*=\s*($zero_initializer)\s*;/) {
 			if (ERROR("INITIALISED_STATIC",
-				  "do not initialise statics to 0 or NULL\n" .
+				  "do not initialise statics to $1\n" .
 				      $herecurr) &&
 			    $fix) {
-				$fixed[$fixlinenr] =~ s/(\bstatic\s.*?)\s*=\s*(0|NULL|false)\s*;/$1;/;
+				$fixed[$fixlinenr] =~ s/(\bstatic\s.*?)\s*=\s*$zero_initializer\s*;/$1;/;
 			}
 		}