Messages in this thread | | | Date | Fri, 2 Sep 2022 13:04:04 +0200 | Subject | Re: [RFC PATCH v1] checkpatch: Handle FILE pointer type | From | Mickaël Salaün <> |
| |
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 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.
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.
| |