Re: [PATCH 0/7] Support for automatic checkpatch running in the kernel

From: Knut Omang
Date: Thu Nov 16 2017 - 23:47:44 EST


On Thu, 2017-11-16 at 14:57 -0800, Kees Cook wrote:
> On Thu, Nov 16, 2017 at 9:01 AM, Knut Omang <knut.omang@xxxxxxxxxx> wrote:
> > The most important checkpatch feature added is the --ignore-cfg feature, which
> > takes a file argument and parses that file according to this minimal language:
> >
> >ÂÂÂÂÂÂÂÂ# comments
> >ÂÂÂÂÂÂÂÂline_len <n>
> >ÂÂÂÂÂÂÂÂexcept checkpatch_type [files ...]
> >ÂÂÂÂÂÂÂÂpervasive checkpatch_type1 [checkpatch_type2 ...]
> >
> > With "make P=2" checkpatch is called with "--file" and "--ignore_cfg
> > checkpatch.cfg" which causes it to look for a file named 'checkpatch.cfg' in the
> > same directory as the source file. If that file exists, checkpatch will be run
> > with an implicit --strict and with the @ignore list expanded with content from
> > the configuration file.ÂÂIf it does not exist, make will simply silently ignore
> > the file.
>
> Will these configurations be cascading? (For example, all of net/ uses
> a different comment style, so having that recorded in a single file
> would be nice.)

Good point, the net/ use case is certainly something I have been thinking about.
I didn't want to make it too complex in the first set, so I let patch 1-4
implement the basics with one file per directory and nothing across directories.

Patch 6 and 7 in this set extends this to "fallback", which (with the net/ case in mind)
should allow a single file under net/ to cover the whole subsystem for all subtrees that
don't have their own checkpatch.cfg, but will be overridden by files in subdirectories.

So from a documentation point of view this could be done with the set as it is,
with people just copying the "pervasive" and "line_len"Âcommands from
a "global" net/checkpatch.cfg setup down into the subtrees as they start dealing with
exceptions.

If we want something more automatic, it can certainly be extended,Â
this would just require checkpatch to parse all the upwards checkpatch.cfg files for each
source file instead just taking the first found, which is what patch 6 implements.

Also the command language in the config files is rather limited, maybe other commands
maybe with more checkpatch additions would be useful to handle such scenarios..

Let me know what you think..

Thanks,
Knut