From 938224b5e596c1c30d968ffd927a578ea7c4f45b Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Wed, 20 Jan 2016 14:59:15 -0800 Subject: checkpatch: warn when casting constants to c90 int or longer types Linus Torvalds wrote: > I can't but help to react that this: > #define IOMMU_ERROR_CODE (~(unsigned long) 0) > Not that this *matters*, but it's a bit odd to have to cast constants > to perfectly regular C types. So add a test that looks for constants that are cast to standard C90 int or longer types and suggest using C90 "6.4.4.1 Integer constants" integer-suffixes instead. Miscellanea: o Add a --fix option too Signed-off-by: Joe Perches Suggested-by: Andrew Morton Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index c7bf1aa2eeb3..86457062db29 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -433,6 +433,28 @@ our @typeList = ( qr{${Ident}_handler_fn}, @typeListMisordered, ); + +our $C90_int_types = qr{(?x: + long\s+long\s+int\s+(?:un)?signed| + long\s+long\s+(?:un)?signed\s+int| + long\s+long\s+(?:un)?signed| + (?:(?:un)?signed\s+)?long\s+long\s+int| + (?:(?:un)?signed\s+)?long\s+long| + int\s+long\s+long\s+(?:un)?signed| + int\s+(?:(?:un)?signed\s+)?long\s+long| + + long\s+int\s+(?:un)?signed| + long\s+(?:un)?signed\s+int| + long\s+(?:un)?signed| + (?:(?:un)?signed\s+)?long\s+int| + (?:(?:un)?signed\s+)?long| + int\s+long\s+(?:un)?signed| + int\s+(?:(?:un)?signed\s+)?long| + + int\s+(?:un)?signed| + (?:(?:un)?signed\s+)?int +)}; + our @typeListFile = (); our @typeListWithAttr = ( @typeList, @@ -5272,6 +5294,26 @@ sub process { } } +# check for cast of C90 native int or longer types constants + if ($line =~ /(\(\s*$C90_int_types\s*\)\s*)($Constant)\b/) { + my $cast = $1; + my $const = $2; + if (WARN("TYPECAST_INT_CONSTANT", + "Unnecessary typecast of c90 int constant\n" . $herecurr) && + $fix) { + my $suffix = ""; + my $newconst = $const; + $newconst =~ s/${Int_type}$//; + $suffix .= 'U' if ($cast =~ /\bunsigned\b/); + if ($cast =~ /\blong\s+long\b/) { + $suffix .= 'LL'; + } elsif ($cast =~ /\blong\b/) { + $suffix .= 'L'; + } + $fixed[$fixlinenr] =~ s/\Q$cast\E$const\b/$newconst$suffix/; + } + } + # check for sizeof(&) if ($line =~ /\bsizeof\s*\(\s*\&/) { WARN("SIZEOF_ADDRESS", -- cgit From 62e15a6daab0f6b3e8233e976201bce9faecaaf7 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Wed, 20 Jan 2016 14:59:18 -0800 Subject: checkpatch: improve macros with flow control test The current test excludes any macro with ## concatenation from being reported with hidden flow control. Some macros are used with return or goto statements along with ##args or ##__VA_ARGS__. A somewhat common case is a logging macro like pr_info(fmt, ...) then a return or goto statement. Check the concatenated variable for args or __VA_ARGS__ and allow those macros to also be reported when they contain a return or goto. Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 86457062db29..77b293d5ce4f 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -4539,7 +4539,7 @@ sub process { #print "LINE<$lines[$ln-1]> len<" . length($lines[$ln-1]) . "\n"; $has_flow_statement = 1 if ($ctx =~ /\b(goto|return)\b/); - $has_arg_concat = 1 if ($ctx =~ /\#\#/); + $has_arg_concat = 1 if ($ctx =~ /\#\#/ && $ctx !~ /\#\#\s*(?:__VA_ARGS__|args)\b/); $dstat =~ s/^.\s*\#\s*define\s+$Ident(?:\([^\)]*\))?\s*//; $dstat =~ s/$;//g; -- cgit From 6b10df4257367dd0ead49f88df473972c00a8b5c Mon Sep 17 00:00:00 2001 From: Vladimir Zapolskiy Date: Wed, 20 Jan 2016 14:59:21 -0800 Subject: checkpatch: fix a number of COMPLEX_MACRO false positives A simple search over the kernel souce displays a number of correctly defined multiline macro, which generally are used as an array element initializer: % find ../linux -type f | xargs grep -B1 -H '^[:space]*\[.*\\$' However checkpatch.pl unexpectedly complains about all these macro definitions: % ./scripts/checkpatch.pl --types COMPLEX_MACRO -f include/linux/perf/arm_pmu.h ERROR: Macros with complex values should be enclosed in parentheses +#define PERF_MAP_ALL_UNSUPPORTED \ + [0 ... PERF_COUNT_HW_MAX - 1] = HW_OP_UNSUPPORTED The change intends to fix this type of false positives by flattening only array members and skipping array element designators. Signed-off-by: Vladimir Zapolskiy Acked-by: Joe Perches Cc: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 77b293d5ce4f..0147c91fa549 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -4550,7 +4550,7 @@ sub process { # Flatten any parentheses and braces while ($dstat =~ s/\([^\(\)]*\)/1/ || $dstat =~ s/\{[^\{\}]*\}/1/ || - $dstat =~ s/\[[^\[\]]*\]/1/) + $dstat =~ s/.\[[^\[\]]*\]/1/) { } @@ -4570,7 +4570,8 @@ sub process { union| struct| \.$Ident\s*=\s*| - ^\"|\"$ + ^\"|\"$| + ^\[ }x; #print "REST<$rest> dstat<$dstat> ctx<$ctx>\n"; if ($dstat ne '' && -- cgit