Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <026c08d9ab841ef626f80d968a532dea94a37c62.camel@perches.com>
Subject: Re: [RFC PATCH v1] checkpatch: Handle FILE pointer type
From:   Joe Perches <joe@perches.com>
To:     =?ISO-8859-1?Q?Micka=EBl_Sala=FCn?= <mic@digikod.net>,
        Andy Whitcroft <apw@canonical.com>
Cc:     Dwaipayan Ray <dwaipayanray1@gmail.com>,
        Lukas Bulwahn <lukas.bulwahn@gmail.com>,
        Shuah Khan <shuah@kernel.org>, linux-kernel@vger.kernel.org,
        linux-kselftest@vger.kernel.org,
        Jerome Forissier <jerome.forissier@linaro.org>
Date:   Fri, 02 Sep 2022 03:39:48 -0700
In-Reply-To: <b079d162-fa10-8cf0-bfc9-91f72a440e34@digikod.net>
References: <20220901145948.1456353-1-mic@digikod.net>
         <4f958a0c7c0aa2fce613371348477c002aa58e90.camel@perches.com>
         <b028cf83798fd562f5d1b992d2651d53e21acaa2.camel@perches.com>
         <b079d162-fa10-8cf0-bfc9-91f72a440e34@digikod.net>
Content-Type: text/plain; charset="ISO-8859-1"
Content-Transfer-Encoding: 8BIT
User-Agent: Evolution 3.44.4 (3.44.4-1.fc36) 
MIME-Version: 1.0
Precedence: bulk

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.