From ec1de490a64368b172f2e7415497efba3eb66ce1 Mon Sep 17 00:00:00 2001 From: Nikolay Shirokovskiy Date: Tue, 4 Oct 2022 10:19:51 +0300 Subject: [PATCH 01/69] Fix a false positive MACRO_ARG_PRECEDENCE case Example which gives the FP: #define uptr_field_addr(obj, field) \ ({ \ (typeof(&((typeof(obj))0)->field)) \ ((char *)(obj) + (size_t)&((typeof(obj))0)->field); \ }) ERROR: Macro argument 'field' may be better as '(field)' to avoid precedence issues And using () around `field` won't compile. Closes #43 --- checkpatch.pl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/checkpatch.pl b/checkpatch.pl index 67d8d25..f855e74 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -4340,7 +4340,8 @@ sub process { # check if any macro arguments may have other precedence issues if ($tmp_stmt =~ m/($Operators)?\s*\b$arg\b\s*($Operators)?/m && ((defined($1) && $1 ne ',') || - (defined($2) && $2 ne ','))) { + (defined($2) && $2 ne ',')) && + !(defined($1) && ($1 eq '->' || $1 eq '.'))) { ERROR("MACRO_ARG_PRECEDENCE", "Macro argument '$arg' may be better as '($arg)' to avoid precedence issues\n" . "$herectx"); } From 2e16816f61275c1da427ef862a32cf8d179ef64a Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Fri, 7 Oct 2022 10:56:02 +0300 Subject: [PATCH 02/69] Fix FP REPEATED_WORD error for variable declarations Don't complain about variable declarations in C/C++ and protobuf files: Foo foo; Bar bar = 123; required Buzz buzz; Closes #45 --- checkpatch.pl | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/checkpatch.pl b/checkpatch.pl index f855e74..ede529a 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2693,7 +2693,7 @@ sub process { my $second = $2; my $start_pos = $-[1]; my $end_pos = $+[2]; - if ($first =~ /(?:struct|union|enum)/) { + if ($first =~ /(?:class|struct|union|enum)/) { pos($rawline) += length($first) + length($second) + 1; next; } @@ -2705,6 +2705,10 @@ sub process { # @param request Request to process next if $rawline =~ /^\+\s\*\s+[\@\\]param(?:\[[a-z,]*\])*\s+$first $second/; + # Ignore variable declarations. + next if $realfile =~ /.h|c|cc|proto$/ && + $rawline =~ /^\+\s*(?:(?:$Storage|$Modifier|optional|required|repeated)\s+)*$first\s+$second\s*[=;]/; + # check for character before and after the word matches my $start_char = ''; my $end_char = ''; From 9479d77317693086e0ec321a22f983bc035a36a0 Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Sun, 16 Oct 2022 01:38:07 +0300 Subject: [PATCH 03/69] spelling: writeable -> writable The correct spelling was a surprise for me (shame on me), but I also discovered that it is not uncommon in tarantool code and its commit history. --- spelling.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spelling.txt b/spelling.txt index 0c8b79c..cef803e 100644 --- a/spelling.txt +++ b/spelling.txt @@ -2,6 +2,8 @@ # removed, and various additions have been made as they've been discovered # in the kernel source. # +# Tarantool authors added a few common mistakes in this checkpatch fork. +# # License: GPLv2 # # The format of each line is: @@ -1619,6 +1621,7 @@ withing||within wnat||want wont||won't workarould||workaround +writeable||writable writeing||writing writting||writing wtih||with From 7b7ed584e4e107b3343ef901da60b6fccc237a22 Mon Sep 17 00:00:00 2001 From: Serge Petrenko Date: Mon, 31 Oct 2022 10:34:51 +0300 Subject: [PATCH 04/69] Add getenv to the list of unsafe functions getenv() is bad: it returns a pointer to the environment, which might be changed by a following call to setenv(), making the value pointed to longer and leading to buffer overflows. See https://github.com/tarantool/tarantool/pull/7807 --- checkpatch.pl | 1 + 1 file changed, 1 insertion(+) diff --git a/checkpatch.pl b/checkpatch.pl index ede529a..567894c 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -4922,6 +4922,7 @@ sub process { if ($line =~ /\b($Ident)\s*\(/) { my $func = $1; my %func_list = ( + "getenv" => "getenv_safe", "sprintf" => "snprintf", "vsprintf" => "vsnprintf", "strcpy" => "strlcpy", From 7fbc8e61576354813d46312c436c637545d1b62f Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Fri, 11 Nov 2022 17:26:30 +0300 Subject: [PATCH 05/69] Allow complex macros in tests Useful for test code generation. --- checkpatch.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/checkpatch.pl b/checkpatch.pl index 567894c..5f42c9f 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -4287,7 +4287,7 @@ sub process { my $stmt_cnt = statement_rawlines($ctx); my $herectx = get_stat_here($linenr, $stmt_cnt, $here); - if ($dstat ne '' && + if (!$is_test && $dstat ne '' && !($dstat =~ /,/ && $dstat !~ /;/) && # comma-separated list used as array initializer $dstat !~ /^(?:$Ident|-?$Constant),$/ && # 10, // foo(), $dstat !~ /^(?:$Ident|-?$Constant);$/ && # foo(); @@ -4353,7 +4353,7 @@ sub process { # check for macros with flow control, but without ## concatenation # ## concatenation is commonly a macro that defines a function so ignore those - if ($has_flow_statement && !$has_arg_concat) { + if (!$is_test && $has_flow_statement && !$has_arg_concat) { my $cnt = statement_rawlines($ctx); my $herectx = get_stat_here($linenr, $cnt, $here); From 24a32ea7ab001d853752b3804d6fb17e7af231ce Mon Sep 17 00:00:00 2001 From: Ilya Verbin Date: Wed, 16 Nov 2022 15:46:31 +0300 Subject: [PATCH 06/69] spelling: yeild -> yield $ git grep yeild | wc -l 35 --- spelling.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/spelling.txt b/spelling.txt index cef803e..0ada5d2 100644 --- a/spelling.txt +++ b/spelling.txt @@ -1625,5 +1625,6 @@ writeable||writable writeing||writing writting||writing wtih||with +yeild||yield zombe||zombie zomebie||zombie From 987055ce92786a3558e0281f7771dcc5c9bb215c Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Fri, 2 Dec 2022 12:48:33 +0300 Subject: [PATCH 07/69] Check for long lines in Lua files --- checkpatch.pl | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/checkpatch.pl b/checkpatch.pl index 5f42c9f..9cc186c 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2785,19 +2785,14 @@ sub process { "Please avoid new tests with .result files\n"); } -# check we are in a valid C source file if not then ignore this hunk - next if ($realfile !~ /\.(h|c|cc)$/); +# check we are in a valid source file if not then ignore this hunk + next if ($realfile !~ /\.(h|c|cc|lua)$/); -# ignore C source files outside the source and test directories when checking patches +# ignore source files outside the source and test directories when checking patches next if !$file and ($realfile !~ /^(?:src|test|perf)\// || $realfile =~ /^$skipSrcPaths/); $is_test = ($realfile =~ /^(?:test|perf)\//); - if ($line =~ /^\+\s*#\s*ifndef\s+[A-Z0-9_]+(?:_H|_INCLUDED)\s*$/) { - ERROR("INCLUDE_GUARD", - "Please use '#pragma once' instead of include guard macro\n" . $herecurr); - } - # line length limit (with some exclusions) # # There are a few types of lines that may extend beyond $max_line_length: @@ -2871,6 +2866,15 @@ sub process { } } +# check we are in a valid C source file if not then ignore this hunk + next if ($realfile !~ /\.(h|c|cc)$/); + +# require pragma instead of include guards + if ($line =~ /^\+\s*#\s*ifndef\s+[A-Z0-9_]+(?:_H|_INCLUDED)\s*$/) { + ERROR("INCLUDE_GUARD", + "Please use '#pragma once' instead of include guard macro\n" . $herecurr); + } + # at the beginning of a line any tabs must come first and anything # more than $tabsize must use tabs. if ($rawline =~ /^\+\s* \t\s*\S/ || From d266ea68a29984bdab6a8d411e0cbccfdef6a5de Mon Sep 17 00:00:00 2001 From: Nick Volynkin Date: Mon, 12 Dec 2022 20:43:30 +0600 Subject: [PATCH 08/69] Create action.yml Use actions/checkout@v3, running on Node.js 16 Removes the following warning from workflows, using tarantool/checkpatch: > Node.js 12 actions are deprecated. For more information see: https://github.blog/changelog/2022-09-22-github-actions-all-actions-will-begin-running-on-node16-instead-of-node12/. Please update the following actions to use Node.js 16: actions/checkout@v2 --- .github/actions/checkpatch/action.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/checkpatch/action.yml b/.github/actions/checkpatch/action.yml index a270700..5cc36ac 100644 --- a/.github/actions/checkpatch/action.yml +++ b/.github/actions/checkpatch/action.yml @@ -7,7 +7,7 @@ inputs: runs: using: composite steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 with: repository: tarantool/checkpatch path: 'checkpatch' From 58825666fb665ba5a1f9351dc21c625cf4f01f4b Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Wed, 14 Dec 2022 12:43:52 +0300 Subject: [PATCH 09/69] Enable codespell in GH action --- .github/actions/checkpatch/action.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/actions/checkpatch/action.yml b/.github/actions/checkpatch/action.yml index 5cc36ac..d4f1ad9 100644 --- a/.github/actions/checkpatch/action.yml +++ b/.github/actions/checkpatch/action.yml @@ -11,5 +11,6 @@ runs: with: repository: tarantool/checkpatch path: 'checkpatch' - - run: checkpatch/checkpatch.pl --color=always --show-types --git ${{ inputs.revision-range }} + - run: apt install -y codespell + - run: checkpatch/checkpatch.pl --codespell --color=always --show-types --git ${{ inputs.revision-range }} shell: bash From 50741d1bf06f3ad5c84146c585bd8e861f5070b7 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Wed, 14 Dec 2022 13:46:56 +0300 Subject: [PATCH 10/69] Fix GH action Fixes commit 58825666fb66 ("Enable codespell in GH action"). --- .github/actions/checkpatch/action.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/actions/checkpatch/action.yml b/.github/actions/checkpatch/action.yml index d4f1ad9..db837be 100644 --- a/.github/actions/checkpatch/action.yml +++ b/.github/actions/checkpatch/action.yml @@ -12,5 +12,6 @@ runs: repository: tarantool/checkpatch path: 'checkpatch' - run: apt install -y codespell + shell: bash - run: checkpatch/checkpatch.pl --codespell --color=always --show-types --git ${{ inputs.revision-range }} shell: bash From 6fb46e1adf4169315533fc544a049f632a1ebb6a Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Wed, 21 Dec 2022 14:44:37 +0300 Subject: [PATCH 11/69] Don't check LONG_LINE for Lua diff tests --- checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkpatch.pl b/checkpatch.pl index 9cc186c..dfbdd47 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2786,7 +2786,7 @@ sub process { } # check we are in a valid source file if not then ignore this hunk - next if ($realfile !~ /\.(h|c|cc|lua)$/); + next if ($realfile !~ /\.(h|c|cc|lua)$/ || $realfile =~ /\.test\.lua$/); # ignore source files outside the source and test directories when checking patches next if !$file and ($realfile !~ /^(?:src|test|perf)\// || $realfile =~ /^$skipSrcPaths/); From c01f182fe7c7d6f483a63f299b4aed11cec1c459 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Thu, 22 Dec 2022 15:10:08 +0300 Subject: [PATCH 12/69] Detect misalignment of string literal break E.g. printf("hello " "world\n"); --- checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkpatch.pl b/checkpatch.pl index dfbdd47..0201eea 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2914,7 +2914,7 @@ sub process { } # check multi-line statement indentation matches previous line - if ($prevline =~ /^\+([ \t]*)((?:$c90_Keywords(?:\s+if)\s*)|(?:$Declare\s*)?(?:$Ident|\(\s*\*\s*$Ident\s*\))\s*|(?:(?:\*\s*)*$Lval|$Declare\s*$Ident)\s*=\s*$Ident\s*)\(.*(\&\&|\|\||,)\s*$/) { + if ($prevline =~ /^\+([ \t]*)((?:$c90_Keywords(?:\s+if)\s*)|(?:$Declare\s*)?(?:$Ident|\(\s*\*\s*$Ident\s*\))\s*|(?:(?:\*\s*)*$Lval|$Declare\s*$Ident)\s*=\s*$Ident\s*)\(.*(\&\&|\|\||,|")\s*$/) { $prevline =~ /^\+(\t*)(.*)$/; my $oldindent = $1; my $rest = $2; From f28eb94fe21e8ad06273af5c2ca8d6e55e131f61 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Wed, 28 Dec 2022 11:01:48 +0300 Subject: [PATCH 13/69] Allow line break between = and {...} in initialization --- checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkpatch.pl b/checkpatch.pl index 0201eea..72e2c65 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -3460,7 +3460,7 @@ sub process { } # check for initialisation to aggregates open brace on the next line - if ($line =~ /^.\s*{/ && + if ($line =~ /^.\s*{/ && $line !~ /}\s*;\s*$/ && $prevline =~ /(?:^|[^=])=\s*$/) { ERROR("OPEN_BRACE", "that open brace { should be on the previous line\n" . $hereprev); From 05126462ec35b6603905ca52825e02567504f627 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Thu, 29 Dec 2022 13:16:00 +0300 Subject: [PATCH 14/69] Ignore typos in anything that looks like regular expression --- checkpatch.pl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/checkpatch.pl b/checkpatch.pl index 72e2c65..c649710 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2640,8 +2640,11 @@ sub process { # Check for various typo / spelling mistakes if (defined($misspellings) && ($in_commit_log || $line =~ /^(?:\+|Subject:)/i)) { - while ($rawline =~ /(?:^|[^\w\-'`])($misspellings)(?:[^\w\-'`]|$)/gi) { - my $typo = $1; + while ($rawline =~ /(^|[^\w\-'`])($misspellings)(?:[^\w\-'`]|$)/gi) { + my $prev = $1; + my $typo = $2; + # Ignore anything that looks like regular expression. + next if $prev =~ /[.|]/; # Ignore ALL UPPER CASE words as it may be an abbreviation, # e.g. TAHT - Tahiti Time next if ($typo eq uc($typo)); From 13b5d07d1cae67440b65f845f3d1b9c290cb5074 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Mon, 9 Jan 2023 14:06:39 +0300 Subject: [PATCH 15/69] Check that code ident matches the comment on the previous line --- checkpatch.pl | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/checkpatch.pl b/checkpatch.pl index c649710..7ca921f 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2895,6 +2895,15 @@ sub process { "code indent shouldn't use spaces if the previous line ends with $what\n" . $herevet); } +# alignment should match the comment on the previous line unless it's the end of a function or struct + if ($prevrawline =~ /^.(\s*)(?:\/\*.*| )\*\/\s*$/) { + my $ident = length($1); + if ($rawline =~ /^\+(\s*)[^\}]/ && length($1) != $ident) { + ERROR("CODE_IDENT", + "code indent should match comment\n" . $hereprev); + } + } + # check for assignments on the start of a line if ($sline =~ /^\+\s+($Assignment)[^=]/) { ERROR("ASSIGNMENT_CONTINUATIONS", From dd81310adefe089a71a0c04cee341c3013543309 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Tue, 17 Jan 2023 15:16:02 +0300 Subject: [PATCH 16/69] Ignore typos in anything that looks like file path --- checkpatch.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/checkpatch.pl b/checkpatch.pl index 7ca921f..69ee5a7 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2643,8 +2643,8 @@ sub process { while ($rawline =~ /(^|[^\w\-'`])($misspellings)(?:[^\w\-'`]|$)/gi) { my $prev = $1; my $typo = $2; - # Ignore anything that looks like regular expression. - next if $prev =~ /[.|]/; + # Ignore anything that looks like regular expression or file path. + next if $prev =~ /[.|\/]/; # Ignore ALL UPPER CASE words as it may be an abbreviation, # e.g. TAHT - Tahiti Time next if ($typo eq uc($typo)); From fa8c89c8ad80a49d031b059bd27ed4bf5bcdecc9 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Thu, 26 Jan 2023 18:49:53 +0300 Subject: [PATCH 17/69] Suppress COMPLEX_MACRO for compound code generation macros For example, #define FOO(a, b, c) \ FUZZ(a, b) \ BUZZ(c) --- checkpatch.pl | 1 + 1 file changed, 1 insertion(+) diff --git a/checkpatch.pl b/checkpatch.pl index 69ee5a7..543ffd7 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -4312,6 +4312,7 @@ sub process { $dstat !~ /$exceptions/ && $dstat !~ /^\.$Ident\s*=/ && # .foo = $dstat !~ /^(?:\#\s*$Ident|\#\s*$Constant)\s*$/ && # stringification #foo + $dstat !~ /^(?:$Ident\s*)+$/ && # compound code generation macros: foo() bar() buzz() $dstat !~ /^do\s*$Constant\s*while\s*$Constant;?$/ && # do {...} while (...); // do {...} while (...) $dstat !~ /^while\s*$Constant\s*$Constant\s*$/ && # while (...) {...} $dstat !~ /^for\s*$Constant$/ && # for (...) From e4a6e78a53e481ee5329114d75803ee61d6e5f90 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Fri, 10 Feb 2023 16:56:30 +0300 Subject: [PATCH 18/69] Fix FP REPEATED_WORD error for 'not not' in Lua It's a common programming idiom. --- checkpatch.pl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/checkpatch.pl b/checkpatch.pl index 543ffd7..bdd838c 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2712,6 +2712,9 @@ sub process { next if $realfile =~ /.h|c|cc|proto$/ && $rawline =~ /^\+\s*(?:(?:$Storage|$Modifier|optional|required|repeated)\s+)*$first\s+$second\s*[=;]/; + # Ignore 'not not' in Lua. + next if $realfile =~ /\.lua$/ && $first eq 'not'; + # check for character before and after the word matches my $start_char = ''; my $end_char = ''; From 5fc4b0ab090614b63ae6c52561b702b63f7475fc Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Mon, 13 Feb 2023 13:34:09 +0300 Subject: [PATCH 19/69] Ignore space before tab in *.patch file In the unified diff format all unchanged lines start with a space so a space before tab is legal. --- checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkpatch.pl b/checkpatch.pl index bdd838c..c1bb352 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2770,7 +2770,7 @@ sub process { } # check for space before tabs. - if ($rawline =~ /^\+/ && $rawline =~ / \t/) { + if ($realfile !~ /\.patch$/ && $rawline =~ /^\+/ && $rawline =~ / \t/) { my $herevet = "$here\n" . cat_vet($rawline) . "\n"; ERROR("SPACE_BEFORE_TAB", "please, no space before tabs\n" . $herevet); From 48ef63b31216df75fd3b5594c8ac7dec540df3d8 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Mon, 13 Feb 2023 13:56:20 +0300 Subject: [PATCH 20/69] Ignore spelling typos in lines removed by patch Sometimes we commit patch files. If a line is removed or unchanged by a patch file, we should ignore spelling typos in it. --- checkpatch.pl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/checkpatch.pl b/checkpatch.pl index c1bb352..2a196d2 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2643,6 +2643,8 @@ sub process { while ($rawline =~ /(^|[^\w\-'`])($misspellings)(?:[^\w\-'`]|$)/gi) { my $prev = $1; my $typo = $2; + # Ignore if this is a patch file and the line is removed or unchanged. + next if $realfile =~ /\.patch$/ && $rawline !~ /^\+\+/; # Ignore anything that looks like regular expression or file path. next if $prev =~ /[.|\/]/; # Ignore ALL UPPER CASE words as it may be an abbreviation, From 7240e0b7a2eed8faaae6072a0e609cad284b2f10 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Mon, 27 Feb 2023 13:34:30 +0300 Subject: [PATCH 21/69] Suppress FUNCTION_NAME_NO_NEWLINE for generic_XXX Historically, we often declare generic callback functions in a compat fashion (place the function type and name on the same line). --- checkpatch.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/checkpatch.pl b/checkpatch.pl index 2a196d2..2f5d088 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -3594,8 +3594,8 @@ sub process { } # check that function name and return value type are placed on different lines - if ($realfile =~ /\bbox\.h$/ && $line =~ /^\+\s*$Declare\s*box_set_/) { - # ignore box_set_XXX in box.h + if ($line =~ /^\+\s*$Declare\s*(?:box_set_|generic_)/) { + # ignore box_set_XXX and generic_XXX } elsif ($line =~ /^\+(?:typedef\s+)?$Declare\s*(?:$Ident|\(\s*\*\s*$Ident\s*\))\s*\(/) { ERROR("FUNCTION_NAME_NO_NEWLINE", "Function name and return value type should be placed on different lines\n" . $herecurr) From f8b39d54fd484b7373f380b1ac757e09136a5953 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Tue, 21 Mar 2023 12:47:23 +0300 Subject: [PATCH 22/69] Don't warn about unsafe functions in tests Code simplicity is preferred over safety in tests. --- checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkpatch.pl b/checkpatch.pl index 2f5d088..be0e2f1 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -4941,7 +4941,7 @@ sub process { } # warn about unsafe functions - if ($line =~ /\b($Ident)\s*\(/) { + if (!$is_test && $line =~ /\b($Ident)\s*\(/) { my $func = $1; my %func_list = ( "getenv" => "getenv_safe", From b7cfb42f70e2d43a437e2640224443c78adf7566 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Wed, 22 Mar 2023 16:46:10 +0300 Subject: [PATCH 23/69] Drop EMAIL_SUBJECT check It's kernel specific. --- checkpatch.pl | 7 ------- doc/checkpatch.rst | 9 ++------- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/checkpatch.pl b/checkpatch.pl index be0e2f1..86c926f 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2483,13 +2483,6 @@ sub process { } } -# Check email subject for common tools that don't need to be mentioned - if ($in_header_lines && - $line =~ /^Subject:.*\b(?:checkpatch|sparse|smatch)\b[^:]/i) { - ERROR("EMAIL_SUBJECT", - "A patch subject line should describe the change not the tool that found it\n" . $herecurr); - } - # Check for Gerrit Change-Ids not in any patch context if ($realfile eq '' && !$has_patch_separator && $line =~ /^\s*change-id:/i) { ERROR("GERRIT_CHANGE_ID", diff --git a/doc/checkpatch.rst b/doc/checkpatch.rst index f99ab86..fc7779d 100644 --- a/doc/checkpatch.rst +++ b/doc/checkpatch.rst @@ -94,7 +94,7 @@ Available options: Example:: - ./scripts/checkpatch.pl mypatch.patch --types EMAIL_SUBJECT,BRACES + ./scripts/checkpatch.pl mypatch.patch --types LINE_SPACING,BRACES - --ignore TYPE(,TYPE2...) @@ -102,7 +102,7 @@ Available options: Example:: - ./scripts/checkpatch.pl mypatch.patch --ignore EMAIL_SUBJECT,BRACES + ./scripts/checkpatch.pl mypatch.patch --ignore LINE_SPACING,BRACES - --show-types @@ -246,11 +246,6 @@ Commit message The patch is missing a commit description. A brief description of the changes made by the patch should be added. - **EMAIL_SUBJECT** - Naming the tool that found the issue is not very useful in the - subject line. A good subject line summarizes the change that - the patch brings. - **FROM_SIGN_OFF_MISMATCH** The author's email does not match with that in the Signed-off-by: line(s). This can be sometimes caused due to an improperly configured From 3242de3fb20ca132347b0d1522f79883c226bdb7 Mon Sep 17 00:00:00 2001 From: Andrey Saranchin Date: Thu, 20 Apr 2023 11:41:55 +0300 Subject: [PATCH 24/69] Do not treat "acknowledgement" as misspelling Word "acknowledgement" is a British variant of word "acknowledgment". Since we use British dialect sometimes, we should not treat this word as misspelling. --- spelling.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/spelling.txt b/spelling.txt index 0ada5d2..1b5c1d0 100644 --- a/spelling.txt +++ b/spelling.txt @@ -53,7 +53,6 @@ acient||ancient acitions||actions acitve||active acknowldegement||acknowledgment -acknowledgement||acknowledgment ackowledge||acknowledge ackowledged||acknowledged acording||according From 1158019fa5a6359daf276c05c6b1d0def1726c38 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Thu, 20 Apr 2023 14:22:07 +0300 Subject: [PATCH 25/69] Fix FP MACRO_ARG_PRECEDENCE in case cast to pointer type Example: #define cast(T, p) (T *)(p) In this case T shouldn't be enclosed in parentheses. --- checkpatch.pl | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/checkpatch.pl b/checkpatch.pl index 86c926f..6568a1a 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -4357,10 +4357,11 @@ sub process { $tmp_stmt =~ s/\#+\s*$arg\b//g; $tmp_stmt =~ s/\b$arg\s*\#\#//g; # check if any macro arguments may have other precedence issues - if ($tmp_stmt =~ m/($Operators)?\s*\b$arg\b\s*($Operators)?/m && - ((defined($1) && $1 ne ',') || - (defined($2) && $2 ne ',')) && - !(defined($1) && ($1 eq '->' || $1 eq '.'))) { + if ($tmp_stmt =~ m/(\()?\s*($Operators)?\s*\b$arg\b\s*($Operators)?[\s\*]*(\))?/m && + ((defined($2) && $2 ne ',') || + (defined($3) && $3 ne ',')) && + !(defined($2) && ($2 eq '->' || $2 eq '.')) && + !(defined($1) && !defined($2) && (!defined($3) || $3 eq '*') && defined($4))) { ERROR("MACRO_ARG_PRECEDENCE", "Macro argument '$arg' may be better as '($arg)' to avoid precedence issues\n" . "$herectx"); } From f783113630bd7508ef8b4a258752c15560af5f16 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Wed, 26 Apr 2023 10:58:20 +0300 Subject: [PATCH 26/69] Ignore CamelCase in TYPO_SPELLING check So that one can write, for example, "PullRequest" of "pullRequest" instead of "pull request". Closes #56 --- checkpatch.pl | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/checkpatch.pl b/checkpatch.pl index 6568a1a..a1dae3d 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2648,6 +2648,10 @@ sub process { my $hereptr = "$hereline$ptr\n"; my $typo_fix = $spelling_fix{lc($typo)}; $typo_fix = ucfirst($typo_fix) if ($typo =~ /^[A-Z]/); + # Ignore CamelCase. + my $typo_fix_camel_case = $typo_fix; + $typo_fix_camel_case =~ s/ (\w)/\U$1/g; + next if ($typo eq $typo_fix_camel_case); $typo_fix = uc($typo_fix) if ($typo =~ /^[A-Z]+$/); # Ignore the case when we drop a character to use a misspelled word as function name, e.g. isnt(). next if ($realfile =~ /\.h|c|cc|lua$/ && $typo_fix =~ /[^a-zA-Z_]/ && $line =~ /\b$typo\s*\(/); From 2fdbfc7bafbbcd1d7b426a69d3ecdfa1d7cd2e55 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Wed, 26 Apr 2023 11:27:51 +0300 Subject: [PATCH 27/69] Ignore uncommented trigger declarations Because we usually put a comment to a trigger function instead. Closes #54 --- checkpatch.pl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/checkpatch.pl b/checkpatch.pl index a1dae3d..84ca38a 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -4685,6 +4685,8 @@ sub process { } if (!defined($decl)) { # not a declaration + } elsif (!$is_func && $decl =~ /\bstruct\s+trigger\b/) { + # ignore trigger declarations because we usually add a comment to a trigger function instead } elsif (!$is_func && defined($context_function)) { # ignore local variables } elsif ($realfile !~ /\.h$/ && $decl !~ /\bstatic\b/ && !defined($context_struct)) { From b3a067a4b9d71b142438853856b0aba0ab2d0963 Mon Sep 17 00:00:00 2001 From: Andrey Saranchin Date: Wed, 26 Apr 2023 14:11:17 +0300 Subject: [PATCH 28/69] Allow line continuation after multiline comment If a multiline comment is written inside of a multiline macro, it's enough to put line continuation symbol at the last line of the comment. --- checkpatch.pl | 1 + 1 file changed, 1 insertion(+) diff --git a/checkpatch.pl b/checkpatch.pl index 84ca38a..ec81298 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -4385,6 +4385,7 @@ sub process { } else { if ($prevline !~ /^..*\\$/ && + $rawline !~ /^\+\s*\*\/\s*\\$/ && # multiline comment $line !~ /^\+\s*\#.*\\$/ && # preprocessor $line !~ /^\+.*\b(__asm__|asm)\b.*\\$/ && # asm $line =~ /^\+.*\\$/) { From 8feacf33f392466328045945f93c894adac4159c Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Thu, 11 May 2023 10:47:28 +0300 Subject: [PATCH 29/69] Disallow closing parenthesis on separate line This looks ugly and has no practical meaning: foo( 1, 2, 3 ); --- checkpatch.pl | 6 ++++++ doc/checkpatch.rst | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/checkpatch.pl b/checkpatch.pl index ec81298..ba99048 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -4096,6 +4096,12 @@ sub process { } } +# check for a closing parenthesis on a separate line + if ($line =~ /^\+\s*\)\s*[;{]?\s*$/) { + ERROR("DANGLING_PARENTHESIS", + "Closing parenthesis should follow function argument or operand\n" . $herecurr); + } + # comparisons with a constant or upper case identifier on the left # avoid cases like "foo + BAR < baz" # only fix matches surrounded by parentheses to avoid incorrect diff --git a/doc/checkpatch.rst b/doc/checkpatch.rst index fc7779d..7b898e9 100644 --- a/doc/checkpatch.rst +++ b/doc/checkpatch.rst @@ -642,6 +642,10 @@ Spacing and Brackets printk(KERN_INFO "bar"); + **DANGLING_PARENTHESIS** + A closing parenthesis should never be placed on a separate line. It should + always follow a function argument or an operand. + **ELSE_AFTER_BRACE** `else {` should follow the closing block `}` on the same line. From f095bf3dcb982c9cf482c936bf171ab200c709b6 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Fri, 19 May 2023 14:10:44 +0300 Subject: [PATCH 30/69] Ban non-ASCII characters in text files They may complicate grepping so better avoid them. Closes #58 --- checkpatch.pl | 9 +++++++++ doc/checkpatch.rst | 5 +++++ 2 files changed, 14 insertions(+) diff --git a/checkpatch.pl b/checkpatch.pl index ba99048..3c1e639 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2768,6 +2768,15 @@ sub process { "adding a line without newline at end of file\n" . $herecurr); } +# Ban non-ASCII characters in text files + if ($rawline =~ /^\+.*($NON_ASCII_UTF8)/) { + my $blank = copy_spacing($rawline); + my $ptr = substr($blank, 0, $-[1]) . "^"; + my $hereptr = "$hereline$ptr\n"; + ERROR("NON_ASCII_CHAR", + "please, don't use non-ASCII characters\n" . $hereptr); + } + # check for space before tabs. if ($realfile !~ /\.patch$/ && $rawline =~ /^\+/ && $rawline =~ / \t/) { my $herevet = "$here\n" . cat_vet($rawline) . "\n"; diff --git a/doc/checkpatch.rst b/doc/checkpatch.rst index 7b898e9..b44a759 100644 --- a/doc/checkpatch.rst +++ b/doc/checkpatch.rst @@ -740,6 +740,11 @@ Others The memset use appears to be incorrect. This may be caused due to badly ordered parameters. Please recheck the usage. + **NON_ASCII_CHAR** + Non-ASCII characters should be avoided because they may complicate + grepping. For example, Russian 'ะก' looks exactly like English 'C' + but grep doesn't find it. + **NO_CHANGELOG** The patch lacks a changelog. Please add a new changelog entry to the changelog/unreleased directory. If the patch doesn't need a changelog From ad44349def75bb5f2df23c8a1b9ae65ba71b4f53 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Mon, 22 May 2023 13:51:47 +0300 Subject: [PATCH 31/69] Fix ptr in NON_ASCII_CHAR error message We can't use $-[1] with UTF8 characters because rawline isn't decoded. Fixes commit f095bf3dcb98 ("Ban non-ASCII characters in text files"). --- checkpatch.pl | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/checkpatch.pl b/checkpatch.pl index 3c1e639..ed5a646 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2769,9 +2769,10 @@ sub process { } # Ban non-ASCII characters in text files - if ($rawline =~ /^\+.*($NON_ASCII_UTF8)/) { - my $blank = copy_spacing($rawline); - my $ptr = substr($blank, 0, $-[1]) . "^"; + if ($rawline =~ /^\+.*$NON_ASCII_UTF8/) { + my $l = $rawline; + $l =~ s/$NON_ASCII_UTF8.*//; + my $ptr = copy_spacing($l) . "^"; my $hereptr = "$hereline$ptr\n"; ERROR("NON_ASCII_CHAR", "please, don't use non-ASCII characters\n" . $hereptr); From 823819da9cc6706c0625990150ddb6f9e475878d Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Mon, 22 May 2023 14:06:32 +0300 Subject: [PATCH 32/69] Ignore NON_ASCII_CHAR in tests Sometimes we need to test UTF8 input. Fixes commit f095bf3dcb98 ("Ban non-ASCII characters in text files"). --- checkpatch.pl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/checkpatch.pl b/checkpatch.pl index ed5a646..b84f6d3 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2684,6 +2684,8 @@ sub process { $has_test = 1; } + $is_test = ($realfile =~ /^(?:test|perf)\//); + # check for repeated words separated by a single space # avoid false positive from list command eg, '-rw-r--r-- 1 root root' if (($rawline =~ /^\+/ || $in_commit_log) && @@ -2768,8 +2770,8 @@ sub process { "adding a line without newline at end of file\n" . $herecurr); } -# Ban non-ASCII characters in text files - if ($rawline =~ /^\+.*$NON_ASCII_UTF8/) { +# Ban non-ASCII characters in text files other than tests + if (!$is_test && $rawline =~ /^\+.*$NON_ASCII_UTF8/) { my $l = $rawline; $l =~ s/$NON_ASCII_UTF8.*//; my $ptr = copy_spacing($l) . "^"; @@ -2806,8 +2808,6 @@ sub process { # ignore source files outside the source and test directories when checking patches next if !$file and ($realfile !~ /^(?:src|test|perf)\// || $realfile =~ /^$skipSrcPaths/); - $is_test = ($realfile =~ /^(?:test|perf)\//); - # line length limit (with some exclusions) # # There are a few types of lines that may extend beyond $max_line_length: From 8061cd44357650b6d7fd0b283aaed9c35fe0c17d Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Mon, 22 May 2023 18:00:09 +0300 Subject: [PATCH 33/69] Check function argument alignment after opening parenthesis A function argument alignment after an opening parenthesis should be greater than the alignment of the function for better readability: * OK: some_function( x, y, z); if (some_function( x, y, z)) { do_something(); } * NOT OK: some_function( x, y, z); if (some_function( x, y, z)) { do_something(); } --- checkpatch.pl | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/checkpatch.pl b/checkpatch.pl index b84f6d3..8a4b13c 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2916,6 +2916,19 @@ sub process { } } +# check function argument alignment after opening parenthesis + if ($prevrawline =~ /^.(\s*)((?:if|while|for|switch)\s*\(.*)?\b$Ident\s*\(\s*$/) { + my $previndent = length(expand_tabs($1)); + my $in_cond = defined($2); + my $expected_tabs = ceil($previndent / $tabsize) + ($in_cond ? 2 : 1); + $rawline =~ /^\+(\s*)/; + my $indent = length(expand_tabs($1)); + if ($indent != $expected_tabs * $tabsize) { + ERROR("CODE_INDENT", + "bad code indent, should be $expected_tabs tabs\n" . $hereprev); + } + } + # check for assignments on the start of a line if ($sline =~ /^\+\s+($Assignment)[^=]/) { ERROR("ASSIGNMENT_CONTINUATIONS", From fd7983a7df4f11f7710226b2f007dd5be8002fe0 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Wed, 14 Jun 2023 11:40:49 +0300 Subject: [PATCH 34/69] Fix FP CODE_INDENT error for unchanged lines If a line is unchanged (starts from a space, not '+'), we fail to match it to '^\+(\s*)' and hence get invalid 'indent'. Fix this. Closes #60 --- checkpatch.pl | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/checkpatch.pl b/checkpatch.pl index 8a4b13c..e4fb232 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2921,11 +2921,12 @@ sub process { my $previndent = length(expand_tabs($1)); my $in_cond = defined($2); my $expected_tabs = ceil($previndent / $tabsize) + ($in_cond ? 2 : 1); - $rawline =~ /^\+(\s*)/; - my $indent = length(expand_tabs($1)); - if ($indent != $expected_tabs * $tabsize) { - ERROR("CODE_INDENT", - "bad code indent, should be $expected_tabs tabs\n" . $hereprev); + if ($rawline =~ /^\+(\s*)/) { + my $indent = length(expand_tabs($1)); + if ($indent != $expected_tabs * $tabsize) { + ERROR("CODE_INDENT", + "bad code indent, should be $expected_tabs tabs\n" . $hereprev); + } } } From 5516604054b897dbf620319ed8e97626b788c761 Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Wed, 21 Jun 2023 00:26:08 +0300 Subject: [PATCH 35/69] spelling: annotatoin -> annotation --- spelling.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/spelling.txt b/spelling.txt index 1b5c1d0..05715c6 100644 --- a/spelling.txt +++ b/spelling.txt @@ -131,6 +131,7 @@ an one||a one analysator||analyzer ang||and anniversery||anniversary +annotatoin||annotation annoucement||announcement anomolies||anomalies anomoly||anomaly From 32f49af19a53357c62fa6385367f2479c6f03338 Mon Sep 17 00:00:00 2001 From: Igor Munkin Date: Fri, 28 Jul 2023 08:12:31 +0000 Subject: [PATCH 36/69] Fix multiline commit reference check There was a classic mess with magic numbers implementing multiline commit reference check: the script looks for "commit" keyword preceding the commit hash and the commit subject in so-called "balanced parens". The script was assuming that the whole structure fits into a one or maximum two consequtive lines. However, with 40-symbol hash the whole reference can spread through three lines: commit + 40-symbol hash + up to 50-symbol commit message in parens and quotes. The aforementioned case occur when "commit" keyword is the only part found in the first line and the subject is split into two lines, since the commit hash takes 40 symbols of the 72 available per commit message line (see example here[1]). Hence, the magic number is simply incremented allowing to use full-length commit hashes in the reference. [1]: https://github.com/tarantool/tarantool/pull/8920/commits/aa8bffe Signed-off-by: Igor Munkin --- checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkpatch.pl b/checkpatch.pl index e4fb232..5699dd1 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2541,7 +2541,7 @@ sub process { my $input = $line; if ($line =~ /(?:\bcommit\s+[0-9a-f]{5,}|\bcommit\s*$)/i) { - for (my $n = 0; $n < 2; $n++) { + for (my $n = 0; $n < 3; $n++) { if ($input =~ /\bcommit\s+[0-9a-f]{5,}\s*($balanced_parens)/i) { $orig_desc = $1; $has_parens = 1; From ac7f4e76ca536734727c4bce6c65f366a4098f5b Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Thu, 10 Aug 2023 11:21:29 +0300 Subject: [PATCH 37/69] Fix CONSTANT_COMPARISON check If the condition has a continuation, for example, static_assert(BOX_SPACE_ID < BOX_INDEX_ID && LvalOrFunc captures a trailing space so we have to take it into account when checking for a macro constant. Another problem with this check is an obvious typo: missing $ before Constant. Closes #65 --- checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkpatch.pl b/checkpatch.pl index 5699dd1..f69363a 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -4134,7 +4134,7 @@ sub process { my $lead = $1; my $to = $2; if ($lead !~ /(?:$Operators|\.)\s*$/ && - $to !~ /^(?:Constant|[A-Z_][A-Z0-9_]*)$/) { + $to !~ /^(?:$Constant|[A-Z_][A-Z0-9_]*)\s*$/) { ERROR("CONSTANT_COMPARISON", "Comparisons should place the constant on the right side of the test\n" . $herecurr); } From 0cd915fcd81da075dd04b2b3ef60b639edcd8d2d Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Mon, 21 Aug 2023 10:54:22 +0300 Subject: [PATCH 38/69] Allow usage of volatile in tests It's often required to simulate long loops. --- checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkpatch.pl b/checkpatch.pl index f69363a..79e4d4e 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -4546,7 +4546,7 @@ sub process { # no volatiles please my $asm_volatile = qr{\b(__asm__|asm)\s+(__volatile__|volatile)\b}; - if ($line =~ /\bvolatile\b/ && $line !~ /$asm_volatile/ && $line !~ /\bsig_atomic_t\b/) { + if (!$is_test && $line =~ /\bvolatile\b/ && $line !~ /$asm_volatile/ && $line !~ /\bsig_atomic_t\b/) { ERROR("VOLATILE", "Use of volatile is usually wrong\n" . $herecurr); } From 8ec1545afe6b53ad3b83e01b6411b47dfd6c6850 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Mon, 21 Aug 2023 16:24:52 +0300 Subject: [PATCH 39/69] Add NO_SANITIZE_ADDRESS attribute --- checkpatch.pl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/checkpatch.pl b/checkpatch.pl index 79e4d4e..c632c54 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -388,6 +388,7 @@ sub hash_show_words { MAYBE_UNUSED| NODISCARD| NORETURN| + NO_SANITIZE_ADDRESS| PACKED }x; our $Modifier; @@ -4793,6 +4794,7 @@ sub process { "nodiscard" => "NODISCARD", "noinline" => "NOINLINE", "noreturn" => "NORETURN", + "no_sanitize_address" => "NO_SANITIZE_ADDRESS", "packed" => "PACKED", "unused" => "MAYBE_UNUSED" ); From 5918761d35eb0bd438fa325496bdbb1f9ac249d1 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Mon, 28 Aug 2023 17:04:22 +0300 Subject: [PATCH 40/69] Check template argument alignment Checks that: - There's no spaces between 'template' and '<'. - Template argument list continuation is properly aligned. Also fixes #66. --- checkpatch.pl | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/checkpatch.pl b/checkpatch.pl index c632c54..6d5ec4c 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2070,6 +2070,7 @@ sub process { my $here = ''; my $context_function; #undef'd unless there's a known function my $context_struct; #undef'd unless there's a known struct + my $in_template_indent; #nddef'd unless in template arguments my $in_comment = 0; my $first_line = 0; my %check_comment_ignore = (); @@ -2206,6 +2207,7 @@ sub process { } else { undef $context_struct; } + undef $in_template_indent; next; # track the line number as we move through the hunk, note that @@ -2944,7 +2946,7 @@ sub process { } # check indentation starts on a tab stop - if ($sline =~ /^\+\t+( +)(?:$c90_Keywords\b|\{\s*$|\}\s*(?:else\b|while\b|\s*$)|$Declare\s*$Ident\s*[;=])/) { + if (!defined($in_template_indent) && $sline =~ /^\+\t+( +)(?:$c90_Keywords\b|\{\s*$|\}\s*(?:else\b|while\b|\s*$)|$Declare\s*$Ident\s*[;=])/) { my $indent = length($1); if ($indent % $tabsize) { ERROR("TABSTOP", @@ -2952,6 +2954,28 @@ sub process { } } +# check template arguments alignment + if (defined($in_template_indent)) { + if ($rawline =~ /^\+(\s*)/) { + my $indent = length(expand_tabs($1)); + if ($indent != $in_template_indent) { + ERROR("CODE_INDENT", + "unaligned template arguments\n" . $hereprev); + } + } + if ($rawline =~ />\s*$/) { + undef $in_template_indent; + } + } elsif ($rawline =~ /^.(\s*)template(\s*) 0) { + ERROR("SPACING", + "space prohibited between 'template' and '<'\n" . $herecurr); + } + if ($rawline !~ />\s*$/) { + $in_template_indent = length(expand_tabs($1)) + length('template<'); + } + } + # check multi-line statement indentation matches previous line if ($prevline =~ /^\+([ \t]*)((?:$c90_Keywords(?:\s+if)\s*)|(?:$Declare\s*)?(?:$Ident|\(\s*\*\s*$Ident\s*\))\s*|(?:(?:\*\s*)*$Lval|$Declare\s*$Ident)\s*=\s*$Ident\s*)\(.*(\&\&|\|\||,|")\s*$/) { $prevline =~ /^\+(\t*)(.*)$/; From 6f039e95c4d9295b15d6ab2a019f64f0f4667084 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Mon, 4 Sep 2023 11:22:47 +0300 Subject: [PATCH 41/69] Use rawline for hashbang lookup The hashbang sign may be sanitized away. --- checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkpatch.pl b/checkpatch.pl index 6d5ec4c..55121e6 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2264,7 +2264,7 @@ sub process { if ($line =~ /^new (file )?mode.*[7531]\d{0,2}$/) { $has_exec_perm = 1 } - if ($realline == 1 and ($line =~ /^\+#!/) != $has_exec_perm) { + if ($realline == 1 and ($rawline =~ /^\+#!/) != $has_exec_perm) { my $permhere = $here . "FILE: $realfile\n"; my $msg = ($has_exec_perm ? "Executable file must have hashbang (#!)" : From 9481f2c7c2082ff20b87d2d02d360f578bc5a4b6 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Wed, 20 Sep 2023 10:43:12 +0300 Subject: [PATCH 42/69] Fix unchecked usage of raw_line result raw_line may return nothing. --- checkpatch.pl | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/checkpatch.pl b/checkpatch.pl index 55121e6..bd6e1fb 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -1583,7 +1583,9 @@ sub get_stat_real { my $stat_real = raw_line($linenr, 0); for (my $count = $linenr + 1; $count <= $lc; $count++) { - $stat_real = $stat_real . "\n" . raw_line($count, 0); + my $rl = raw_line($count, 0); + last if !defined($rl); + $stat_real = $stat_real . "\n" . $rl; } return $stat_real; @@ -1594,7 +1596,9 @@ sub get_stat_here { my $herectx = $here . "\n"; for (my $n = 0; $n < $cnt; $n++) { - $herectx .= raw_line($linenr, $n) . "\n"; + my $rl = raw_line($linenr, $n); + last if !defined($rl); + $herectx .= $rl . "\n"; } return $herectx; @@ -4730,6 +4734,7 @@ sub process { my $cnt = statement_rawlines($stat); for (my $n = 0; $n < $cnt; $n++) { my $rl = raw_line($linenr, $n); + last if !defined($rl); if (!defined($func_body_size)) { $func_body_size = -1 if $rl =~ /\{/; next; @@ -4952,6 +4957,7 @@ sub process { my $herectx = $here . "\n"; for (my $n = 0; $n < $cnt; $n++) { my $rl = raw_line($linenr, $n); + last if !defined($rl); $herectx .= $rl . "\n"; $ok = 1 if ($rl =~ /^[ \+]\s*\{/); # allow opening and closing braces on the same line From 566f34fffd9ea2cf9d6d247e4c56d14330e28f72 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Wed, 20 Sep 2023 11:53:01 +0300 Subject: [PATCH 43/69] Ignore function arguments in UNCOMMENTED_DEFINITION check A function argument may look like a function pointer definition but we shouldn't require comments to it. --- checkpatch.pl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/checkpatch.pl b/checkpatch.pl index bd6e1fb..26f8468 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -4712,6 +4712,8 @@ sub process { # operator() is used as a callback so a comment isn't necessary } elsif ($line =~ /^\+\s*(?:$Declare)?\s*(?:generic_|disabled_|exhausted_)/) { # generic_XXX, disabled_XXX, exhausted_XXX are functions are stubs so comments are not required + } elsif ($prevline =~ /[,(]\s*$/) { + # function argument list } elsif ($line =~ /^\+\s*($Declare)?\s*(?:($Ident)\s*\(|\(\s*\*\s*($Ident)\s*\)\s*\(|($Ident)\s*;)/) { # function, function pointer, variable / struct member my $decl = $1; From 1bca2903e51b75227ac562113b2e0c940356b2a6 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Thu, 28 Sep 2023 17:20:18 +0300 Subject: [PATCH 44/69] Suppress STATIC_CONST_CHAR_ARRAY error for mkdtemp template strings --- checkpatch.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/checkpatch.pl b/checkpatch.pl index 26f8468..2df8b99 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -3592,8 +3592,8 @@ sub process { "const array should probably be static const\n" . $herecurr); } -# check for static char foo[] = "bar" declarations. - if ($line =~ /\bstatic\s+char\s+(\w+)\s*\[\s*\]\s*=\s*"/) { +# check for static char foo[] = "bar" declarations; ignore mkdtemp template + if ($line =~ /\bstatic\s+char\s+(\w+)\s*\[\s*\]\s*=\s*"/ && $line !~ /XXXXXX/) { ERROR("STATIC_CONST_CHAR_ARRAY", "static char array declaration should probably be static const char\n" . $herecurr); From f5a07bca272c17b323115291b30e6b6ef8686be8 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Thu, 5 Oct 2023 13:43:17 +0300 Subject: [PATCH 45/69] Suppress COMMIT_LOG_LONG_LINE error for code sections --- checkpatch.pl | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/checkpatch.pl b/checkpatch.pl index 2df8b99..cc947ed 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2053,6 +2053,7 @@ sub process { my $commit_log_lines = 0; #Number of commit log lines my $commit_log_long_line = 0; my $commit_log_no_wrap = 0; + my $commit_log_code_section = 0; my $commit_log_has_diff = 0; my $non_utf8_charset = 0; my $has_exec_perm = 0; #Current file has exec permissions @@ -2499,9 +2500,12 @@ sub process { if ($in_commit_log && $line =~ /^NO_WRAP$/) { $commit_log_no_wrap = !$commit_log_no_wrap; } + if ($in_commit_log && $line =~ /^```/) { + $commit_log_code_section = !$commit_log_code_section; + } # Check for line lengths > 75 in commit log, warn once - if ($in_commit_log && !$commit_log_no_wrap && !$commit_log_long_line && + if ($in_commit_log && !$commit_log_no_wrap && !$commit_log_code_section && !$commit_log_long_line && length($line) > 75 && !($line =~ /^\s*[a-zA-Z0-9_\/\.]+\s+\|\s+\d+/ || # file delta changes @@ -5197,6 +5201,10 @@ sub process { ERROR("UNTERMINATED_TAG", "Unterminated NO_WRAP section in commit description\n") } + if ($commit_log_code_section) { + ERROR("UNTERMINTATED_CODE_SECTION", + "Untermintated code section (```) in commit description\n"); + } if (!$has_doc && !exists($commit_log_tags{'NO_DOC'})) { ERROR("NO_DOC", "Please add doc or NO_DOC= tag\n"); From a1750965bfb7479c4195f767bd785598fa7e867c Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Mon, 16 Oct 2023 11:07:01 +0300 Subject: [PATCH 46/69] Fix FP spacing error for () --- checkpatch.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/checkpatch.pl b/checkpatch.pl index cc947ed..d645a09 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -3080,13 +3080,13 @@ sub process { # check for space after cast like "(int) foo" or "(struct foo) bar" # avoid checking a few false positives: -# "sizeof()" or "__alignof__()" +# "()" # function pointer declarations like "(*foo)(int) = bar;" # structure definitions like "(struct foo) { 0 };" # multiline macros that define functions # known attributes or the __attribute__ keyword if ($line =~ /^\+(.*)\(\s*$Type\s*\)([ \t]++)((?![={]|\\$|$Attribute|__attribute__))/ && - (!defined($1) || $1 !~ /\b(?:sizeof|__alignof__)\s*$/)) { + (!defined($1) || $1 !~ /\b$Ident\s*$/)) { ERROR("SPACING", "No space is necessary after a cast\n" . $herecurr); } From 647cc14112a1ac803e57379420b641d24f2e8cca Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Fri, 20 Oct 2023 13:28:53 +0300 Subject: [PATCH 47/69] Add ALWAYS_INLINE attribute --- checkpatch.pl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/checkpatch.pl b/checkpatch.pl index d645a09..5b84a36 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -381,6 +381,7 @@ sub hash_show_words { const| volatile| alignas| + ALWAYS_INLINE| API_EXPORT| CFORMAT| DEPREACTED| @@ -4823,6 +4824,7 @@ sub process { my %attr_list = ( "aligned" => "alignas", + "always_inline" => "ALWAYS_INLINE", "format" => "CFORMAT", "deprecated" => "DEPREACTED", "fallthrough" => "FALLTHROUGH", From 5a3d42716e56465d1084e2718b623aa9ae6cd073 Mon Sep 17 00:00:00 2001 From: Nikolay Shirokovskiy Date: Wed, 13 Dec 2023 11:21:29 +0300 Subject: [PATCH 48/69] Allow different indent after comment for the block end Currently the check fails next sane case. We should allow misindent for the case of block end. ```diff */ + } catch (FiberIsCancelled *e) { ``` --- checkpatch.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/checkpatch.pl b/checkpatch.pl index 5b84a36..9b718bf 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2919,10 +2919,10 @@ sub process { "code indent shouldn't use spaces if the previous line ends with $what\n" . $herevet); } -# alignment should match the comment on the previous line unless it's the end of a function or struct +# alignment should match the comment on the previous line unless it's the end of block if ($prevrawline =~ /^.(\s*)(?:\/\*.*| )\*\/\s*$/) { my $ident = length($1); - if ($rawline =~ /^\+(\s*)[^\}]/ && length($1) != $ident) { + if ($rawline =~ /^\+(\s*)[^}\s]/ && length($1) != $ident) { ERROR("CODE_IDENT", "code indent should match comment\n" . $hereprev); } From 84ec80fe25fd44c68f4930b7d6e7a0acdc2c1238 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Fri, 29 Dec 2023 13:08:11 +0300 Subject: [PATCH 49/69] Detect C++ style types Detect CamelCase with an optional :: separator as a type. Closes #68 --- checkpatch.pl | 1 + 1 file changed, 1 insertion(+) diff --git a/checkpatch.pl b/checkpatch.pl index 9b718bf..4921c43 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -595,6 +595,7 @@ sub find_standard_signature { qr{${Ident}_f}, qr{${Ident}_fn}, qr{${Ident}_cb}, + qr{(?:::)?(?:[A-Z][a-z][A-Za-z\d]*::)*[A-Z][a-z][A-Za-z\d]*}, @typeListMisordered, ); From 1f56b850bec6f8c4c7c6f5b6db0b7a106c1d1093 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Thu, 11 Jan 2024 18:15:17 +0300 Subject: [PATCH 50/69] Ignore unaligned comments used for logically separating code parts We have plenty of those in memtx mvcc. --- checkpatch.pl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/checkpatch.pl b/checkpatch.pl index 4921c43..58fb2e2 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2921,7 +2921,9 @@ sub process { } # alignment should match the comment on the previous line unless it's the end of block - if ($prevrawline =~ /^.(\s*)(?:\/\*.*| )\*\/\s*$/) { +# or a special comment that starts with '/****' used to separate logical code parts + if ($prevrawline !~ /^.\/\*\*\*\*/ && + $prevrawline =~ /^.(\s*)(?:\/\*.*| )\*\/\s*$/) { my $ident = length($1); if ($rawline =~ /^\+(\s*)[^}\s]/ && length($1) != $ident) { ERROR("CODE_IDENT", From fe7e50aebb53f194091c856018006dab7a9a2c28 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Fri, 12 Jan 2024 13:12:58 +0300 Subject: [PATCH 51/69] Don't complain about template argument spacing for unchanged lines --- checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkpatch.pl b/checkpatch.pl index 58fb2e2..50a3ec2 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2978,7 +2978,7 @@ sub process { if ($rawline =~ />\s*$/) { undef $in_template_indent; } - } elsif ($rawline =~ /^.(\s*)template(\s*) 0) { ERROR("SPACING", "space prohibited between 'template' and '<'\n" . $herecurr); From 1b1f844f475d4079426932ba09fcc7ad0c7e2165 Mon Sep 17 00:00:00 2001 From: Ilya Verbin Date: Fri, 12 Jan 2024 17:14:41 +0300 Subject: [PATCH 52/69] Improve LONG_LINE and SPACING errors for array initializers Ignore the errors for array initializers that contain numbers, e.g.: static bool my_array[] = { /* 01 02 03 */ --- checkpatch.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/checkpatch.pl b/checkpatch.pl index 50a3ec2..762bca4 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2871,8 +2871,8 @@ sub process { } elsif ($line =~ /^\+.*\\$/ && $length == $max_line_length + 1) { $msg_type = ""; # Comment which is used as a header in array initializer - # /* FOO BAR BAZ */ - } elsif ($rawline =~ /^.\s*\/\*[a-zA-Z_\s]+\*\/\s*$/) { + # /* FOO BAR 12 34 */ + } elsif ($rawline =~ /^.\s*\/\*[0-9a-zA-Z_\s]+\*\/\s*$/) { $msg_type = ""; # Otherwise set the alternate message types From c39d7d73d21ce5daa4ea63aef26f14fed27d6817 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Wed, 31 Jan 2024 13:15:21 +0300 Subject: [PATCH 53/69] Ignore lines ending with backslash for UNCOMMENTED_DEFINITION check So as not to warn about uncommented lines in multi-line macros. --- checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkpatch.pl b/checkpatch.pl index 762bca4..da20022 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -4722,7 +4722,7 @@ sub process { # generic_XXX, disabled_XXX, exhausted_XXX are functions are stubs so comments are not required } elsif ($prevline =~ /[,(]\s*$/) { # function argument list - } elsif ($line =~ /^\+\s*($Declare)?\s*(?:($Ident)\s*\(|\(\s*\*\s*($Ident)\s*\)\s*\(|($Ident)\s*;)/) { + } elsif ($line =~ /^\+\s*($Declare)?\s*(?:($Ident)\s*\(|\(\s*\*\s*($Ident)\s*\)\s*\(|($Ident)\s*;)\s*$/) { # function, function pointer, variable / struct member my $decl = $1; my $is_func = defined($2); From 99cbe1d30dd9b7ad875019f6d62d5783df7f0b7d Mon Sep 17 00:00:00 2001 From: Magomed Kostoev Date: Tue, 14 May 2024 11:52:23 +0300 Subject: [PATCH 54/69] Don't check class and namespace name macro args for precedence issues If we see an argument is used with the `::` operator, that means it's either class or namespace name. There's no sense to check the argument for precedence issues in this case so let's skip it. --- checkpatch.pl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/checkpatch.pl b/checkpatch.pl index da20022..e74003c 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -4421,8 +4421,9 @@ sub process { # check if any macro arguments are reused (ignore '...' and 'type') foreach my $arg (@def_args) { - next if ($arg =~ /\.\.\./); - next if ($arg =~ /^type$/i); + next if ($arg =~ /\.\.\./); # Skip the ellipsis. + next if ($arg =~ /^type$/i); # Skip the argument with a special name. + next if ($define_stmt =~ /\b$arg\s*::/); # Skip the argument if it's used as a scope name (it's a class or a namespace name). my $tmp_stmt = $define_stmt; $tmp_stmt =~ s/\b(__must_be_array|offsetof|sizeof|sizeof_field|__stringify|typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*\)*\b//g; $tmp_stmt =~ s/\#+\s*$arg\b//g; From 83d077d0016e7308ddaa9c63df6b9862318d0eca Mon Sep 17 00:00:00 2001 From: Magomed Kostoev Date: Thu, 16 May 2024 14:16:04 +0300 Subject: [PATCH 55/69] Allow passing empty last argument to macros In C it's posssible to pass empty token sequences to a macro, for example: `F(a, b,,)`, the third and fourth arguments of the F macro invokation are empty token sequences. Let's make the checkpatch understand having the last macro argument empty (the `,)` sequence). This by the way allows `,]` and `,;` but these are illegal in C anyways. --- checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkpatch.pl b/checkpatch.pl index e74003c..ce71fc6 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -3923,7 +3923,7 @@ sub process { ERROR("SPACING", "space prohibited before that '$op' $at\n" . $hereptr); } - if ($ctx !~ /.x[WEC]/ && $cc !~ /^}/) { + if ($ctx !~ /.x[WECB]/ && $cc !~ /^}/) { ERROR("SPACING", "space required after that '$op' $at\n" . $hereptr); } From b21b8a242be58e0e1f0564bb27768d39bee7fcda Mon Sep 17 00:00:00 2001 From: Magomed Kostoev Date: Thu, 16 May 2024 14:09:32 +0300 Subject: [PATCH 56/69] Don't treat calls of functions from a scope as declarations In C++ it's illegal to declare a function using a syntax involving the scope access (`::`) operator. Nevertheless, the checkpatch identifies the `class::method(a, &b)`, being a function call, as a declaration of a function with nameless arguments. The problem is that the checkpatch recognizes a sequence of identifiers and preprocessor concatenation (`##`) with namespace access (`::`) as a single identifier. For example: `class_name##_base::method`. If the sequence only had a `##` operator, it could be considered as a single identifier, but since it contains the `::` operator, it can't, so the pattern detecting function declrations becomes invalid. In order to not break other existing rules that could rely on the fact the namespace access is a part of an identifier, this patch only fixes the function declaration recognition by introducing a new pattern for sequences of identifiers with preprocessor concatenation operators but without namespace accesses and using it. --- checkpatch.pl | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/checkpatch.pl b/checkpatch.pl index ce71fc6..0bf01d7 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -370,11 +370,14 @@ sub hash_show_words { my $emitted_corrupt = 0; -our $Ident = qr{ - (?:::)?(?:[A-Za-z_][A-Za-z\d_]*::)* +our $IdentNoNs = qr{ [A-Za-z_][A-Za-z\d_]* (?:\s*\#\#\s*[A-Za-z_][A-Za-z\d_]*)* }x; +our $Ident = qr{ + (?:::)?(?:[A-Za-z_][A-Za-z\d_]*::)* + $IdentNoNs + }x; our $Storage = qr{extern|static}; our $Attribute = qr{ typename| @@ -4946,7 +4949,7 @@ sub process { # check for function declarations that have arguments without identifier names # suppress the check for expressions in function context to avoid FP error for class object definition if (defined $stat && !defined $context_function && - $stat =~ /^.\s*(?:extern\s+)?$Type\s*(?:$Ident|\(\s*\*\s*$Ident\s*\))\s*\(\s*([^{]+)\s*\)\s*;/s && + $stat =~ /^.\s*(?:extern\s+)?$Type\s*(?:$IdentNoNs|\(\s*\*\s*$IdentNoNs\s*\))\s*\(\s*([^{]+)\s*\)\s*;/s && $1 ne "void") { my $args = trim($1); while ($args =~ m/\s*($Type\s*(?:$Ident|\(\s*\*\s*$Ident?\s*\)\s*$balanced_parens)?)/g) { From 8095875a81444b3ea853f9862f353933c874f988 Mon Sep 17 00:00:00 2001 From: Magomed Kostoev Date: Mon, 24 Jun 2024 20:30:24 +0300 Subject: [PATCH 57/69] Allow defining template functions in macros Such structure is used in the memtx tree implementation in order to automatically create iterator advancing wrappers: ```C++ template \ static int \ name(struct iterator *iterator, struct tuple **ret) \ { \ do { \ int rc = name##_base(iterator, ret); \ if (rc != 0 || \ iterator->next_internal == exhausted_iterator_next) \ return rc; \ } while (*ret == NULL); \ return 0; \ } \ ``` But the checkpatch does not recognize the end of template parameter list since it's not folowed by a newline, so it assumes the following lines of the macro are next template parameters (and they're misaligned). This patch fixes this by making the checkpatch aware that the line a template is defined on can end on a backslash character. --- checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkpatch.pl b/checkpatch.pl index 0bf01d7..8ef0e66 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2986,7 +2986,7 @@ sub process { ERROR("SPACING", "space prohibited between 'template' and '<'\n" . $herecurr); } - if ($rawline !~ />\s*$/) { + if ($rawline !~ />\s*\\?$/) { $in_template_indent = length(expand_tabs($1)) + length('template<'); } } From 9326ed5ecd1196add0e5e86a92c96f13713a6573 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Fri, 26 Jul 2024 17:18:06 +0300 Subject: [PATCH 58/69] Improve parenthesis alignment check Make the PARENTHESIS_ALIGNMENT check work in the following cases: 1. Returnting a function call. ```C return foo(a, b, d, d); ``` 2. With non-logical continuations. ```C if (a != b) return 0; if (a != b) return 0; ``` --- checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkpatch.pl b/checkpatch.pl index 8ef0e66..af2de7f 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2992,7 +2992,7 @@ sub process { } # check multi-line statement indentation matches previous line - if ($prevline =~ /^\+([ \t]*)((?:$c90_Keywords(?:\s+if)\s*)|(?:$Declare\s*)?(?:$Ident|\(\s*\*\s*$Ident\s*\))\s*|(?:(?:\*\s*)*$Lval|$Declare\s*$Ident)\s*=\s*$Ident\s*)\(.*(\&\&|\|\||,|")\s*$/) { + if ($prevline =~ /^\+([ \t]*)((?:$c90_Keywords(?:\s+if)\s*)|(?:$Declare\s*|return\s*)?(?:$Ident|\(\s*\*\s*$Ident\s*\))\s*|(?:(?:\*\s*)*$Lval|$Declare\s*$Ident)\s*=\s*$Ident\s*)\(.*($Operators|"|\))\s*$/) { $prevline =~ /^\+(\t*)(.*)$/; my $oldindent = $1; my $rest = $2; From a6701e1adeed568add738374a2a876285c150cae Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Sat, 27 Jul 2024 01:29:51 +0300 Subject: [PATCH 59/69] Ignore files marked as generated in .gitattributes See [1] for the `linguist-generated=true` attribute description. [1]: https://docs.github.com/en/repositories/working-with-files/managing-files/customizing-how-changed-files-appear-on-github --- checkpatch.pl | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/checkpatch.pl b/checkpatch.pl index af2de7f..037ec44 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -455,6 +455,8 @@ sub hash_show_words { src\/lib\/tzcode\/ )}; +our $generatedFiles; + our $typeMacros = qr{(?x: LIGHT )}; @@ -890,6 +892,26 @@ sub git_commit_info { @ARGV = @commits; } +# Collect files marked as generated in .gitattributes +if ($git && $gitroot) { + if (open(my $gitattributes, '<', "$gitroot/../.gitattributes")) { + my @acc = (); + while (<$gitattributes>) { + my $line = $_; + # https://docs.github.com/en/repositories/working-with-files/managing-files/customizing-how-changed-files-appear-on-github + next if ($line !~ /linguist-generated=true/); + $line =~ s/ +[^ ]+$//; + # https://www.git-scm.com/docs/gitignore#_pattern_format + # + # Assume that the file pattern is just a file path or + # a directory path in the repository. + push(@acc, "^\Q$line\E"); + } + close($gitattributes); + $generatedFiles = join("|", @acc); + } +} + my $vname; for my $filename (@ARGV) { my $FILE; @@ -2646,6 +2668,9 @@ sub process { "8-bit UTF-8 used in possible commit log\n" . $herecurr); } +# Skip files marked as generated in .gitattributes + next if $realfile ne '' && defined($generatedFiles) && $realfile =~ $generatedFiles; + # Check for various typo / spelling mistakes if (defined($misspellings) && ($in_commit_log || $line =~ /^(?:\+|Subject:)/i)) { From 2770f41d32c9a6f30c6315df61e471a1a67c5ca4 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Mon, 5 Aug 2024 17:51:14 +0300 Subject: [PATCH 60/69] Fix bug when all files are skipped if generatedFiles is empty The variable generatedFiles is always defined but it may be empty. Currently, if it's empty we skip checks for all files, which is wrong. Fix this by checking if it's empty instead of if it's defined. Fixes commit a6701e1adeed ("Ignore files marked as generated in .gitattributes"). --- checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkpatch.pl b/checkpatch.pl index 037ec44..1f78660 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2669,7 +2669,7 @@ sub process { } # Skip files marked as generated in .gitattributes - next if $realfile ne '' && defined($generatedFiles) && $realfile =~ $generatedFiles; + next if $realfile ne '' && $generatedFiles ne '' && $realfile =~ $generatedFiles; # Check for various typo / spelling mistakes if (defined($misspellings) && From 5e43984687b641df8a03ad361b0b5b1108b1b8af Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Mon, 2 Sep 2024 23:24:47 +0300 Subject: [PATCH 61/69] Fix use of undef value if no .gitattributes file The `$generatedFiles` variable is uninitialized but compared with an empty string if the `.gitattributes` file is missing. Let's initialize the variable with an empty string, so missing `.gitattributes` file is interpreted in the same way as no `linguist-generated=true` attributed items in an existing `.gitattributes` file. Fixes 2770f41d32c9 ("Fix bug when all files are skipped if generatedFiles is empty"). Part of #76 --- checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkpatch.pl b/checkpatch.pl index 1f78660..ef5b28e 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -455,7 +455,7 @@ sub hash_show_words { src\/lib\/tzcode\/ )}; -our $generatedFiles; +our $generatedFiles = ''; our $typeMacros = qr{(?x: LIGHT From 50348499d23fe7639e1b5b85e182e6537e360bd2 Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Tue, 3 Sep 2024 00:09:18 +0300 Subject: [PATCH 62/69] Fix generated files check in a git submodule A git submodule contains a `.git` file, not a directory with this name. So, `open(<...>, ".git/../.gitattributes")` fails with the `ENOTDIR` errno code. This commit fixes the repository root calculation, so `.gitattributes` file is found in the given case. Fixes #76 --- checkpatch.pl | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/checkpatch.pl b/checkpatch.pl index ef5b28e..cc0a8b6 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -15,6 +15,7 @@ use Cwd 'abs_path'; use Term::ANSIColor qw(:constants); use Encode qw(decode encode); +use File::Spec::Functions 'catfile'; my $P = $0; my $D = dirname(abs_path($P)); @@ -894,7 +895,13 @@ sub git_commit_info { # Collect files marked as generated in .gitattributes if ($git && $gitroot) { - if (open(my $gitattributes, '<', "$gitroot/../.gitattributes")) { + # $gitroot points to a .git directory/file inside a repository + # root. Calculate its parent directory -- the repository root. + # + # TODO: Should we handle a bare git repository somehow? + my $repo_root = dirname(abs_path($gitroot)); + + if (open(my $gitattributes, '<', catfile($repo_root, ".gitattributes"))) { my @acc = (); while (<$gitattributes>) { my $line = $_; From f7d74a44a49db17e7179be11030e9ebf45dd9f7d Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Fri, 6 Sep 2024 10:49:26 +0300 Subject: [PATCH 63/69] Warn about commit log tags in doc request Tags defined in the doc request will slip into the doc ticket. Let's ask to move them before the doc request. --- checkpatch.pl | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/checkpatch.pl b/checkpatch.pl index cc0a8b6..abfe7f6 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2721,6 +2721,10 @@ sub process { if ($in_commit_log && $line =~ /^($custom_tags)=/) { $commit_log_tags{$1} = 1; + if ($has_doc) { + ERROR("TAG_IN_DOC", + "Please move $1 tag before doc request\n"); + } } if ($in_commit_log && $line =~ /^\@TarantoolBot document$/) { $has_doc = 1; From c61eb2e9191f58f53b3a667aff07e1d0cfb3118e Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Mon, 9 Sep 2024 17:40:08 +0300 Subject: [PATCH 64/69] Skip all checks for third party files Third-party files may not follow any of our rules, even those that seem sane for any text file. Let's skip them all just like we skip generated files. --- checkpatch.pl | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/checkpatch.pl b/checkpatch.pl index abfe7f6..39d94cd 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -448,14 +448,11 @@ sub hash_show_words { # Skip all checks for the following paths. our $skipPaths = qr{(?x: + src\/lib\/tzcode\/| + third_party\/| test\/static\/corpus\/ )}; -# Skip C/C++ source code checks for the following paths. -our $skipSrcPaths = qr{(?x: - src\/lib\/tzcode\/ -)}; - our $generatedFiles = ''; our $typeMacros = qr{(?x: @@ -2636,8 +2633,6 @@ sub process { $herecurr) if (!$emitted_corrupt++); } - next if !$file and $realfile =~ /^$skipPaths/; - # UTF-8 regex found at http://www.w3.org/International/questions/qa-forms-utf-8.en.php if (($realfile =~ /^$/ || $line =~ /^\+/) && $rawline !~ m/^$UTF8*$/) { @@ -2675,6 +2670,8 @@ sub process { "8-bit UTF-8 used in possible commit log\n" . $herecurr); } + next if $realfile ne '' && $realfile =~ /^$skipPaths/; + # Skip files marked as generated in .gitattributes next if $realfile ne '' && $generatedFiles ne '' && $realfile =~ $generatedFiles; @@ -2854,12 +2851,6 @@ sub process { "Please avoid new tests with .result files\n"); } -# check we are in a valid source file if not then ignore this hunk - next if ($realfile !~ /\.(h|c|cc|lua)$/ || $realfile =~ /\.test\.lua$/); - -# ignore source files outside the source and test directories when checking patches - next if !$file and ($realfile !~ /^(?:src|test|perf)\// || $realfile =~ /^$skipSrcPaths/); - # line length limit (with some exclusions) # # There are a few types of lines that may extend beyond $max_line_length: @@ -2878,7 +2869,8 @@ sub process { # if LONG_LINE is ignored, the other 2 types are also ignored # - if ($line =~ /^\+/ && $length > $max_line_length) { + if ($realfile =~ /\.(h|c|cc|lua)$/ && $realfile !~ /\.test\.lua$/ && + $line =~ /^\+/ && $length > $max_line_length) { my $msg_type = "LONG_LINE"; # Check the allowed long line types first From 3003a1d9bb736489ed234880911555783f9dbfcd Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Mon, 14 Oct 2024 10:53:14 +0300 Subject: [PATCH 65/69] Disable REPEATED_WORD check in commit log code section Closes #78 --- checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkpatch.pl b/checkpatch.pl index 39d94cd..84af866 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -2737,7 +2737,7 @@ sub process { # check for repeated words separated by a single space # avoid false positive from list command eg, '-rw-r--r-- 1 root root' - if (($rawline =~ /^\+/ || $in_commit_log) && + if (($rawline =~ /^\+/ || ($in_commit_log && !$commit_log_code_section)) && $rawline !~ /[bcCdDlMnpPs\?-][rwxsStT-]{9}/) { pos($rawline) = 1 if (!$in_commit_log); while ($rawline =~ /\b($word_pattern) (?=($word_pattern))/g) { From f302b42f9b6e6f8799e63a495a84e45dffc8fd27 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Wed, 16 Oct 2024 14:03:31 +0300 Subject: [PATCH 66/69] Fix C++ type detection A C++ type can't be followed by `::`, e.g. we should never match `Foo` in `Foo::Bar` as a type. Fixes commit 84ec80fe25fd ("Detect C++ style types"). Closes #79 --- checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkpatch.pl b/checkpatch.pl index 84af866..06f74f8 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -598,7 +598,7 @@ sub find_standard_signature { qr{${Ident}_f}, qr{${Ident}_fn}, qr{${Ident}_cb}, - qr{(?:::)?(?:[A-Z][a-z][A-Za-z\d]*::)*[A-Z][a-z][A-Za-z\d]*}, + qr{(?:::)?(?:[A-Z][a-z][A-Za-z\d]*::)*[A-Z][a-z][A-Za-z\d]*(?!::)}, @typeListMisordered, ); From 0f2efb9249d98f1ab09ff19adce230c44c5e0ec0 Mon Sep 17 00:00:00 2001 From: Georgiy Belyanin Date: Thu, 21 Nov 2024 18:38:25 +0300 Subject: [PATCH 67/69] Error on issue reference in doc requests Issues can be referenced after @TarantoolBot doc request. Though it will be parsed as a part of the documentation request which is wrong. This patch makes the checkpatch issue an error if there is an issue reference in the doc request body. Only the most popular constructions are handled (e.g. "closes", "part of" etc.) to avoid false-positives. Examples of handled errors: ``` @TarantoolBot document Closes #234 Follow-up #234 Follows-up: #234 EE-Issue #234 Close #999 Close #999, #123, #9900 Close http://github.com/reference/to_issue Close tarantool/tarantool-ee#234 ``` These aren't handled (referencing commits and uncommon constructions): ``` @TarantoolBot document Follows up the commit 1e94aeff Closes the issue #1234 Repairs #1234 ``` --- checkpatch.pl | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/checkpatch.pl b/checkpatch.pl index 06f74f8..cda572e 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -500,6 +500,36 @@ sub hash_show_words { NO_CHANGELOG )}; +our $github_ref = qr{(https?:\/\/)?(www\.)?github\.com\b([-a-zA-Z0-9()@:%_\+.~\#?&\/=]*)}; +our $github_issue_ref = qr{((tarantool\/\S+)?\#[1-9][0-9]*)}; + +our $github_issue_markers = qr{(?x: + Close| + Closes| + Part\s+of| + Part-of| + Issue| + Follow\s+up| + Follows\s+up| + Follow-up| + Follows-up| + Needed\s+for| + Needed-for| + Fix| + Fixes| + Fixed| + See| + See\s+also| + See-also| + Related\s+to| + Related-to| + Related| + Relates| + Workaround| + Implement| + Implements +)}; + sub edit_distance_min { my (@arr) = @_; my $len = scalar @arr; @@ -2723,6 +2753,10 @@ sub process { "Please move $1 tag before doc request\n"); } } + if ($in_commit_log && $has_doc && $line =~ /^($github_issue_markers)\s*:?\s*($github_issue_ref|$github_ref)/) { + ERROR("ISSUE_IN_DOC", + "Please move \"$line\" issue reference before doc request\n"); + } if ($in_commit_log && $line =~ /^\@TarantoolBot document$/) { $has_doc = 1; } From 3d1a4e983b5123e317482c0964fb0c901e7527a4 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Fri, 11 Apr 2025 17:09:43 +0300 Subject: [PATCH 68/69] Fix UNCOMMENTED_DEFINITION check For an unknown reason, no line matches the regexp used for detecting functions, function pointers, and variable / struct members because of `$` at the end. Let's remove `$` and add one more condition for filtering out macros (lines ending with a backslash). Fixes commit c39d7d73d21c ("Ignore lines ending with backslash for UNCOMMENTED_DEFINITION check"). --- checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkpatch.pl b/checkpatch.pl index cda572e..e7e5599 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -4788,7 +4788,7 @@ sub process { # generic_XXX, disabled_XXX, exhausted_XXX are functions are stubs so comments are not required } elsif ($prevline =~ /[,(]\s*$/) { # function argument list - } elsif ($line =~ /^\+\s*($Declare)?\s*(?:($Ident)\s*\(|\(\s*\*\s*($Ident)\s*\)\s*\(|($Ident)\s*;)\s*$/) { + } elsif ($line =~ /^\+\s*($Declare)?\s*(?:($Ident)\s*\(|\(\s*\*\s*($Ident)\s*\)\s*\(|($Ident)\s*;)/ && $line !~ /\\\s*$/) { # function, function pointer, variable / struct member my $decl = $1; my $is_func = defined($2); From c5a5294e2700daaf269c38d436af986f9a7673df Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Mon, 23 Jun 2025 18:04:03 +0300 Subject: [PATCH 69/69] Treat auto as a type It is a type in C++. Let's treat it as such to avoid false-positive spacing errors when processing C++ source files. --- checkpatch.pl | 1 + 1 file changed, 1 insertion(+) diff --git a/checkpatch.pl b/checkpatch.pl index e7e5599..e2049ac 100755 --- a/checkpatch.pl +++ b/checkpatch.pl @@ -608,6 +608,7 @@ sub find_standard_signature { ); our @typeList = ( + qr{auto}, qr{void}, qr{(?:(?:un)?signed\s+)?char}, qr{(?:(?:un)?signed\s+)?short\s+int},