Commit 1574a29f8e76 ("checkpatch: allow multiple const * types")
claims to support repetition of pattern "const *", but it actually
allows only one extra instance.
Check the following lines
int a(char const * const x[]);
int b(char const * const *x);
int c(char const * const * const x[]);
int d(char const * const * const *x);
with command
./scripts/checkpatch.pl --show-types -f filename
to find that only the first line passes the test, while a warning
is triggered by the other 3 lines:
WARNING:FUNCTION_ARGUMENTS: function definition argument
'char const * const' should also have an identifier name
The reason is that the pattern match halts at the second asterisk
in the line, thus the remaining text starting with asterisk fails
to match a valid name for a variable.
Fixed by replacing "?" (Match 1 or 0 times) with "*" (Match 0 or
more times) in the regular expression.
Fix also the similar test for types in unusual order.
Signed-off-by: Antonio Borneo <[email protected]>
Fixes: 1574a29f8e76 ("checkpatch: allow multiple const * types")
---
scripts/checkpatch.pl | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index a09333fd7cef..f40d4bb2fbb9 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -802,12 +802,12 @@ sub build_types {
}x;
$Type = qr{
$NonptrType
- (?:(?:\s|\*|\[\])+\s*const|(?:\s|\*\s*(?:const\s*)?|\[\])+|(?:\s*\[\s*\])+)?
+ (?:(?:\s|\*|\[\])+\s*const|(?:\s|\*\s*(?:const\s*)?|\[\])+|(?:\s*\[\s*\])+)*
(?:\s+$Inline|\s+$Modifier)*
}x;
$TypeMisordered = qr{
$NonptrTypeMisordered
- (?:(?:\s|\*|\[\])+\s*const|(?:\s|\*\s*(?:const\s*)?|\[\])+|(?:\s*\[\s*\])+)?
+ (?:(?:\s|\*|\[\])+\s*const|(?:\s|\*\s*(?:const\s*)?|\[\])+|(?:\s*\[\s*\])+)*
(?:\s+$Inline|\s+$Modifier)*
}x;
$Declare = qr{(?:$Storage\s+(?:$Inline\s+)?)?$Type};
--
2.21.0
The warning LINE_CONTINUATIONS does not offer a --fix.
Add the trivial --fix.
In case of consecutive lines with the same issue, this will
fix only the first line.
Signed-off-by: Antonio Borneo <[email protected]>
---
scripts/checkpatch.pl | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index f40d4bb2fbb9..9a247183b65c 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5207,8 +5207,11 @@ sub process {
$line !~ /^\+\s*\#.*\\$/ && # preprocessor
$line !~ /^\+.*\b(__asm__|asm)\b.*\\$/ && # asm
$line =~ /^\+.*\\$/) {
- WARN("LINE_CONTINUATIONS",
- "Avoid unnecessary line continuations\n" . $herecurr);
+ if (WARN("LINE_CONTINUATIONS",
+ "Avoid unnecessary line continuations\n" . $herecurr) &&
+ $fix) {
+ $fixed[$fixlinenr] =~ s/\s*\\$//;
+ }
}
}
--
2.21.0
Fix spelling of "concatenation".
Don't use tab after space in indentation.
Signed-off-by: Antonio Borneo <[email protected]>
---
scripts/checkpatch.pl | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9a247183b65c..373ad345f732 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -4546,7 +4546,7 @@ sub process {
($op eq '>' &&
$ca =~ /<\S+\@\S+$/))
{
- $ok = 1;
+ $ok = 1;
}
# for asm volatile statements
@@ -4881,7 +4881,7 @@ sub process {
# conditional.
substr($s, 0, length($c), '');
$s =~ s/\n.*//g;
- $s =~ s/$;//g; # Remove any comments
+ $s =~ s/$;//g; # Remove any comments
if (length($c) && $s !~ /^\s*{?\s*\\*\s*$/ &&
$c !~ /}\s*while\s*/)
{
@@ -4920,7 +4920,7 @@ sub process {
# if and else should not have general statements after it
if ($line =~ /^.\s*(?:}\s*)?else\b(.*)/) {
my $s = $1;
- $s =~ s/$;//g; # Remove any comments
+ $s =~ s/$;//g; # Remove any comments
if ($s !~ /^\s*(?:\sif|(?:{|)\s*\\?\s*$)/) {
ERROR("TRAILING_STATEMENTS",
"trailing statements should be on next line\n" . $herecurr);
@@ -5095,7 +5095,7 @@ sub process {
{
}
- # Flatten any obvious string concatentation.
+ # Flatten any obvious string concatenation.
while ($dstat =~ s/($String)\s*$Ident/$1/ ||
$dstat =~ s/$Ident\s*($String)/$1/)
{
--
2.21.0
On Wed, 2019-05-08 at 14:27 +0200, Antonio Borneo wrote:
> The warning LINE_CONTINUATIONS does not offer a --fix.
>
> Add the trivial --fix.
> In case of consecutive lines with the same issue, this will
> fix only the first line.
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -5207,8 +5207,11 @@ sub process {
> $line !~ /^\+\s*\#.*\\$/ && # preprocessor
> $line !~ /^\+.*\b(__asm__|asm)\b.*\\$/ && # asm
> $line =~ /^\+.*\\$/) {
> - WARN("LINE_CONTINUATIONS",
> - "Avoid unnecessary line continuations\n" . $herecurr);
> + if (WARN("LINE_CONTINUATIONS",
> + "Avoid unnecessary line continuations\n" . $herecurr) &&
I prefer to not apply this.
Problem here is that these should generally be fixed by hand
as the automated fix isn't very good style either.
> + $fix) {
> + $fixed[$fixlinenr] =~ s/\s*\\$//;
> + }
> }
> }
>
On Wed, 2019-05-08 at 14:27 +0200, Antonio Borneo wrote:
> Commit 1574a29f8e76 ("checkpatch: allow multiple const * types")
> claims to support repetition of pattern "const *", but it actually
> allows only one extra instance.
> Check the following lines
> int a(char const * const x[]);
> int b(char const * const *x);
> int c(char const * const * const x[]);
> int d(char const * const * const *x);
> with command
> ./scripts/checkpatch.pl --show-types -f filename
> to find that only the first line passes the test, while a warning
> is triggered by the other 3 lines:
> WARNING:FUNCTION_ARGUMENTS: function definition argument
> 'char const * const' should also have an identifier name
> The reason is that the pattern match halts at the second asterisk
> in the line, thus the remaining text starting with asterisk fails
> to match a valid name for a variable.
>
> Fixed by replacing "?" (Match 1 or 0 times) with "*" (Match 0 or
> more times) in the regular expression.
> Fix also the similar test for types in unusual order.
It might be better to use a max match like {0,4} instead of *
perl is pretty memory intensive at multiple unrestricted matches
of somewhat complex patterns.
On Wed, May 8, 2019 at 4:51 PM Joe Perches <[email protected]> wrote:
...
> It might be better to use a max match like {0,4} instead of *
>
> perl is pretty memory intensive at multiple unrestricted matches
> of somewhat complex patterns.
Agree! Will send a V2!
Thanks for the review!
Antonio
Fix spelling of "concatenation".
Don't use tab after space in indentation.
Signed-off-by: Antonio Borneo <[email protected]>
---
v1 -> v2
rebased
---
scripts/checkpatch.pl | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 30da08d9646a..4c1be774b0ed 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -4582,7 +4582,7 @@ sub process {
($op eq '>' &&
$ca =~ /<\S+\@\S+$/))
{
- $ok = 1;
+ $ok = 1;
}
# for asm volatile statements
@@ -4917,7 +4917,7 @@ sub process {
# conditional.
substr($s, 0, length($c), '');
$s =~ s/\n.*//g;
- $s =~ s/$;//g; # Remove any comments
+ $s =~ s/$;//g; # Remove any comments
if (length($c) && $s !~ /^\s*{?\s*\\*\s*$/ &&
$c !~ /}\s*while\s*/)
{
@@ -4956,7 +4956,7 @@ sub process {
# if and else should not have general statements after it
if ($line =~ /^.\s*(?:}\s*)?else\b(.*)/) {
my $s = $1;
- $s =~ s/$;//g; # Remove any comments
+ $s =~ s/$;//g; # Remove any comments
if ($s !~ /^\s*(?:\sif|(?:{|)\s*\\?\s*$)/) {
ERROR("TRAILING_STATEMENTS",
"trailing statements should be on next line\n" . $herecurr);
@@ -5132,7 +5132,7 @@ sub process {
{
}
- # Flatten any obvious string concatentation.
+ # Flatten any obvious string concatenation.
while ($dstat =~ s/($String)\s*$Ident/$1/ ||
$dstat =~ s/$Ident\s*($String)/$1/)
{
--
2.25.0
Commit 1574a29f8e76 ("checkpatch: allow multiple const * types")
claims to support repetition of pattern "const *", but it actually
allows only one extra instance.
Check the following lines
int a(char const * const x[]);
int b(char const * const *x);
int c(char const * const * const x[]);
int d(char const * const * const *x);
with command
./scripts/checkpatch.pl --show-types -f filename
to find that only the first line passes the test, while a warning
is triggered by the other 3 lines:
WARNING:FUNCTION_ARGUMENTS: function definition argument
'char const * const' should also have an identifier name
The reason is that the pattern match halts at the second asterisk
in the line, thus the remaining text starting with asterisk fails
to match a valid name for a variable.
Fixed by replacing "?" (Match 1 or 0 times) with "{0,4}" (Match
no more than 4 times) in the regular expression.
Fix also the similar test for types in unusual order.
Signed-off-by: Antonio Borneo <[email protected]>
Fixes: 1574a29f8e76 ("checkpatch: allow multiple const * types")
---
V1->V2
use a max match {0,4} instead of *, to limit the memory
used by perl
V2->V3
rebased
---
scripts/checkpatch.pl | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index a63380c6b0d2..30da08d9646a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -804,12 +804,12 @@ sub build_types {
}x;
$Type = qr{
$NonptrType
- (?:(?:\s|\*|\[\])+\s*const|(?:\s|\*\s*(?:const\s*)?|\[\])+|(?:\s*\[\s*\])+)?
+ (?:(?:\s|\*|\[\])+\s*const|(?:\s|\*\s*(?:const\s*)?|\[\])+|(?:\s*\[\s*\])+){0,4}
(?:\s+$Inline|\s+$Modifier)*
}x;
$TypeMisordered = qr{
$NonptrTypeMisordered
- (?:(?:\s|\*|\[\])+\s*const|(?:\s|\*\s*(?:const\s*)?|\[\])+|(?:\s*\[\s*\])+)?
+ (?:(?:\s|\*|\[\])+\s*const|(?:\s|\*\s*(?:const\s*)?|\[\])+|(?:\s*\[\s*\])+){0,4}
(?:\s+$Inline|\s+$Modifier)*
}x;
$Declare = qr{(?:$Storage\s+(?:$Inline\s+)?)?$Type};
--
2.25.0
On Wed, 2020-01-22 at 17:38 +0100, Antonio Borneo wrote:
> Fix spelling of "concatenation".
> Don't use tab after space in indentation.
>
> Signed-off-by: Antonio Borneo <[email protected]>
I've no objection to any of these 3 patches.
Andrew, might you pick them up please?
https://lore.kernel.org/patchwork/patch/1183806/
https://lore.kernel.org/patchwork/patch/1183805/
https://lore.kernel.org/patchwork/patch/1183804/
> ---
>
> v1 -> v2
> rebased
>
> ---
> scripts/checkpatch.pl | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 30da08d9646a..4c1be774b0ed 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -4582,7 +4582,7 @@ sub process {
> ($op eq '>' &&
> $ca =~ /<\S+\@\S+$/))
> {
> - $ok = 1;
> + $ok = 1;
> }
>
> # for asm volatile statements
> @@ -4917,7 +4917,7 @@ sub process {
> # conditional.
> substr($s, 0, length($c), '');
> $s =~ s/\n.*//g;
> - $s =~ s/$;//g; # Remove any comments
> + $s =~ s/$;//g; # Remove any comments
> if (length($c) && $s !~ /^\s*{?\s*\\*\s*$/ &&
> $c !~ /}\s*while\s*/)
> {
> @@ -4956,7 +4956,7 @@ sub process {
> # if and else should not have general statements after it
> if ($line =~ /^.\s*(?:}\s*)?else\b(.*)/) {
> my $s = $1;
> - $s =~ s/$;//g; # Remove any comments
> + $s =~ s/$;//g; # Remove any comments
> if ($s !~ /^\s*(?:\sif|(?:{|)\s*\\?\s*$)/) {
> ERROR("TRAILING_STATEMENTS",
> "trailing statements should be on next line\n" . $herecurr);
> @@ -5132,7 +5132,7 @@ sub process {
> {
> }
>
> - # Flatten any obvious string concatentation.
> + # Flatten any obvious string concatenation.
> while ($dstat =~ s/($String)\s*$Ident/$1/ ||
> $dstat =~ s/$Ident\s*($String)/$1/)
> {