When using a "FILE *" type, checkpatch considers this an error. Fix this by explicitly defining "FILE" as a common type.
This is useful for user space patches.
With this patch, we now get: static void a(FILE *const b)
<E> <E> <_>WS( ) <E> <E> <_>IDENT(static) <E> <V> <_>WS( ) <E> <V> <_>DECLARE(void ) <E> <T> <_>FUNC(a) <E> <V> <V>PAREN('(') <EV> <N> <_>DECLARE(FILE *const ) <EV> <T> <_>IDENT(b) <EV> <V> <_>PAREN(')') -> V <E> <V> <_>WS( ) 32 > . static void a(FILE *const b) 32 > EEVVVVVVVTTTTTVNTTTTTTTTTTTTVVV 32 > ______________________________
Another error may be throw when we use FIXTURE_{DATA,VARIANT}() structs, as defined in kselftest_harness.h . The commented $typeKselftestHarnessTypedefs should fix it but such definition is considered as a function instead, because of the closing parenthesis. I'd like some help to fix this as well.
With $typeKselftestHarnessTypedefs added to $typeTypedefs, we get: static void c(const FIXTURE_VARIANT(d) *const e)
<E> <E> <_>WS( ) <E> <E> <_>IDENT(static) <E> <V> <_>WS( ) <E> <V> <_>DECLARE(void ) <E> <T> <_>FUNC(c) <E> <V> <V>PAREN('(') <EV> <N> <_>MODIFIER(const) <EV> <T> <_>WS( ) <EV> <T> <_>FUNC(FIXTURE_VARIANT) <EV> <V> <V>PAREN('(') <EVV> <N> <_>IDENT(d) <EVV> <V> <_>PAREN(')') -> V <EV> <V> <_>WS( ) <EV> <V> <_>OPV(*) <EV> <N> <_>MODIFIER(const) <EV> <T> <_>WS( ) <EV> <T> <_>IDENT(e) <EV> <V> <_>PAREN(')') -> V <E> <V> <_>WS( ) 30 > . static void c(const FIXTURE_VARIANT(d) *const e) 30 > EEVVVVVVVTTTTTVNTTTTTTVVVVVVVVVVVVVVVNVVVNTTTTTTVVV 30 > ________________________________________B_________
Cc: Andy Whitcroft apw@canonical.com Cc: Dwaipayan Ray dwaipayanray1@gmail.com Cc: Joe Perches joe@perches.com Cc: Lukas Bulwahn lukas.bulwahn@gmail.com Signed-off-by: Mickaël Salaün mic@digikod.net Link: https://lore.kernel.org/r/20220901145948.1456353-1-mic@digikod.net --- scripts/checkpatch.pl | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 79e759aac543..016b47c35742 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -576,10 +576,17 @@ our $typeKernelTypedefs = qr{(?x: (?:__)?(?:u|s|be|le)(?:8|16|32|64)| atomic_t )}; +our $typeStdioTypedefs = qr{(?x: + FILE +)}; +# our $typeKselftestHarnessTypedefs = qr{(?x: +# FIXTURE_(?:DATA|VARIANT)($Ident) +# )}; our $typeTypedefs = qr{(?x: $typeC99Typedefs\b| $typeOtherOSTypedefs\b| - $typeKernelTypedefs\b + $typeKernelTypedefs\b| + $typeStdioTypedefs\b )};
our $zero_initializer = qr{(?:(?:0[xX])?0+$Int_type?|NULL|false)\b};
base-commit: b90cb1053190353cc30f0fef0ef1f378ccc063c5
On Thu, 2022-09-01 at 16:59 +0200, Mickaël Salaün wrote:
When using a "FILE *" type, checkpatch considers this an error. Fix this by explicitly defining "FILE" as a common type.
[]
Another error may be throw when we use FIXTURE_{DATA,VARIANT}() structs, as defined in kselftest_harness.h .
[]
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
@@ -576,10 +576,17 @@ our $typeKernelTypedefs = qr{(?x: (?:__)?(?:u|s|be|le)(?:8|16|32|64)| atomic_t )}; +our $typeStdioTypedefs = qr{(?x:
- FILE
+)};
I'm fine with this.
+# our $typeKselftestHarnessTypedefs = qr{(?x: +# FIXTURE_(?:DATA|VARIANT)($Ident) +# )};
But not this. Random userspace typedefs should likely be kept in some local version of checkpatch.
Or maybe add a command line option like --additional_typedefs=<file>.
our $typeTypedefs = qr{(?x: $typeC99Typedefs\b| $typeOtherOSTypedefs\b|
- $typeKernelTypedefs\b
- $typeKernelTypedefs\b|
- $typeStdioTypedefs\b
)};
On 01/09/2022 17:49, Joe Perches wrote:
On Thu, 2022-09-01 at 16:59 +0200, Mickaël Salaün wrote:
[...]
+# our $typeKselftestHarnessTypedefs = qr{(?x: +# FIXTURE_(?:DATA|VARIANT)($Ident) +# )};
But not this. Random userspace typedefs should likely be kept in some local version of checkpatch.
Or maybe add a command line option like --additional_typedefs=<file>.
This is part of the kselftest harness API. Could we take this into account for files under tools/testing/selftests ?
our $typeTypedefs = qr{(?x: $typeC99Typedefs\b| $typeOtherOSTypedefs\b|
- $typeKernelTypedefs\b
- $typeKernelTypedefs\b|
- $typeStdioTypedefs\b )};
On Thu, 2022-09-01 at 11:49 -0400, Joe Perches wrote:
On Thu, 2022-09-01 at 16:59 +0200, Mickaël Salaün wrote:
When using a "FILE *" type, checkpatch considers this an error. Fix this by explicitly defining "FILE" as a common type.
[]
Another error may be throw when we use FIXTURE_{DATA,VARIANT}() structs, as defined in kselftest_harness.h .
[]
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
@@ -576,10 +576,17 @@ our $typeKernelTypedefs = qr{(?x: (?:__)?(?:u|s|be|le)(?:8|16|32|64)| atomic_t )}; +our $typeStdioTypedefs = qr{(?x:
- FILE
+)};
I'm fine with this.
+# our $typeKselftestHarnessTypedefs = qr{(?x: +# FIXTURE_(?:DATA|VARIANT)($Ident) +# )};
But not this. Random userspace typedefs should likely be kept in some local version of checkpatch.
Or maybe add a command line option like --additional_typedefs=<file>.
Oops. I forgot it already exists:
--typedefsfile Read additional types from this file
commit 75ad8c575a5ad105e2afc2051c68abceb9c65431 Author: Jerome Forissier jerome.forissier@linaro.org Date: Mon May 8 15:56:00 2017 -0700
checkpatch: add --typedefsfile
When using checkpatch on out-of-tree code, it may occur that some project-specific types are used, which will cause spurious warnings. Add the --typedefsfile option as a way to extend the known types and deal with this issue.
On 01/09/2022 20:22, Joe Perches wrote:
On Thu, 2022-09-01 at 11:49 -0400, Joe Perches wrote:
On Thu, 2022-09-01 at 16:59 +0200, Mickaël Salaün wrote:
When using a "FILE *" type, checkpatch considers this an error. Fix this by explicitly defining "FILE" as a common type.
[]
Another error may be throw when we use FIXTURE_{DATA,VARIANT}() structs, as defined in kselftest_harness.h .
[]
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
@@ -576,10 +576,17 @@ our $typeKernelTypedefs = qr{(?x: (?:__)?(?:u|s|be|le)(?:8|16|32|64)| atomic_t )}; +our $typeStdioTypedefs = qr{(?x:
- FILE
+)};
I'm fine with this.
+# our $typeKselftestHarnessTypedefs = qr{(?x: +# FIXTURE_(?:DATA|VARIANT)($Ident) +# )};
But not this. Random userspace typedefs should likely be kept in some local version of checkpatch.
Or maybe add a command line option like --additional_typedefs=<file>.
Oops. I forgot it already exists:
--typedefsfile Read additional types from this file
commit 75ad8c575a5ad105e2afc2051c68abceb9c65431 Author: Jerome Forissier jerome.forissier@linaro.org Date: Mon May 8 15:56:00 2017 -0700
checkpatch: add --typedefsfile When using checkpatch on out-of-tree code, it may occur that some project-specific types are used, which will cause spurious warnings. Add the --typedefsfile option as a way to extend the known types and deal with this issue.
This doesn't work for the FIXTURE_DATA() case. And I'm not sure how contributors would know that they need to use this option with a specific file.
On Fri, 2022-09-02 at 11:04 +0200, Mickaël Salaün wrote:
On 01/09/2022 20:22, Joe Perches wrote:
On Thu, 2022-09-01 at 11:49 -0400, Joe Perches wrote:
On Thu, 2022-09-01 at 16:59 +0200, Mickaël Salaün wrote:
When using a "FILE *" type, checkpatch considers this an error. Fix this by explicitly defining "FILE" as a common type.
[]
Another error may be throw when we use FIXTURE_{DATA,VARIANT}() structs, as defined in kselftest_harness.h .
[]
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
@@ -576,10 +576,17 @@ our $typeKernelTypedefs = qr{(?x: (?:__)?(?:u|s|be|le)(?:8|16|32|64)| atomic_t )}; +our $typeStdioTypedefs = qr{(?x:
- FILE
+)};
I'm fine with this.
+# our $typeKselftestHarnessTypedefs = qr{(?x: +# FIXTURE_(?:DATA|VARIANT)($Ident) +# )};
But not this. Random userspace typedefs should likely be kept in some local version of checkpatch.
Or maybe add a command line option like --additional_typedefs=<file>.
Oops. I forgot it already exists:
--typedefsfile Read additional types from this file
commit 75ad8c575a5ad105e2afc2051c68abceb9c65431 Author: Jerome Forissier jerome.forissier@linaro.org Date: Mon May 8 15:56:00 2017 -0700
checkpatch: add --typedefsfile When using checkpatch on out-of-tree code, it may occur that some project-specific types are used, which will cause spurious warnings. Add the --typedefsfile option as a way to extend the known types and deal with this issue.This doesn't work for the FIXTURE_DATA() case.
checkpatch is a stupid little script. It's not a c preprocessor nor a syntax complete compiler. It's really easy for macros to make its output invalid. If you feed obfuscated c to checkpatch, it'd be confused. (Same is true for tools like coccinelle btw, though cocci is far better) checkpatch will never be comprehensive nor perfect. It's expected its users will use their common sense about its output message validity.
And I'm not sure how contributors would know that they need to use this option with a specific file.
--help exists
Maybe Documentation/dev-tools/checkpatch.rst could be expanded for --verbose mode.
On 02/09/2022 12:39, Joe Perches wrote:
On Fri, 2022-09-02 at 11:04 +0200, Mickaël Salaün wrote:
On 01/09/2022 20:22, Joe Perches wrote:
On Thu, 2022-09-01 at 11:49 -0400, Joe Perches wrote:
On Thu, 2022-09-01 at 16:59 +0200, Mickaël Salaün wrote:
When using a "FILE *" type, checkpatch considers this an error. Fix this by explicitly defining "FILE" as a common type.
[]
Another error may be throw when we use FIXTURE_{DATA,VARIANT}() structs, as defined in kselftest_harness.h .
[]
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
@@ -576,10 +576,17 @@ our $typeKernelTypedefs = qr{(?x: (?:__)?(?:u|s|be|le)(?:8|16|32|64)| atomic_t )}; +our $typeStdioTypedefs = qr{(?x:
- FILE
+)};
I'm fine with this.
+# our $typeKselftestHarnessTypedefs = qr{(?x: +# FIXTURE_(?:DATA|VARIANT)($Ident) +# )};
But not this. Random userspace typedefs should likely be kept in some local version of checkpatch.
Or maybe add a command line option like --additional_typedefs=<file>.
Oops. I forgot it already exists:
--typedefsfile Read additional types from this filecommit 75ad8c575a5ad105e2afc2051c68abceb9c65431 Author: Jerome Forissier jerome.forissier@linaro.org Date: Mon May 8 15:56:00 2017 -0700
checkpatch: add --typedefsfile When using checkpatch on out-of-tree code, it may occur that some project-specific types are used, which will cause spurious warnings. Add the --typedefsfile option as a way to extend the known types and deal with this issue.This doesn't work for the FIXTURE_DATA() case.
checkpatch is a stupid little script. It's not a c preprocessor nor a syntax complete compiler. It's really easy for macros to make its output invalid. If you feed obfuscated c to checkpatch, it'd be confused. (Same is true for tools like coccinelle btw, though cocci is far better) checkpatch will never be comprehensive nor perfect. It's expected its users will use their common sense about its output message validity.
And I'm not sure how contributors would know that they need to use this option with a specific file.
--help exists
Maybe Documentation/dev-tools/checkpatch.rst could be expanded for --verbose mode.
I was thinking about which file to use, but I understand your point. I'll send a v2 with only the "FILE" addition. FIXTURE_{DATA,VARIANT}() will just not be handled but that's OK.
linux-kselftest-mirror@lists.linaro.org
-
Joe Perches -
Mickaël Salaün