Hi,
I found some odd behavior of checkpatch.pl, which I'm currently using
for own projects outside the kernel, but I verified that kernel patches
can be affected as well.
Reproducer code (file test.c):
------------------>8--
void subfunc(void)
{
}
FILE *g;
int main(void)
{
return 0;
}
------------------>8--
Checkpatch (called with extra options to only show the error to
demonstrate here):
--
$ diff -u /dev/null test.c | ./checkpatch.pl --no-tree --quiet --ignore
MISSING_SIGN_OFF,SPDX_LICENSE_TAG,COMMIT_MESSAGE
ERROR: need consistent spacing around '*' (ctx:WxV)
#8: FILE: test.c:5:
+FILE *g;
^
total: 1 errors, 0 warnings, 10 lines checked
--
"Consistent spacing"? Checkpatch must be in the wrong mode here. When
you add another declaration ("int a;") or even an empty statement (";")
before the "FILE *g;" line the reported error vanishes.
So patching $dbg_values = 2 in the code gives:
--
6 > . }
6 > EEEE
6 > ___
<E> <E> <_>WS(
)
7 > .
7 > EEE
7 > __
FILE *g;
<E> <E> <_>WS( )
<E> <E> <_>IDENT(FILE)
<E> <V> <_>WS( )
<E> <V> <_>OPV(*)
<E> <N> <_>IDENT(g)
<E> <V> <_>END(;)
<E> <E> <_>WS(
)
8 > . FILE *g;
8 > EEVVVVVNVEE
8 > ______B___
--
I found the (not too recent) patch 3e469cdc08ac ("checkpatch: optimise
statement scanner when mid-statement") and reverting it manually solves
the issue.
Also applying the following patch fixes the error for me:
------------------>8--
@@ -3913,7 +3913,7 @@ sub process {
# it there is no point in retrying a statement scan
# until we hit end of it.
my $frag = $stat; $frag =~ s/;+\s*$//;
- if ($frag !~ /(?:{|;)/) {
+ if ($frag !~ /(?:{|;|})/) {
#print "skip<$line_nr_next>\n";
$suppress_statement = $line_nr_next;
}
------------------>8--
The debug output changes to:
--
6 > . }
6 > EEEE
6 > ___
<E> <E> <_>WS(
)
7 > .
7 > EEE
7 > __
FILE *g;
<E> <E> <_>WS( )
<E> <E> <_>DECLARE(FILE *)
<E> <T> <_>IDENT(g)
<E> <V> <_>END(;)
<E> <E> <_>WS(
)
8 > . FILE *g;
8 > EETTTTTTVEE
8 > __________
--
Note "DECLARE(FILE *)" vs. "IDENT(FILE) WS( ) OPV(*)" before.
The same behavior can be constructed after any closing brace. So any
declaration of a non-standard type after a function, branch or loop
will trigger the error. And there are quite a few non-standard types in
the kernel tree as well.
I am not sure about the optimization patch referred to earlier and if my
suggested patch is correct with respect to it. If you think it is, feel
free to commit it yourself or tell me to send a [PATCH].
Thanks,
-Jan
On Wed, 2020-12-30 at 14:36 +0100, Jan Schlien wrote:
> Hi,
>
> I found some odd behavior of checkpatch.pl, which I'm currently using
> for own projects outside the kernel, but I verified that kernel patches
> can be affected as well.
Hello Jan.
I've had this patch locally for awhile that I believe fixes this
and another issue ($stat blocks can start with a close brace).
I've been waiting for Andy to review it in case there's some other
parsing problem associated with it.
---
scripts/checkpatch.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 00085308ed9d..012c8dc6cb1a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1555,7 +1555,7 @@ sub ctx_statement_block {
# Statement ends at the ';' or a close '}' at the
# outermost level.
- if ($level == 0 && $c eq ';') {
+ if ($level == 0 && ($c eq ';' || $c eq '}')) {
last;
}
On 30.12.20 19:52, Joe Perches wrote:
> On Wed, 2020-12-30 at 14:36 +0100, Jan Schlien wrote:
>> Hi,
>>
>> I found some odd behavior of checkpatch.pl, which I'm currently using
>> for own projects outside the kernel, but I verified that kernel patches
>> can be affected as well.
>
> Hello Jan.
>
> I've had this patch locally for awhile that I believe fixes this
> and another issue ($stat blocks can start with a close brace).
>
> I've been waiting for Andy to review it in case there's some other
> parsing problem associated with it.
Hello Joe,
Thanks for digging that out promptly. Looks good to me and also fixes my
issue. I cannot argue that I fully understand that function, so no
review but you can add
Tested-by: Jan Schlien <[email protected]>
if you like.
Best,
-Jan