2017-11-17 13:00:43

by Knut Omang

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

On Thu, 2017-11-16 at 14:57 -0800, Kees Cook wrote:
> On Thu, Nov 16, 2017 at 9:01 AM, Knut Omang <[email protected]> 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

From 1584301603998122432@xxx Fri Nov 17 08:35:49 +0000 2017
X-GM-THRID: 1584245555255033826
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread