[PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program

[PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Knut Omang]

This patch series implements features to make it easier to run checkers on the
entire kernel as part of automatic and developer testing.

This is done by replacing the sparse specific setup for the C={1,2} variable
in the makefiles with setup for running scripts/runchecks, a new program that
can run any number of different "checkers". The behaviour of runchecks is
defined by simple "global" configuration in scripts/runchecks.cfg which can be
extended by local configuration applying to individual files, directories or
subtrees in the source.

It also fixes a minor issue with "checkpatch --fix-inplace" found during testing
(patch #3).

The runchecks.cfg files are parsed according to this minimal language:

       # comments
       # "Global configuration in scripts/runchecks.cfg:
       checker <name>
       typedef NAME regex
       run <list of checkers or "all"

       # "local" configuration:
       line_len <n>
       except checkpatch_type [files ...]
       pervasive checkpatch_type1 [checkpatch_type2 ...]

With "make C=2" runchecks first parse the file scripts/runchecks.cfg, then
look for a file named 'runchecks.cfg' in the same directory as the source file.
If that file exists, it will be parsed and it's local configuration applied to
allow suppression on a per checker, per check and per file basis.
If a "local" configuration does not exist, either in the source directory or
above, make will simply silently ignore the file.

The idea is that the community can work to add runchecks.cfg files to
directories, serving both as documentation and as a way for subsystem
maintainers to enforce policies and individual tastes as well as TODOs and/or
priorities, to make it easier for newcomers to contribute in this area. By
ignoring directories/subtrees without such files, automation can start right
away as it is trivially possible to run errorless with C=2 for the entire
kernel.

For the checker maintainers this should be a benefit as well: new
or improved checks would generate new errors/warnings. With automatic testing
for the checkers, these new checks would generate error reports and cause
builds to fail. Adding the new check a 'pervasive' option at the top level or
even for specific files, marked with a "new check - please consider fixing" comment
or similar would make those builds pass while documenting and making the new check
more visible.

The patches includes a documentation file with some more details.

This patch set has evolved from an earlier shell script implementation I made
as only a wrapper script around checkpatch. That version have been used for a
number of years on a driver project I worked on where we had automatic checkin
regression testing. I extended that to also run checkpatch to avoid having to
clean up frequent unintended whitespace changes and style violations from others...

I have also tested this version on some directories I am familiar with.  The
result of that work is available in two patch sets of 10 and 11 patches, but we
agreed that it would be better to post them as separate patch sets later.

Those patch sets illustrates how I picture the "flow" from just "reining in" the
checkpatch detections to actually fixing classes of checkpatch issues one by
one, while updating the checkpatch.cfg file(s) to have 0 errors or warnings at
any commit boundary.

The combined set is available here:

   git://github.com/knuto/linux.git  branch runchecks

I only include version 0 of runchecks.cfg in the two directories I
worked on here as the final two patches. These files both documents where
the issues are in those two directories, and can be used by the maintainer
to indicate to potential helpers what to focus on as I have tried to
illustrate by means of comments.

Changes from v1:
-----------------
Based on feedback, the implementation is completely rewritten and extended.
Instead of patches to checkpatch, and a sole checkpatch focus, it is now a
generic solution implemented in python, for any type of checkers, extendable
through some basic generic functionality, and for special needs by subclassing
the Checker class in the implementation.

This implementation fully supports checkpatch, sparse and
checkdoc == kernel-doc -none, and also has been tested with coccicheck.
To facilitate the same mechanism of using check types to filter what
checks to be suppressed, I introduced the concept of "typedefs" which allows
runchecks to effectively augment the check type space of the checker in cases
where types either are not available at all (checkdoc) or where only a few
can be filtered out (sparse)

With this in place it also became trivial to make the look and feel similar
for sparse and checkdoc as for checkpatch, with some optional color support
too, to make fixing issues in the code, the goal of this whole exercise,
much more pleasant IMHO :-)

Thanks,
Knut

Knut Omang (5):
  runchecks: Generalize make C={1,2} to support multiple checkers
  Documentation: Add doc for runchecks, a checker runner
  checkpatch: Improve --fix-inplace for TABSTOP
  rds: Add runchecks.cfg for net/rds
  RDMA/core: Add runchecks.cfg for drivers/infiniband/core

 Documentation/dev-tools/coccinelle.rst |  12 +-
 Documentation/dev-tools/index.rst      |   1 +-
 Documentation/dev-tools/runchecks.rst  | 215 ++++++++-
 Documentation/dev-tools/sparse.rst     |  30 +-
 Documentation/kbuild/kbuild.txt        |   9 +-
 Makefile                               |  23 +-
 drivers/infiniband/core/runchecks.cfg  |  83 +++-
 net/rds/runchecks.cfg                  |  76 +++-
 scripts/Makefile.build                 |   4 +-
 scripts/checkpatch.pl                  |   2 +-
 scripts/runchecks                      | 734 ++++++++++++++++++++++++++-
 scripts/runchecks.cfg                  |  63 ++-
 scripts/runchecks_help.txt             |  43 ++-
 13 files changed, 1274 insertions(+), 21 deletions(-)
 create mode 100644 Documentation/dev-tools/runchecks.rst
 create mode 100644 drivers/infiniband/core/runchecks.cfg
 create mode 100644 net/rds/runchecks.cfg
 create mode 100755 scripts/runchecks
 create mode 100644 scripts/runchecks.cfg
 create mode 100644 scripts/runchecks_help.txt

base-commit: ae64f9bd1d3621b5e60d7363bc20afb46aede215
-- 
git-series 0.9.1

[PATCH v2 1/5] runchecks: Generalize make C={1,2} to support multiple checkers

[Knut Omang]

Add scripts/runchecks which has generic support for running
checker tools in a convenient and user friendly way that
the author hopes can contribute to rein in issues detected
by these tools in a manageable and convenient way.

scripts/runchecks provides the following basic functionality:

* Makes it possible to selectively suppress output from individual
  checks on a per file or per subsystem basis.
* Unifies output and suppression input from different tools
  by providing a single unified syntax for the underlying tools
  in the style of "scripts/checkpatch.pl --show-types".
* Allows selective run of one, or more (or all) configured tools
  for each file.

In the Makefile system, the sparse specific setup has been replaced
by setup for runchecks.

This version of runchecks together with a "global" configuration
file in "scripts/runchecks.cfg" supports sparse, checkpatch and checkdoc,
a trivial abstraction above a call to 'kernel-doc -none'.
It also supports forwarding calls to coccicheck for coccinelle support
but this is not quite as worked through as the three other checkers,
mainly because of lack of error data as all checks pass by default
right now.

The code is designed to be easily extensible to support more checkers
as they emerge, and some generic checker support is even available
just via simple additions to "scripts/runchecks.cfg".

Signed-off-by: Knut Omang <knut.omang@oracle.com>
---
 Makefile                   |  23 +-
 scripts/Makefile.build     |   4 +-
 scripts/runchecks          | 734 ++++++++++++++++++++++++++++++++++++++-
 scripts/runchecks.cfg      |  63 +++-
 scripts/runchecks_help.txt |  43 ++-
 5 files changed, 857 insertions(+), 10 deletions(-)
 create mode 100755 scripts/runchecks
 create mode 100644 scripts/runchecks.cfg
 create mode 100644 scripts/runchecks_help.txt

diff --git a/Makefile b/Makefile
index c988e46..791e8df 100644
--- a/Makefile
+++ b/Makefile
@@ -159,14 +159,22 @@ ifeq ($(skip-makefile),)
 # so that IDEs/editors are able to understand relative filenames.
 MAKEFLAGS += --no-print-directory
 
-# Call a source code checker (by default, "sparse") as part of the
-# C compilation.
+# Do source code checking as part of the C compilation.
+#
 #
 # Use 'make C=1' to enable checking of only re-compiled files.
 # Use 'make C=2' to enable checking of *all* source files, regardless
 # of whether they are re-compiled or not.
 #
-# See the file "Documentation/dev-tools/sparse.rst" for more details,
+# Source code checking is done via the runchecks script, which
+# has knowledge of each individual cheker and how it wants to be called,
+# as well as options for rules as to which checks that are applicable
+# to different parts of the kernel, at source file granularity.
+#
+# Several types of checking is available, and custom checkers can also
+# be added.
+#
+# See the file "Documentation/dev-tools/runchecks.rst" for more details,
 # including where to get the "sparse" utility.
 
 ifeq ("$(origin C)", "command line")
@@ -383,10 +391,9 @@ INSTALLKERNEL  := installkernel
 DEPMOD		= /sbin/depmod
 PERL		= perl
 PYTHON		= python
-CHECK		= sparse
 
-CHECKFLAGS     := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ \
-		  -Wbitwise -Wno-return-void $(CF)
+CHECK		= $(srctree)/scripts/runchecks
+CHECKFLAGS      =
 NOSTDINC_FLAGS  =
 CFLAGS_MODULE   =
 AFLAGS_MODULE   =
@@ -429,7 +436,7 @@ GCC_PLUGINS_CFLAGS :=
 export ARCH SRCARCH CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC
 export CPP AR NM STRIP OBJCOPY OBJDUMP HOSTLDFLAGS HOST_LOADLIBES
 export MAKE AWK GENKSYMS INSTALLKERNEL PERL PYTHON UTS_MACHINE
-export HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS
+export HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECK_CFLAGS CHECKFLAGS
 
 export KBUILD_CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS LDFLAGS
 export KBUILD_CFLAGS CFLAGS_KERNEL CFLAGS_MODULE CFLAGS_KASAN CFLAGS_UBSAN
@@ -778,7 +785,7 @@ endif
 
 # arch Makefile may override CC so keep this after arch Makefile is included
 NOSTDINC_FLAGS += -nostdinc -isystem $(call shell-cached,$(CC) -print-file-name=include)
-CHECKFLAGS     += $(NOSTDINC_FLAGS)
+CHECK_CFLAGS     += $(NOSTDINC_FLAGS)
 
 # warn about C99 declaration after statement
 KBUILD_CFLAGS += $(call cc-option,-Wdeclaration-after-statement,)
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index cb8997e..13325b3 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -93,10 +93,10 @@ __build: $(if $(KBUILD_BUILTIN),$(builtin-target) $(lib-target) $(extra-y)) \
 ifneq ($(KBUILD_CHECKSRC),0)
   ifeq ($(KBUILD_CHECKSRC),2)
     quiet_cmd_force_checksrc = CHECK   $<
-          cmd_force_checksrc = $(CHECK) $(CHECKFLAGS) $(c_flags) $< ;
+          cmd_force_checksrc = $(CHECK) $(CF) $< -- $(CHECKFLAGS) $(c_flags);
   else
       quiet_cmd_checksrc     = CHECK   $<
-            cmd_checksrc     = $(CHECK) $(CHECKFLAGS) $(c_flags) $< ;
+            cmd_checksrc     = $(CHECK) $(CF) $< -- $(CHECKFLAGS) $(c_flags);
   endif
 endif
 
diff --git a/scripts/runchecks b/scripts/runchecks
new file mode 100755
index 0000000..4dd2969
--- /dev/null
+++ b/scripts/runchecks
@@ -0,0 +1,734 @@
+#!/usr/bin/python
+
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
+#    Author: Knut Omang <knut.omang@oracle.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 2
+# as published by the Free Software Foundation.
+
+# The program implements a generic and extensible code checker runner
+# that supports running various checker tools from the kernel Makefile
+# or standalone, with options for selectively suppressing individual
+# checks on a per file or per check basis.
+#
+# The program has some generic support for checkers, but to implement
+# support for a new checker to the full extent, it might be necessary to
+#   1) subclass the Checker class in this file with checker specific processing.
+#   2) add typedef definitions in runchecks.cfg in this directory
+#
+# This version of runchecks has full support for the following tools:
+#   sparse:      installed separately
+#   checkpatch:  checkpatch.pl
+#   checkdoc:    kernel-doc -none
+#
+# See file "Documentation/dev-tools/runchecks.rst" for more details
+#
+
+import sys, os, subprocess, fcntl, select, re
+from os.path import dirname, basename
+
+class CheckError(Exception):
+    def __init__(self, value):
+        self.value = value
+    def __str__(self):
+        return self.value
+
+def usage():
+    manual = os.path.join(srctree, "scripts/runchecks_help.txt")
+    f = open(manual, "r")
+    for line in f:
+        sys.stdout.write(line)
+    f.close()
+    print ""
+    print "Configured checkers:"
+    for (c, v) in checker_types.iteritems():
+        enabled = "[default disabled]"
+        for c_en in config.checkers:
+            if c_en.name == c:
+                enabled = ""
+                break
+        print "  %-20s %s" % (c, enabled)
+    exit(1)
+
+
+# A small configuration file parser:
+#
+class Config:
+    def __init__(self, srctree, workdir, filename):
+        self.path = []
+        self.relpath = {}
+        relpath = ""
+
+        # Look for a global config file in the scripts directory:
+        file = os.path.join(srctree, "scripts/%s" % filename)
+        if os.path.exists(file):
+            self.path.append(file)
+            self.relpath[file] = relpath
+
+        while not ignore_config:
+            self.file = os.path.join(workdir,filename)
+            if os.path.exists(self.file):
+                self.path.append(self.file)
+                self.relpath[self.file] = relpath
+            if len(workdir) <= len(srctree):
+                break
+            relpath = "%s/%s" % (basename(workdir), relpath)
+            workdir = dirname(workdir)
+
+        self.checkers = []
+        self.cur_chk = None
+        self.color = False
+        self.list_only = False
+
+        self.command = {
+            "checker"	: self.checker,
+            "addflags"	: self.addflags,
+            "run"	: self.runlist,
+            "except"	: self.exception,
+            "pervasive" : self.pervasive,
+            "cflags"	: self.cflags,
+            "typedef"	: self.typedef
+        }
+
+        if verbose:
+            print "  ** runchecks: config path: %s" % self.path
+        for f in self.path:
+            self.ParseConfig(f)
+
+    def checker(self, argv):
+        try:
+            self.cur_chk = checker_types[argv[0]]
+        except KeyError:
+            if len(argv) < 2:
+                d1 = "generic checker configurations!"
+                raise CheckError("%s:%d: use 'checker %s command' for %s" % \
+                                 (self.file, self.lineno, argv[0], d1))
+
+            AddChecker(Checker(argv[0], argv[1], srctree, workdir))
+            self.cur_chk = checker_types[argv[0]]
+
+    def addflags(self, argv):
+        self.cur_chk.addflags(argv)
+
+    def exception(self, argv):
+        type = argv[0]
+        if self.cur_chk:
+            relpath = self.relpath[self.file]
+            self.cur_chk.exception(type, relpath, argv[1:])
+        else:
+            raise CheckError("%s:%d: checker has not been set" % (self.file, self.lineno))
+
+    def pervasive(self, argv):
+        self.cur_chk.pervasive(argv)
+
+    def runlist(self, argv):
+        try:
+            for c in argv:
+                self.checkers.append(checker_types[c])
+        except KeyError, k:
+            if str(k) == "'all'":
+                self.checkers = checker_types.values()
+            else:
+                available = "\n  -- avaliable checkers are: %s" % ",".join(checker_types.keys())
+                raise CheckError("Checker %s not found - not configured?%s" % (str(k), available))
+
+    def cflags(self, argv):
+        self.cur_chk.cflags = True
+
+    def typedef(self, argv):
+        self.cur_chk.typedef(argv)
+
+    # Parse one configuration file in the configuration file list:
+    #
+    def ParseConfig(self, file):
+        f = open(file, 'r')
+        self.file = file
+        self.lineno = 0
+        for line in f:
+            self.lineno = self.lineno + 1
+            token = line.split()
+            if len(token) < 1:
+                continue
+            if token[0][0] == '#':
+                continue
+            try:
+                self.command[token[0]](token[1:])
+            except KeyError:
+                if not self.cur_chk:
+                    raise CheckError("%s:%s: checker has not been set" % (self.file, self.lineno))
+                self.cur_chk.ParseOptional(token[0], token[1:])
+            except AttributeError:
+                if not self.cur_chk:
+                    raise CheckError("%s:%s: checker has not been set" % (self.file, self.lineno))
+
+        f.close()
+        self.cur_chk = None
+
+    # Option forwarding to checkers
+    # and optional selection of which checkers to run:
+    def ProcessOpts(self, opts):
+        for opt in opts:
+            if opt == "--color":
+                self.color = True
+                continue
+            elif opt == "--list":
+                self.list_only = True
+                continue
+            elif opt == "--help":
+                usage_only = True
+
+            fw = re.match("^--to-(\w+):(.*)$", opt)
+            if fw:
+                try:
+                    cname = fw.group(1)
+                    checker = checker_types[cname]
+                except:
+                    raise CheckError("Unknown checker '%s' specified in option '%s'" % (cname, opt))
+                newargs = fw.group(2).split(',')
+                checker.cmdvec += newargs
+                if verbose:
+                    print "Added extra args for %s: %s" % (cname, newargs)
+                continue
+
+            runopt = re.match("^--run:(.*)$", opt)
+            if runopt:
+                clist = runopt.group(1).split(",")
+                # Command line override: reset list of checkers
+                self.checkers = []
+                self.runlist(clist)
+                continue
+
+            if len(self.checkers) == 1:
+                # If only one checker enabled, just pass everything we don't know about through:
+                self.checkers[0].cmdvec.append(opt)
+            else:
+                raise CheckError("Unknown option '%s'" % opt)
+
+    # We always expect at least one config file that sets up the active checkers:
+    #
+    def HasPathConfig(self):
+        return len(self.path) > 1
+
+
+# The base class for checkers:
+# For specific support a particular checker, implement a subclass of this:
+#
+class Checker:
+    def __init__(self, name, cmd, srctree, workdir, ofilter = None, efilter = None):
+        self.name = name
+        self.srctree = srctree
+	self.workdir = workdir
+	self.efilter = efilter
+        if ofilter:
+            self.ofilter = ofilter
+        else:
+            self.ofilter = self.suppress
+        self.strout = ""
+        self.strerr = ""
+        self.cflags = False
+        if cmd[0:7] == "scripts":
+            cmd = os.path.join(self.srctree, cmd)
+        self.cmd = cmd
+        self.cmdvec = cmd.split()
+        self.pervasive_opts = [] # "global" ignore list
+        self.exceptions = []     # exception list for this file
+        self.file_except = []    # Aggregated list of check types to ignore for this file
+        self.re_except_def = {}  # check_type -> <regex to match it in stderr>
+        self.doc = {}		 # Used when parsing documentation: check type -> doc string
+        self.cont = []
+        self.last_ignore = False
+        self.unclassified = 0	 # With RegexFilter: Number of "red" lines not classified
+
+    def filter_env(self, dict):
+        return dict
+
+    def readline(self, is_stdout, fd):
+        tmp_str = ""
+        try:
+            s = os.read(fd, 1000)
+            while s != '':
+                tmp_str += s
+                s = os.read(fd, 1)
+        except OSError:
+            None
+
+        if is_stdout:
+            self.strout += tmp_str
+            tmp_str = self.strout
+        else:
+            self.strerr += tmp_str
+            tmp_str = self.strerr
+
+        inx = tmp_str.find('\n') + 1
+        if inx != 0:
+            t = tmp_str[:inx]
+            if is_stdout:
+                self.strout = tmp_str[inx:]
+            else:
+                self.strerr = tmp_str[inx:]
+        else:
+            return ''
+        return t
+
+    def SetNonblocking(self, fd):
+        fl = fcntl.fcntl(fd, fcntl.F_GETFL)
+        try:
+            fcntl.fcntl(fd, fcntl.F_SETFL, fl | os.O_NDELAY)
+        except AttributeError:
+            fcntl.fcntl(fd, fcntl.F_SETFL, fl | fcntl.FNDELAY)
+
+    def Run(self, file, verbose):
+        cmdvec = self.cmdvec
+        if self.cflags:
+            cmdvec += c_argv
+        if not force:
+            self.file_except = set(self.exceptions + self.pervasive_opts)
+        self.Postprocess()
+        if not file:
+            raise CheckError("error: missing file parameter")
+        cmdvec.append(file)
+        if debug:
+            print "  ** running %s: %s" % (self.name, " ".join(cmdvec))
+        elif verbose:
+            print "  -- checker %s --" % self.name
+        try:
+            ret = self.RunCommand(cmdvec, self.ofilter, self.efilter)
+        except OSError, e:
+            if re.match(".*No such file or directory", str(e)):
+                if len(config.checkers) == 1:
+                    raise CheckError("Failed to run checker %s: %s: %s" % (self.name, self.cmd, str(e)))
+                if verbose:
+                    print "  ** %s does not exist - ignoring %s **" % (self.name, self.cmd)
+                return 0
+        ret = self.PostRun(ret)
+        return ret
+
+    def RunCommand(self, cmdvec, ofilter, efilter):
+	my_env = self.filter_env(os.environ)
+        child = subprocess.Popen(cmdvec, shell = False, \
+                     stdout=subprocess.PIPE, stderr=subprocess.PIPE, cwd=".", env=my_env)
+        sout = child.stdout
+        serr = child.stderr
+        ofd = sout.fileno()
+        efd = serr.fileno()
+        oeof = False
+        eeof = False
+        check_errors = []
+        self.SetNonblocking(ofd)
+        self.SetNonblocking(efd)
+        while True:
+            ready = select.select([ofd,efd],[],[],0.1)
+            if ofd in ready[0]:
+                if child.poll() != None:
+                    oeof = True
+                oline = self.readline(True, ofd)
+                while oline != '':
+                    if ofilter:
+                        ofilter(oline, verbose)
+                    else:
+                        sys.stdout.write(oline)
+                    oline = self.readline(True, ofd)
+            if efd in ready[0]:
+                if child.poll() != None:
+                    eeof = True
+                eline = self.readline(False, efd)
+                while eline != '':
+                    if efilter:
+                        check_err = efilter(eline, verbose)
+                        if check_err != None:
+                                check_errors.append(check_err)
+                    else:
+                        sys.stderr.write(eline)
+                    eline = self.readline(False, efd)
+            if oeof and eeof:
+                break
+        serr.close()
+        sout.close()
+        retcode = child.wait()
+        if check_errors != []:
+            estr = "".join(check_errors)
+            if estr != "":
+                sys.stderr.write(estr)
+                if not retcode:
+                    retcode = 131
+            else:
+                if verbose:
+                    print "%s  ** %d suppressed errors/warnings from %s%s" % (BLUE, len(check_errors), self.name, ENDCOLOR)
+                retcode = 0
+        return retcode
+
+    def ParseOptional(self, cmd, argv):
+        raise CheckError("Undefined command '%s' for checker '%s'" % (cmd, self.name))
+
+    # Called as final step before running the checker:
+    def Postprocess(self):
+        # Do nothing - just for redefinition in subclasses
+        return
+
+    # Called as a post processing step after running the checker:
+    # Input parameter is return value from Run()
+    def PostRun(self, retval):
+        # Do nothing - just for redefinition in subclasses
+        return retval
+
+    # Default standard output filter:
+    def suppress(self, line, verbose):
+        if verbose:
+            sys.stdout.write(line)
+
+    # A matching filter for stderr:
+    def RegexFilter(self, line, verbose):
+        if self.cont:
+            m = re.match(self.cont[0], line)
+            self.cont = self.cont[1:]
+            if m:
+                if self.last_ignore:
+                    return ""
+                else:
+                    return line
+
+        for t, regex in self.re_except_def.iteritems():
+            r = "^(.*:\d+:)\s(\w+:)\s(%s.*)$" % regex[0]
+            m = re.match(r, line)
+            if m:
+                if len(regex) > 1:
+                    self.cont = regex[1:]
+                if t in self.file_except:
+                    self.last_ignore = True
+                    return ""
+                else:
+                    self.last_ignore = False
+                    return "%s%s %s:%s%s%s: %s %s\n" % (BROWN, m.group(1), self.name.upper(), BLUE, t, ENDCOLOR, m.group(2), m.group(3))
+        self.unclassified = self.unclassified + 1
+        return RED + line + ENDCOLOR
+
+    def ListTypes(self):
+        if len(self.re_except_def) > 0:
+            print BLUE + BOLD + "  Check types declared for %s in runchecks configuration%s" % (self.name, ENDCOLOR)
+        for t, regex in self.re_except_def.iteritems():
+            print "\t%-22s %s" % (t, "\\n".join(regex))
+        if len(self.re_except_def) > 0:
+            print ""
+        return 0
+
+    def addflags(self, argv):
+        self.cmdvec += argv
+
+    def exception(self, type, relpath, argv):
+        for f in argv:
+            if f == ("%s%s" % (relpath, bfile)):
+                self.exceptions.append(type)
+
+    def pervasive(self, argv):
+        self.pervasive_opts += argv
+
+    def typedef(self, argv):
+        exp = " ".join(argv[1:])
+        elist = exp.split("\\n")
+        self.re_except_def[argv[0]] = elist
+
+
+# Individual checker implementations:
+#
+
+# checkpatch
+class CheckpatchRunner(Checker):
+    def __init__(self, srctree, workdir):
+        Checker.__init__(self, "checkpatch", "scripts/checkpatch.pl", srctree, workdir)
+        self.cmdvec.append("--file")
+        self.line_len = 0
+        # checkpatch sends all it's warning and error output to stdout,
+        # redirect and do limited filtering:
+        self.ofilter = self.out_filter
+
+    def ParseOptional(self, cmd, argv):
+        if cmd == "line_len":
+            self.line_len = int(argv[0])
+        else:
+            Checker.ParseOptional(self, cmd, argv)
+
+    def Postprocess(self):
+        if config.color:
+            self.cmdvec.append("--color=always")
+        if self.line_len:
+            self.cmdvec.append("--max-line-length=%d" % self.line_len)
+        if self.file_except:
+            self.cmdvec.append("--ignore=%s" % ",".join(self.file_except))
+
+    # Extracting a condensed doc of types to filter on:
+    def man_filter(self, line, verbose):
+        t = line.split()
+        if len(t) > 1 and t[1] != "Message":
+            sys.stdout.write("\t%s\n" % t[1])
+
+    def out_filter(self, line, verbose):
+        # --terse produces this message even with no errors,
+        #  suppress unless run with -v:
+        if not verbose and re.match("^total: 0 errors, 0 warnings, 0 checks,", line):
+            return
+        sys.write.stderr(line)
+
+    def ListTypes(self):
+        print BLUE + BOLD + "  Supported check types for checkpatch" + ENDCOLOR
+        # Parse help output:
+        cmdvec = ["%s/scripts/checkpatch.pl" % self.srctree, "--list-types"]
+        self.RunCommand(cmdvec, self.man_filter, None)
+        print ""
+        return 0
+
+# sparse
+class SparseRunner(Checker):
+    def __init__(self, srctree, workdir):
+        Checker.__init__(self, "sparse", "sparse", srctree, workdir)
+        self.efilter = self.RegexFilter
+
+    def sparse_name(self, rs_type):
+        l_name = rs_type.lower()
+        s_name = ""
+        for c in l_name:
+            if c == '_':
+                s_name += '-'
+            else:
+                s_name += c
+        return s_name
+
+    def runchecks_name(self, sparse_type):
+        u_name = sparse_type.upper()
+        rc_name =""
+        for c in u_name:
+            if c == '-':
+                rc_name += '_'
+            else:
+                rc_name += c
+        return rc_name
+
+    def Postprocess(self):
+        if self.file_except:
+            for e in self.file_except:
+                self.cmdvec.append("-Wno-%s" % self.sparse_name(e))
+
+    # Extracting a condensed doc of types to filter on:
+    def man_filter(self, line, verbose):
+        if self.doc_next:
+            doc = line.strip()
+            self.doc[self.doc_next] = doc
+            self.doc_next = False
+            return
+        match = re.search("^\s+-W([\w-]+)\s*$", line)
+        if match:
+            name = match.group(1)
+            if re.match("sparse-", name):
+                return
+            rs_type = self.runchecks_name(name)
+            self.doc_next = rs_type
+
+    def ListTypes(self):
+        # Parse manual output:
+        cmdvec = ["man", "sparse"]
+        self.doc_next = False
+        ret = self.RunCommand(cmdvec, self.man_filter, None)
+        if ret:
+            return ret
+        print BLUE + BOLD + "\n  Types derived from sparse from documentation in manpage" + ENDCOLOR
+        for t, doc in self.doc.iteritems():
+            print "\t%-22s %s" % (t, doc)
+            try:
+                regex = self.re_except_def[t]
+                print "\t%-22s %s" % ("", GREEN + "\\n".join(regex) + ENDCOLOR)
+            except:
+                print "\t%-22s %s" % ("", RED + "(regex match (typedef) missing)" + ENDCOLOR)
+        print BLUE + BOLD + "\n  Types for sparse only declared for runchecks or not documented in manpage" + ENDCOLOR
+        for t, regex in self.re_except_def.iteritems():
+            try:
+                self.doc[t]
+            except:
+                print "\t%-22s %s" % (t, GREEN + "\\n".join(regex) + ENDCOLOR)
+        print ""
+        return 0
+
+# checkdoc
+class CheckdocRunner(Checker):
+    def __init__(self, srctree, workdir):
+        Checker.__init__(self, "checkdoc", "scripts/kernel-doc", srctree, workdir)
+        self.cmdvec.append("-none")
+        self.efilter = self.RegexFilter
+
+# coccicheck (coccinelle) (WIP)
+class CoccicheckRunner(Checker):
+    def __init__(self, srctree, workdir):
+        Checker.__init__(self, "coccicheck", "scripts/coccicheck", srctree, workdir)
+        self.debug_file = None
+        self.efilter = self.CoccicheckFilter
+
+    def filter_env(self, dict):
+        newdict = os.environ
+        # If debug file is not set by the user, override it and present the output on stderr:
+        try:
+            df = newdict["DEBUG_FILE"]
+        except:
+            print "*** debug_file!"
+            self.debug_file = '/tmp/cocci_%s.log' % os.getpid()
+            newdict["DEBUG_FILE"] = self.debug_file
+        return newdict
+
+    def CoccicheckFilter(self, line, verbose):
+        self.unclassified = self.unclassified + 1
+        if re.match(".*spatch -D report", line):
+            if verbose:
+                sys.stdout.write(line)
+        else:
+            return RED + line + ENDCOLOR
+
+    def PostRun(self, retval):
+        if not self.debug_file:
+            return retval
+        f = open(self.debug_file)
+        for line in f:
+            line = self.CoccicheckFilter(line, verbose)
+            if line:
+                sys.stderr.write(line)
+        f.close()
+        if self.debug_file:
+            os.remove(self.debug_file)
+        if retval == 0:
+            reval = ret
+        return retval
+
+checker_types = {}
+
+def AddChecker(checker):
+    checker_types[checker.name] = checker
+
+#
+# Start main program:
+#
+program = os.path.realpath(sys.argv[0])
+progname = basename(program)
+scriptsdir = dirname(program)
+srctree = dirname(scriptsdir)
+force = False
+ignore_config = False
+verbose = False
+debug = False
+error = True
+error_on_red = False
+usage_only = False
+argv = []
+c_argv = []
+fw_opts = []
+workdir = os.getcwd()
+optarg = False
+argc = 0
+
+AddChecker(CheckpatchRunner(srctree, workdir))
+AddChecker(SparseRunner(srctree, workdir))
+AddChecker(CheckdocRunner(srctree, workdir))
+AddChecker(CoccicheckRunner(srctree, workdir))
+
+for arg in sys.argv[1:]:
+    argc = argc + 1
+
+    if arg == "--":
+        argc = argc + 1
+        c_argv = sys.argv[argc:]
+        break;
+    elif arg == "-f":
+        force = True
+    elif arg == "-n":
+        ignore_config = True
+        force = True
+    elif arg == "-w":
+        error = False
+    elif arg == "-t":
+        error_on_red = True
+        error = False
+    elif arg == "-d":
+        debug = True
+    elif arg == "-v":
+        verbose = True
+    elif arg == "-h":
+        usage_only = True
+    else:
+        opt = re.match("^-.*$", arg)
+        if opt:
+            # Delay processing of these until we know the configuration:
+            fw_opts.append(opt.group(0))
+        else:
+            argv.append(arg)
+
+if not verbose:
+    try:
+        verb = int(os.environ["V"])
+        if verb != 0:
+            verbose = True
+    except KeyError:
+        verbose = False
+
+if not os.path.exists(os.path.join(srctree, "MAINTAINERS")):
+    srctree = None
+
+try:
+    file = argv[0]
+    bfile = basename(file)
+    workdir = dirname(file)
+except:
+    bfile = None
+    file = None
+
+unclassified = 0
+
+if debug:
+    print "Kernel root:\t%s\nFile:\t\t%s\nWorkdir:\t%s" % \
+        (srctree, bfile, workdir)
+    print "C args:\t\t%s\nargv:\t\t%s\n" % (" ".join(c_argv), " ".join(argv))
+
+try:
+    config = Config(srctree, workdir, "runchecks.cfg")
+    config.ProcessOpts(fw_opts)
+
+    if usage_only:
+        usage()
+    if not config.HasPathConfig() and not config.list_only and not force:
+        if verbose:
+            print "  ** %s: No configuration found - skip checks for %s" % (progname, file)
+        exit(0)
+
+    if config.color:
+        GREEN = '\033[32m'
+        RED = '\033[91m'
+        BROWN = '\033[33m'
+        BLUE = '\033[34m'
+        BOLD = '\033[1m'
+        ENDCOLOR = '\033[0m'
+    else:
+        BOLD = ''
+        GREEN = ''
+        RED = ''
+        BROWN = ''
+        BLUE = ''
+        ENDCOLOR = ''
+
+    ret = 0
+    for checker in config.checkers:
+        if config.list_only:
+            ret = checker.ListTypes()
+        else:
+            ret = checker.Run(file, verbose)
+            unclassified += checker.unclassified
+        if ret and error:
+            break
+
+    if not error and not (error_on_red and unclassified > 0):
+        ret = 0
+except CheckError, e:
+    print "  ** %s: %s" % (progname, str(e))
+    ret = 22
+except KeyboardInterrupt:
+    if verbose:
+        print "  ** %s: Interrupted by user" % progname
+    ret = 4
+
+exit(ret)
diff --git a/scripts/runchecks.cfg b/scripts/runchecks.cfg
new file mode 100644
index 0000000..c0b12cf
--- /dev/null
+++ b/scripts/runchecks.cfg
@@ -0,0 +1,63 @@
+checker checkpatch
+addflags  --quiet --show-types --strict --emacs
+line_len 110
+
+checker sparse
+addflags -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ -Wsparse-all
+cflags
+
+#	Name		Regular expression for matching in checker output
+typedef DECL		symbol '.*' was not declared. Should it be static\?
+typedef SHADOW		symbol '\w+' shadows an earlier one\n.*originally declared here
+typedef TYPESIGN	incorrect type in argument \d+ \(different signedness\)\n.*expected\n.*got
+typedef RETURN_VOID	returning void-valued expression
+typedef SIZEOF_BOOL	expression using sizeof bool
+typedef CONTEXT		context imbalance in '.*'
+typedef MEMCPY_MAX_COUNT \w+ with byte count of
+typedef CAST_TO_AS	cast adds address space to expression
+typedef ADDRESS_SPACE	incorrect type in .* \(different address spaces\)\n.*expected\n.*got
+typedef PTR_INHERIT	incorrect type in .* \(different base types\)\n.*expected\n.*got
+typedef PTR_SUBTRACTION_BLOWS potentially expensive pointer subtraction
+typedef VLA		Variable length array is used
+typedef OVERFLOW	constant [x\dA-F]+ is so big it is \w+
+typedef TAUTOLOGICAL_COMPARE self-comparison always evaluates to (true|false)
+typedef NON_POINTER_NULL Using plain integer as NULL pointer
+typedef BOOL_CAST_RESTRICTED restricted \w+ degrades to integer
+typedef TYPESIGN	incorrect type in .* \(different signedness\)\n.*expected\n.*got
+typedef FUNCTION_REDECL symbol '.*' redeclared with different type \(originally declared at
+typedef COND_ADDRESS_ARRAY the address of an array will always evaluate as true
+typedef BITWISE 	cast (to|from) restricted
+
+# Type names invented here - not maskable from sparse?
+typedef NO_DEREF	dereference of noderef expression
+typedef ARG_TYPE_MOD	incorrect type in .* \(different modifiers\)\n.*expected\n.*got
+typedef ARG_TYPE_COMP	incorrect type in .* \(incompatible .*\(.*\)\)\n.*expected\n.*got
+typedef ARG_AS_COMP	incompatible types in comparison expression \(different address spaces\)
+typedef CMP_TYPE	incompatible types in comparison expression \(different base types\)
+typedef SPARSE_OFF	"Sparse checking disabled for this file"
+typedef CAST_TRUNC	cast truncates bits from constant value
+typedef CAST_FROM_AS	cast removes address space of expression
+typedef EXT_LINK_DEF	function '\w+' with external linkage has definition
+typedef FUNC_ARITH	arithmetics on pointers to functions
+typedef CALL_NO_TYPE	call with no type!
+typedef FUNC_SUB	subtraction of functions\? Share your drugs
+typedef STRING_CONCAT	trying to concatenate \d+-character string \(\d+ bytes max\)
+typedef INARG_DIRECTIVE directive in argument list
+typedef NONSCALAR_CAST  cast (to|from) non-scalar
+
+checker checkdoc
+typedef PARAM_DESC	No description found for parameter
+typedef X_PARAM    	Excess function parameter
+typedef X_STRUCT	Excess struct member
+typedef FUN_PROTO    	cannot understand function prototype
+typedef DOC_FORMAT   	Incorrect use of kernel-doc format
+typedef BAD_LINE	bad line
+typedef AMBIGUOUS	Cannot understand.*\n on line
+typedef BOGUS_STRUCT	Cannot parse struct or union
+typedef DUPL_SEC	duplicate section name
+
+checker coccicheck
+cflags
+
+run sparse checkpatch checkdoc
+#run all
diff --git a/scripts/runchecks_help.txt b/scripts/runchecks_help.txt
new file mode 100644
index 0000000..aa3b69a
--- /dev/null
+++ b/scripts/runchecks_help.txt
@@ -0,0 +1,43 @@
+Usage: runchecks  [<options>] c_file [-- <c_parameters>]
+  - run code checkers in a conformant way.
+
+Options:
+  -h|--help	List this text
+  --list	List the different configured checkers and the list of interpreted check
+  		types for each of them.
+  --		Separator between parameters to runchecks and compiler parameters to be
+		passed directly to the checkers.
+  --run:[checker1[,checker2..]|all]
+  		Override the default set of checkers to be run for each source file. By
+		default the checkers to run will be the intersection of the checkers
+		configured by ``run`` commands in the configuration file and the
+		checkers that is actually available on the machine. Use 'all'
+		to run all the configured checkers.
+  --color	Use coloring in the error and warning output. In this mode
+		output from checkers that are supported by typedefs but not
+		captured by any such will be highlighted in red to make it
+		easy to detect that a typedef rule is missing. See -t below.
+  -f		Force mode: force runchecks to run a full run in directories/trees
+		where runchecks does not find a	runchecks.cfg file. The default
+		behaviour is to skip running checkers in directories/trees
+		where no matching runchecks.cfg file is found either in the
+		source file directory or above.
+  -n		Ignore all runchecks.cfg files except the one in scripts,
+		which are used for basic runchecks configuration. This allows
+		an easy way to run a "bare" version of checking where all
+		issues are reported, even those intended to be suppressed.
+		Implicitly enables force mode.
+  -w		Behave as if 0 on exit from all checkers. Normally
+		runchecks will fail on the first checker to produce errors or
+		warnings, in fact anything that produces not suppressed
+		output on stderr. This is to make it easy to work interactively,
+		avoiding overlooking anything, but sometimes it is useful to
+		be able to produce a full report of status.
+  -t		Typedef setup mode: For checkers where runchecks enable typedefs:
+  		Behaves as -w except for stderr output that is not captured
+		by any typedefs. This is a convenience mode while
+		fixing/improving typedef setup. Use with --color to get red
+		output for the statements to capture with new typedefs.
+  -v		Verbose output. Also enabled if called from make with V=1,
+		but it is useful to be able to only enable verbose mode for runchecks.
+  -d		Debugging output - more verbose.
-- 
git-series 0.9.1

[PATCH v2 2/5] Documentation: Add doc for runchecks, a checker runner

[Knut Omang]

The runchecks program unifies configuration, processing
and output for multiple checker tools to make them
all run as part of the C=1 or C=2 option to make.

Currently with full support and unified behaviour for
sparse:     sparse
checkpatch: scripts/checkpatch.pl
checkdoc:   kernel-doc -none

In principle supported but not unified in output(yet):
coccinelle: scripts/coccicheck

Introduces a new documentation section titled
"Makefile support for running checkers"

Also updates documentation for the make C= option
in some other doc files, as the behaviour has
been changed to be less sparse specific and more
generic. The coccinelle documentation also had the
behaviour of C=1 and C=2 swapped.

Signed-off-by: Knut Omang <knut.omang@oracle.com>
Reviewed-by: Håkon Bugge <haakon.bugge@oracle.com>
Reviewed-by: Åsmund Østvold <asmund.ostvold@oracle.com>
Reviewed-by: John Haxby <john.haxby@oracle.com>
---
 Documentation/dev-tools/coccinelle.rst |  12 +-
 Documentation/dev-tools/index.rst      |   1 +-
 Documentation/dev-tools/runchecks.rst  | 215 ++++++++++++++++++++++++++-
 Documentation/dev-tools/sparse.rst     |  30 +++-
 Documentation/kbuild/kbuild.txt        |   9 +-
 5 files changed, 257 insertions(+), 10 deletions(-)
 create mode 100644 Documentation/dev-tools/runchecks.rst

diff --git a/Documentation/dev-tools/coccinelle.rst b/Documentation/dev-tools/coccinelle.rst
index 94f41c2..c98cc44 100644
--- a/Documentation/dev-tools/coccinelle.rst
+++ b/Documentation/dev-tools/coccinelle.rst
@@ -157,17 +157,19 @@ For example, to check drivers/net/wireless/ one may write::
 
     make coccicheck M=drivers/net/wireless/
 
-To apply Coccinelle on a file basis, instead of a directory basis, the
-following command may be used::
+To apply Coccinelle as the only checker on a file basis,
+instead of a directory basis, the following command may be used::
 
-    make C=1 CHECK="scripts/coccicheck"
+    make C=2 CF="--run:coccicheck"
 
-To check only newly edited code, use the value 2 for the C flag, i.e.::
+To check only newly edited code, use the value 1 for the C flag, i.e.::
 
-    make C=2 CHECK="scripts/coccicheck"
+    make C=1 CF="--run:coccicheck"
 
 In these modes, which works on a file basis, there is no information
 about semantic patches displayed, and no commit message proposed.
+For more information about options in this calling mode, see
+Documentation/dev-tools/runchecks.rst .
 
 This runs every semantic patch in scripts/coccinelle by default. The
 COCCI variable may additionally be used to only apply a single
diff --git a/Documentation/dev-tools/index.rst b/Documentation/dev-tools/index.rst
index e313925..cb4506d 100644
--- a/Documentation/dev-tools/index.rst
+++ b/Documentation/dev-tools/index.rst
@@ -16,6 +16,7 @@ whole; patches welcome!
 
    coccinelle
    sparse
+   runchecks
    kcov
    gcov
    kasan
diff --git a/Documentation/dev-tools/runchecks.rst b/Documentation/dev-tools/runchecks.rst
new file mode 100644
index 0000000..b25b3de
--- /dev/null
+++ b/Documentation/dev-tools/runchecks.rst
@@ -0,0 +1,215 @@
+.. Copyright 2017 Knut Omang <knut.omang@oracle.com>
+
+Makefile support for running checkers
+=====================================
+
+Tools like sparse, coccinelle and scripts/checkpatch.pl is able to detect a
+lot of syntactic and semantic issues with the code, and is also constantly
+evolving and detecting more. In an ideal world, all source files should
+adhere to whatever rules imposed by checkpatch.pl and sparse etc. with all
+bells and whistles enabled, in a way that these checkers can be run as a reflex
+by developers (and by bots) from the top level Makefile for every changing
+source file. In the real world however there's a number of challenges:
+
+* Sometimes there are valid reasons for accepting violations of a checker
+  rule, even if that rule is a sensible one in the general case.
+* Some subsystems have different restrictions and requirements.
+  (Ideally, the number of subsystems with differing restrictions and
+  requirements will diminish over time.)
+* Similarly, the kernel contains a lot of code that predates the tools, or at
+  least some of the newer rules, and we would like these tools to evolve without
+  requiring the need to fix all issues detected with it in the same commit.
+  We also want to accommodate new tools, so that each new tool does not
+  have to reinvent it's own mechanism for running checks.
+* On the other hand, we want to make sure that files that are clean
+  (to some well defined extent, such as passing checkpatch or sparse
+  with checks only for certain important types of issues) keep being so.
+
+This is the purpose of ``scripts/runchecks``.
+
+The ``runchecks`` program looks for files named ``runchecks.cfg`` in the
+``scripts`` directory, then in the directory hierarchy of the source file,
+starting where that source file is located, searching upwards. If at least
+one such file exists in the source tree, ``runchecks`` parses a set of
+rules from it, and use them to determine how to invoke a set of individual
+checker tools for a particular file. The kernel Makefile system supports using
+this feature as an integrated part of compiling the code, using the
+``C={1,2}`` option. With::
+
+	make C=1
+
+runchecks will be invoked if the file needs to be recompiled. With ::
+
+	make C=2
+
+runchecks will be invoked for all source files, even if they do not need
+recompiling. Based on the configuration, ``runchecks`` will invoke one or
+more checkers. The number and types of checkers to run are configurable and
+can also be selected on the command line::
+
+	make C=2 CF="--run:sparse,checkpatch"
+
+If only one checker is run, any parameter that is not recognized by
+runchecks itself will be forwarded to the checker. If more than one checker
+is enabled, parameters can be forwarded to a specific checker by means of
+this syntax::
+
+	make C=2 CF="--to-checkpatch:--terse"
+
+A comma separated list of parameters can be supplied if necessary.
+
+Supported syntax of the runchecks.cfg configuration file
+--------------------------------------------------------
+
+The runchecks configuration file chain can be used to set policies and "rein in"
+checker errors piece by piece for a particular subsystem or driver. It can
+also be used to mitigate and extend checkers that does not support
+selective suppression of all it's checks.
+
+Two classes of configuration is available. The first class is configuration
+that defines what checkers are enabled, and also allow a limited form of
+pattern matching to extend checking, to mitigate for checks that cannot be
+selectively disabled by command line parameters to the underlying tool
+itself. This type of configuration should go into the first accessed
+configuration file, and has been preconfigured for the currently supported
+checkers in ``scripts/runchecks.cfg``. The second class is the features for
+configuring the output of the checkers by selectively suppressing checks on
+a per file or per check basis. These typically goes in the source tree in
+the directory of the source file or above. Some of the syntax is generic
+and some is only supported by some checkers.
+
+For the first class of configuration the following syntax is supported::
+
+	# comments
+	checker checkpatch [command]
+	addflags <list of extra flags and parameters>
+	cflags
+	typedef NAME <regular expression>
+	run [checker list|all]
+
+The ``checker`` command switches ``runchecks``'s attention to a particular
+checker. The following commands until the next ``checker`` statement
+applies to that particular checker. The first occurrence of ``checker``
+also serves as a potentially defining operation, if the checker name
+has not been preconfigured. In that case, a second parameter can be used
+to provide the name of the command used to run the checker.
+A full checker integration into runchecks will typically require some
+additions to runchecks, and will then have been preconfigured,
+but simple checkers might just be configured on the fly.
+
+The ``addflags`` command incrementally adds more flags and parameters to
+the command line used to invoke the checker. This applies to all
+invocations of the checker from runchecks.
+The ``cflags`` command tells to forward all the flags and options passed to
+the compiler invocation to the checker. The default is to suppress these
+parameters when invoking the checker.
+
+The ``typedef`` command adds ``NAME`` and associates it with the given
+regular expression. This expression is used to match against standard error
+output from the checker and ``NAME`` can be used as a new named check that
+runchecks understands and that can be used with checker supported names
+below to selectively suppress that particular set of warning or error
+messages. This is useful to handle output checks for which the underlying
+checker is does not provide any suppression. Check type namespaces are
+separate for the individual checkers. You can list the state of the built in and
+configured checker and check types with::
+
+	scripts/runchecks --list
+
+The checker implementations of the ``typedef`` command also allows
+runchecks to perform some unification of output by rewriting the output
+lines, adding optional color support, and use of the new type names in the
+error output, to ease the process of updating the runchecks.cfg files.
+Having a unified representation of the error output also makes it much
+easier to do statistics or other operations on top of an aggregated output
+from several checkers.
+
+For the second class of configuration the following syntax is supported::
+
+	# comments
+	checker checker_name
+	line_len <n>
+	except check_type [files ...]
+	pervasive check_type1 [check_type2 ...]
+
+The ``line_len`` directive defines the upper bound of characters per line
+tolerated in this directory. Currently only ``checkpatch`` supports this
+command.  The ``except`` directive takes a check type such as for example
+``MACRO_ARG_REUSE``, and a set of files that should not be subject to this
+particular check type. The ``pervasive`` command disables the listed types
+of checks for all the files in the subtree.  The ``except`` and
+``pervasive`` directives can be used cumulatively to add more exceptions.
+
+Running checker programs from make
+----------------------------------
+
+You can run checkpatch subject to rules defined in ``runcheck.cfg`` in the
+directory of the source file by using "make P=1" to run checkpatch on all files
+that gets recompiled, or "make P=2" to run checkpatch on all source files.
+
+A make variable ``CF`` allows passing additional parameters to
+``runchecks``. You can for instance use::
+
+	make C=2 CF="--run:checkpatch --fix-inplace"
+
+to run only the ``checkpatch`` checker, and to have checkpatch trying to fix
+issues it finds - *make sure you have a clean git tree and carefully review
+the output afterwards!* Combine this with selectively enabling of types of
+errors via changes under ``checker checkpatch`` to the local
+``runchecks.cfg``, and you can focus on fixing up errors subsystem or
+driver by driver on a type by type basis.
+
+By default runchecks will skip all files if a ``runchecks.cfg`` file cannot
+be found in the directory of the file or in the tree above.  This is to
+allow builds with ``C=2`` to pass even for subsystems that has not yet done
+anything to rein in checker errors. At some point when all subsystems and
+drivers either have fixed all checker errors or added proper
+``runchecks.cfg`` files, this can be changed.
+
+To force runchecks to run a full run in directories/trees where runchecks
+does not find  a ``runchecks.cfg`` file as well, use::
+
+	make C=2 CF="-f"
+
+If you like to see all the warnings and errors produced by the checkers, ignoring
+any runchecks.cfg files except the one under ``scripts``, you can use::
+
+	make C=2 CF="-n"
+
+or for a specific module directory::
+
+	make C=2 M=drivers/infiniband/core CF="--color -n -w"
+
+with the -w option to ``runchecks`` to suppress errors from any of the
+checkers and just continue on, and the ``--color`` option to present errors
+with colors where supported.
+
+Ever tightening checker rules
+-----------------------------
+
+Commit the changes to the relevant ``runchecks.cfg`` together with the code
+changes that fixes a particular type of issue, this will allow automatic
+checker running by default. This way we can ensure that new errors of that
+particular type do not inadvertently sneak in again! This can be done at
+any subsystem or module maintainer's discretion and at the right time
+without having to do it all at the same time.
+
+Before submitting your changes, verify that a full "make C=2" passes with no
+errors.
+
+Extending and improving checker support in ``runchecks``
+--------------------------------------------------------
+
+The runchecks program has been written with extensibility in mind.
+If the checker starts it's reporting lines with filename:lineno, there's a
+good chance that a new checker can simply be added by adding::
+
+	checker mychecker path_to_mychecker
+
+to ``scripts/runchecks.cfg`` and suitable ``typedef`` expressions to provide
+selective suppressions of output, however it is likely that some quirks are
+needed to make the new checker behave similar to the others, and to support
+the full set of features, such as the ``--list`` option. This is done by
+implementing a new subclass of the Checker class in ``runchecks``. This is the way
+all the available default supported checkers are implemented, and those
+relatively lean implementations could serve as examples.
diff --git a/Documentation/dev-tools/sparse.rst b/Documentation/dev-tools/sparse.rst
index 78aa00a..e3e8b27 100644
--- a/Documentation/dev-tools/sparse.rst
+++ b/Documentation/dev-tools/sparse.rst
@@ -101,5 +101,31 @@ recompiled, or use "make C=2" to run sparse on the files whether they need to
 be recompiled or not.  The latter is a fast way to check the whole tree if you
 have already built it.
 
-The optional make variable CF can be used to pass arguments to sparse.  The
-build system passes -Wbitwise to sparse automatically.
+The "make C={1,2}" form of kernel make indirectly calls sparse via "runchecks",
+which dependent on configuration and command line options may dispatch calls to
+other checkers in addition to sparse. Details on how this works is covered
+in Documentation/dev-tools/runchecks.rst .
+
+The optional make variable CF can be used to pass arguments to runchecks for dispatch
+to sparse. If sparse is the only tool enabled, any option not recognized by
+runchecks will be forwarded to sparse. If more than one tool is active, you must
+add the parameters you want sparse to get as a comma separated list prefixed by
+``--to-sparse:``. If you want sparse to be the only checker run, and you want
+some nice colored output, you can specify this as::
+
+	make C=2 CF="--run:sparse --color"
+
+This will cause sparse to be called for all files which are supported by a valid
+runchecks configuration (again see Documentation/dev-tools/runchecks.rst for
+details). If you want to run sparse on all files and ignore any missing
+configuration files(s), just add ``-n`` to the list of options passed to
+runchecks. This will cause runchecks to call sparse with all errors enabled for
+all files even if no valid configuration is found in the tree for the source files.
+
+By default "runchecks" is set to enable all sparse errors, but you can
+configure what checks to be applied by sparse on a per file or per subsystem
+basis. With the above invocation, make will fail and stop on the first file
+encountered with sparse errors or warnings in it. If you want to continue
+anyway, you can use::
+
+	make C=2 CF="--run:sparse --color -w"
diff --git a/Documentation/kbuild/kbuild.txt b/Documentation/kbuild/kbuild.txt
index ac2363e..260e688 100644
--- a/Documentation/kbuild/kbuild.txt
+++ b/Documentation/kbuild/kbuild.txt
@@ -103,10 +103,13 @@ CROSS_COMPILE is also used for ccache in some setups.
 
 CF
 --------------------------------------------------
-Additional options for sparse.
-CF is often used on the command-line like this:
+Additional options for runchecks, the generic checker runner.
+CF is often used on the command-line for instance like this:
 
-    make CF=-Wbitwise C=2
+    make C=2 CF="--run:sparse --color -w"
+
+to run the sparse tool only, and to use colored output and continue on warnings
+or errors.
 
 INSTALL_PATH
 --------------------------------------------------
-- 
git-series 0.9.1

Re: [PATCH v2 2/5] Documentation: Add doc for runchecks, a checker runner

[Julia Lawall]

> diff --git a/Documentation/dev-tools/runchecks.rst b/Documentation/dev-tools/runchecks.rst
> new file mode 100644
> index 0000000..b25b3de
> --- /dev/null
> +++ b/Documentation/dev-tools/runchecks.rst
> @@ -0,0 +1,215 @@
> +.. Copyright 2017 Knut Omang <knut.omang@oracle.com>
> +
> +Makefile support for running checkers
> +=====================================
> +
> +Tools like sparse, coccinelle and scripts/checkpatch.pl is able to detect a

is -> are

> +lot of syntactic and semantic issues with the code, and is also constantly

is -> are

> +evolving and detecting more. In an ideal world, all source files should
> +adhere to whatever rules imposed by checkpatch.pl and sparse etc. with all
> +bells and whistles enabled, in a way that these checkers can be run as a reflex
> +by developers (and by bots) from the top level Makefile for every changing
> +source file. In the real world however there's a number of challenges:
> +
> +* Sometimes there are valid reasons for accepting violations of a checker
> +  rule, even if that rule is a sensible one in the general case.
> +* Some subsystems have different restrictions and requirements.
> +  (Ideally, the number of subsystems with differing restrictions and
> +  requirements will diminish over time.)
> +* Similarly, the kernel contains a lot of code that predates the tools, or at
> +  least some of the newer rules, and we would like these tools to evolve without
> +  requiring the need to fix all issues detected with it in the same commit.
> +  We also want to accommodate new tools, so that each new tool does not
> +  have to reinvent it's own mechanism for running checks.

it's -> its

> +* On the other hand, we want to make sure that files that are clean
> +  (to some well defined extent, such as passing checkpatch or sparse
> +  with checks only for certain important types of issues) keep being so.
> +
> +This is the purpose of ``scripts/runchecks``.
> +
> +The ``runchecks`` program looks for files named ``runchecks.cfg`` in the
> +``scripts`` directory, then in the directory hierarchy of the source file,
> +starting where that source file is located, searching upwards. If at least
> +one such file exists in the source tree, ``runchecks`` parses a set of
> +rules from it, and use them to determine how to invoke a set of individual
> +checker tools for a particular file. The kernel Makefile system supports using
> +this feature as an integrated part of compiling the code, using the
> +``C={1,2}`` option. With::
> +
> +	make C=1
> +
> +runchecks will be invoked if the file needs to be recompiled. With ::
> +
> +	make C=2
> +
> +runchecks will be invoked for all source files, even if they do not need
> +recompiling. Based on the configuration, ``runchecks`` will invoke one or
> +more checkers. The number and types of checkers to run are configurable and
> +can also be selected on the command line::
> +
> +	make C=2 CF="--run:sparse,checkpatch"
> +
> +If only one checker is run, any parameter that is not recognized by
> +runchecks itself will be forwarded to the checker. If more than one checker
> +is enabled, parameters can be forwarded to a specific checker by means of
> +this syntax::
> +
> +	make C=2 CF="--to-checkpatch:--terse"
> +
> +A comma separated list of parameters can be supplied if necessary.
> +
> +Supported syntax of the runchecks.cfg configuration file
> +--------------------------------------------------------
> +
> +The runchecks configuration file chain can be used to set policies and "rein in"
> +checker errors piece by piece for a particular subsystem or driver. It can
> +also be used to mitigate and extend checkers that does not support

does -> do

> +selective suppression of all it's checks.

it's -> its

> +Two classes of configuration is available. The first class is configuration
> +that defines what checkers are enabled, and also allow a limited form of
> +pattern matching to extend checking, to mitigate for checks that cannot be
> +selectively disabled by command line parameters to the underlying tool
> +itself. This type of configuration should go into the first accessed
> +configuration file, and has been preconfigured for the currently supported
> +checkers in ``scripts/runchecks.cfg``. The second class is the features for
> +configuring the output of the checkers by selectively suppressing checks on
> +a per file or per check basis. These typically goes in the source tree in

goes -> go

> +the directory of the source file or above. Some of the syntax is generic
> +and some is only supported by some checkers.
> +
> +For the first class of configuration the following syntax is supported::
> +
> +	# comments
> +	checker checkpatch [command]
> +	addflags <list of extra flags and parameters>
> +	cflags
> +	typedef NAME <regular expression>
> +	run [checker list|all]
> +
> +The ``checker`` command switches ``runchecks``'s attention to a particular
> +checker. The following commands until the next ``checker`` statement
> +applies to that particular checker. The first occurrence of ``checker``

applies -> apply

> +also serves as a potentially defining operation, if the checker name
> +has not been preconfigured. In that case, a second parameter can be used
> +to provide the name of the command used to run the checker.
> +A full checker integration into runchecks will typically require some
> +additions to runchecks, and will then have been preconfigured,
> +but simple checkers might just be configured on the fly.
> +
> +The ``addflags`` command incrementally adds more flags and parameters to
> +the command line used to invoke the checker. This applies to all
> +invocations of the checker from runchecks.
> +The ``cflags`` command tells to forward all the flags and options passed to
> +the compiler invocation to the checker. The default is to suppress these
> +parameters when invoking the checker.
> +
> +The ``typedef`` command adds ``NAME`` and associates it with the given
> +regular expression. This expression is used to match against standard error
> +output from the checker and ``NAME`` can be used as a new named check that
> +runchecks understands and that can be used with checker supported names
> +below to selectively suppress that particular set of warning or error
> +messages. This is useful to handle output checks for which the underlying
> +checker is does not provide any suppression. Check type namespaces are
> +separate for the individual checkers. You can list the state of the built in and
> +configured checker and check types with::
> +
> +	scripts/runchecks --list
> +
> +The checker implementations of the ``typedef`` command also allows
> +runchecks to perform some unification of output by rewriting the output
> +lines, adding optional color support, and use of the new type names in the
> +error output, to ease the process of updating the runchecks.cfg files.
> +Having a unified representation of the error output also makes it much
> +easier to do statistics or other operations on top of an aggregated output
> +from several checkers.
> +
> +For the second class of configuration the following syntax is supported::
> +
> +	# comments
> +	checker checker_name
> +	line_len <n>
> +	except check_type [files ...]
> +	pervasive check_type1 [check_type2 ...]
> +
> +The ``line_len`` directive defines the upper bound of characters per line
> +tolerated in this directory. Currently only ``checkpatch`` supports this
> +command.  The ``except`` directive takes a check type such as for example
> +``MACRO_ARG_REUSE``, and a set of files that should not be subject to this
> +particular check type. The ``pervasive`` command disables the listed types
> +of checks for all the files in the subtree.  The ``except`` and
> +``pervasive`` directives can be used cumulatively to add more exceptions.
> +
> +Running checker programs from make
> +----------------------------------
> +
> +You can run checkpatch subject to rules defined in ``runcheck.cfg`` in the
> +directory of the source file by using "make P=1" to run checkpatch on all files
> +that gets recompiled, or "make P=2" to run checkpatch on all source files.
> +
> +A make variable ``CF`` allows passing additional parameters to
> +``runchecks``. You can for instance use::
> +
> +	make C=2 CF="--run:checkpatch --fix-inplace"
> +
> +to run only the ``checkpatch`` checker, and to have checkpatch trying to fix


trying -> try

> +issues it finds - *make sure you have a clean git tree and carefully review
> +the output afterwards!* Combine this with selectively enabling of types of
> +errors via changes under ``checker checkpatch`` to the local
> +``runchecks.cfg``, and you can focus on fixing up errors subsystem or
> +driver by driver on a type by type basis.
> +
> +By default runchecks will skip all files if a ``runchecks.cfg`` file cannot
> +be found in the directory of the file or in the tree above.  This is to
> +allow builds with ``C=2`` to pass even for subsystems that has not yet done
> +anything to rein in checker errors. At some point when all subsystems and
> +drivers either have fixed all checker errors or added proper
> +``runchecks.cfg`` files, this can be changed.
> +
> +To force runchecks to run a full run in directories/trees where runchecks
> +does not find  a ``runchecks.cfg`` file as well, use::
> +
> +	make C=2 CF="-f"
> +
> +If you like to see all the warnings and errors produced by the checkers, ignoring
> +any runchecks.cfg files except the one under ``scripts``, you can use::
> +
> +	make C=2 CF="-n"
> +
> +or for a specific module directory::
> +
> +	make C=2 M=drivers/infiniband/core CF="--color -n -w"
> +
> +with the -w option to ``runchecks`` to suppress errors from any of the
> +checkers and just continue on, and the ``--color`` option to present errors
> +with colors where supported.
> +
> +Ever tightening checker rules
> +-----------------------------
> +
> +Commit the changes to the relevant ``runchecks.cfg`` together with the code
> +changes that fixes a particular type of issue, this will allow automatic
> +checker running by default. This way we can ensure that new errors of that
> +particular type do not inadvertently sneak in again! This can be done at
> +any subsystem or module maintainer's discretion and at the right time
> +without having to do it all at the same time.
> +
> +Before submitting your changes, verify that a full "make C=2" passes with no
> +errors.
> +
> +Extending and improving checker support in ``runchecks``
> +--------------------------------------------------------
> +
> +The runchecks program has been written with extensibility in mind.
> +If the checker starts it's reporting lines with filename:lineno, there's a

it's -> its

> +good chance that a new checker can simply be added by adding::
> +
> +	checker mychecker path_to_mychecker
> +
> +to ``scripts/runchecks.cfg`` and suitable ``typedef`` expressions to provide
> +selective suppressions of output, however it is likely that some quirks are
> +needed to make the new checker behave similar to the others, and to support

similar -> similarly

julia

> +the full set of features, such as the ``--list`` option. This is done by
> +implementing a new subclass of the Checker class in ``runchecks``. This is the way
> +all the available default supported checkers are implemented, and those
> +relatively lean implementations could serve as examples.
> diff --git a/Documentation/dev-tools/sparse.rst b/Documentation/dev-tools/sparse.rst
> index 78aa00a..e3e8b27 100644
> --- a/Documentation/dev-tools/sparse.rst
> +++ b/Documentation/dev-tools/sparse.rst
> @@ -101,5 +101,31 @@ recompiled, or use "make C=2" to run sparse on the files whether they need to
>  be recompiled or not.  The latter is a fast way to check the whole tree if you
>  have already built it.
>
> -The optional make variable CF can be used to pass arguments to sparse.  The
> -build system passes -Wbitwise to sparse automatically.
> +The "make C={1,2}" form of kernel make indirectly calls sparse via "runchecks",
> +which dependent on configuration and command line options may dispatch calls to
> +other checkers in addition to sparse. Details on how this works is covered
> +in Documentation/dev-tools/runchecks.rst .
> +
> +The optional make variable CF can be used to pass arguments to runchecks for dispatch
> +to sparse. If sparse is the only tool enabled, any option not recognized by
> +runchecks will be forwarded to sparse. If more than one tool is active, you must
> +add the parameters you want sparse to get as a comma separated list prefixed by
> +``--to-sparse:``. If you want sparse to be the only checker run, and you want
> +some nice colored output, you can specify this as::
> +
> +	make C=2 CF="--run:sparse --color"
> +
> +This will cause sparse to be called for all files which are supported by a valid
> +runchecks configuration (again see Documentation/dev-tools/runchecks.rst for
> +details). If you want to run sparse on all files and ignore any missing
> +configuration files(s), just add ``-n`` to the list of options passed to
> +runchecks. This will cause runchecks to call sparse with all errors enabled for
> +all files even if no valid configuration is found in the tree for the source files.
> +
> +By default "runchecks" is set to enable all sparse errors, but you can
> +configure what checks to be applied by sparse on a per file or per subsystem
> +basis. With the above invocation, make will fail and stop on the first file
> +encountered with sparse errors or warnings in it. If you want to continue
> +anyway, you can use::
> +
> +	make C=2 CF="--run:sparse --color -w"
> diff --git a/Documentation/kbuild/kbuild.txt b/Documentation/kbuild/kbuild.txt
> index ac2363e..260e688 100644
> --- a/Documentation/kbuild/kbuild.txt
> +++ b/Documentation/kbuild/kbuild.txt
> @@ -103,10 +103,13 @@ CROSS_COMPILE is also used for ccache in some setups.
>
>  CF
>  --------------------------------------------------
> -Additional options for sparse.
> -CF is often used on the command-line like this:
> +Additional options for runchecks, the generic checker runner.
> +CF is often used on the command-line for instance like this:
>
> -    make CF=-Wbitwise C=2
> +    make C=2 CF="--run:sparse --color -w"
> +
> +to run the sparse tool only, and to use colored output and continue on warnings
> +or errors.
>
>  INSTALL_PATH
>  --------------------------------------------------
> --
> git-series 0.9.1
>

Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Joe Perches]

On Sat, 2017-12-16 at 15:42 +0100, Knut Omang wrote:
> This patch series implements features to make it easier to run checkers on the
> entire kernel as part of automatic and developer testing.

This seems like a useful and reasonable series, thanks.
Do please take Julia's grammar updates.

How is this series to be applied?

Re: [PATCH v2 2/5] Documentation: Add doc for runchecks, a checker runner

[Knut Omang]

On Sat, 2017-12-16 at 16:08 +0100, Julia Lawall wrote:
> > diff --git a/Documentation/dev-tools/runchecks.rst b/Documentation/dev-
> tools/runchecks.rst
> > new file mode 100644
> > index 0000000..b25b3de
> > --- /dev/null
> > +++ b/Documentation/dev-tools/runchecks.rst
> > @@ -0,0 +1,215 @@
> > +.. Copyright 2017 Knut Omang <knut.omang@oracle.com>
> > +
> > +Makefile support for running checkers
> > +=====================================
> > +
> > +Tools like sparse, coccinelle and scripts/checkpatch.pl is able to detect a
> 
> is -> are
> 
> > +lot of syntactic and semantic issues with the code, and is also constantly
> 
> is -> are
> 
> > +evolving and detecting more. In an ideal world, all source files should
> > +adhere to whatever rules imposed by checkpatch.pl and sparse etc. with all
> > +bells and whistles enabled, in a way that these checkers can be run as a reflex
> > +by developers (and by bots) from the top level Makefile for every changing
> > +source file. In the real world however there's a number of challenges:
> > +
> > +* Sometimes there are valid reasons for accepting violations of a checker
> > +  rule, even if that rule is a sensible one in the general case.
> > +* Some subsystems have different restrictions and requirements.
> > +  (Ideally, the number of subsystems with differing restrictions and
> > +  requirements will diminish over time.)
> > +* Similarly, the kernel contains a lot of code that predates the tools, or at
> > +  least some of the newer rules, and we would like these tools to evolve without
> > +  requiring the need to fix all issues detected with it in the same commit.
> > +  We also want to accommodate new tools, so that each new tool does not
> > +  have to reinvent it's own mechanism for running checks.
> 
> it's -> its
> 
> > +* On the other hand, we want to make sure that files that are clean
> > +  (to some well defined extent, such as passing checkpatch or sparse
> > +  with checks only for certain important types of issues) keep being so.
> > +
> > +This is the purpose of ``scripts/runchecks``.
> > +
> > +The ``runchecks`` program looks for files named ``runchecks.cfg`` in the
> > +``scripts`` directory, then in the directory hierarchy of the source file,
> > +starting where that source file is located, searching upwards. If at least
> > +one such file exists in the source tree, ``runchecks`` parses a set of
> > +rules from it, and use them to determine how to invoke a set of individual
> > +checker tools for a particular file. The kernel Makefile system supports using
> > +this feature as an integrated part of compiling the code, using the
> > +``C={1,2}`` option. With::
> > +
> > +	make C=1
> > +
> > +runchecks will be invoked if the file needs to be recompiled. With ::
> > +
> > +	make C=2
> > +
> > +runchecks will be invoked for all source files, even if they do not need
> > +recompiling. Based on the configuration, ``runchecks`` will invoke one or
> > +more checkers. The number and types of checkers to run are configurable and
> > +can also be selected on the command line::
> > +
> > +	make C=2 CF="--run:sparse,checkpatch"
> > +
> > +If only one checker is run, any parameter that is not recognized by
> > +runchecks itself will be forwarded to the checker. If more than one checker
> > +is enabled, parameters can be forwarded to a specific checker by means of
> > +this syntax::
> > +
> > +	make C=2 CF="--to-checkpatch:--terse"
> > +
> > +A comma separated list of parameters can be supplied if necessary.
> > +
> > +Supported syntax of the runchecks.cfg configuration file
> > +--------------------------------------------------------
> > +
> > +The runchecks configuration file chain can be used to set policies and "rein in"
> > +checker errors piece by piece for a particular subsystem or driver. It can
> > +also be used to mitigate and extend checkers that does not support
> 
> does -> do
> 
> > +selective suppression of all it's checks.
> 
> it's -> its
> 
> > +Two classes of configuration is available. The first class is configuration
> > +that defines what checkers are enabled, and also allow a limited form of
> > +pattern matching to extend checking, to mitigate for checks that cannot be
> > +selectively disabled by command line parameters to the underlying tool
> > +itself. This type of configuration should go into the first accessed
> > +configuration file, and has been preconfigured for the currently supported
> > +checkers in ``scripts/runchecks.cfg``. The second class is the features for
> > +configuring the output of the checkers by selectively suppressing checks on
> > +a per file or per check basis. These typically goes in the source tree in
> 
> goes -> go
> 
> > +the directory of the source file or above. Some of the syntax is generic
> > +and some is only supported by some checkers.
> > +
> > +For the first class of configuration the following syntax is supported::
> > +
> > +	# comments
> > +	checker checkpatch [command]
> > +	addflags <list of extra flags and parameters>
> > +	cflags
> > +	typedef NAME <regular expression>
> > +	run [checker list|all]
> > +
> > +The ``checker`` command switches ``runchecks``'s attention to a particular
> > +checker. The following commands until the next ``checker`` statement
> > +applies to that particular checker. The first occurrence of ``checker``
> 
> applies -> apply
> 
> > +also serves as a potentially defining operation, if the checker name
> > +has not been preconfigured. In that case, a second parameter can be used
> > +to provide the name of the command used to run the checker.
> > +A full checker integration into runchecks will typically require some
> > +additions to runchecks, and will then have been preconfigured,
> > +but simple checkers might just be configured on the fly.
> > +
> > +The ``addflags`` command incrementally adds more flags and parameters to
> > +the command line used to invoke the checker. This applies to all
> > +invocations of the checker from runchecks.
> > +The ``cflags`` command tells to forward all the flags and options passed to
> > +the compiler invocation to the checker. The default is to suppress these
> > +parameters when invoking the checker.
> > +
> > +The ``typedef`` command adds ``NAME`` and associates it with the given
> > +regular expression. This expression is used to match against standard error
> > +output from the checker and ``NAME`` can be used as a new named check that
> > +runchecks understands and that can be used with checker supported names
> > +below to selectively suppress that particular set of warning or error
> > +messages. This is useful to handle output checks for which the underlying
> > +checker is does not provide any suppression. Check type namespaces are
> > +separate for the individual checkers. You can list the state of the built in and
> > +configured checker and check types with::
> > +
> > +	scripts/runchecks --list
> > +
> > +The checker implementations of the ``typedef`` command also allows
> > +runchecks to perform some unification of output by rewriting the output
> > +lines, adding optional color support, and use of the new type names in the
> > +error output, to ease the process of updating the runchecks.cfg files.
> > +Having a unified representation of the error output also makes it much
> > +easier to do statistics or other operations on top of an aggregated output
> > +from several checkers.
> > +
> > +For the second class of configuration the following syntax is supported::
> > +
> > +	# comments
> > +	checker checker_name
> > +	line_len <n>
> > +	except check_type [files ...]
> > +	pervasive check_type1 [check_type2 ...]
> > +
> > +The ``line_len`` directive defines the upper bound of characters per line
> > +tolerated in this directory. Currently only ``checkpatch`` supports this
> > +command.  The ``except`` directive takes a check type such as for example
> > +``MACRO_ARG_REUSE``, and a set of files that should not be subject to this
> > +particular check type. The ``pervasive`` command disables the listed types
> > +of checks for all the files in the subtree.  The ``except`` and
> > +``pervasive`` directives can be used cumulatively to add more exceptions.
> > +
> > +Running checker programs from make
> > +----------------------------------
> > +
> > +You can run checkpatch subject to rules defined in ``runcheck.cfg`` in the
> > +directory of the source file by using "make P=1" to run checkpatch on all files
> > +that gets recompiled, or "make P=2" to run checkpatch on all source files.
> > +
> > +A make variable ``CF`` allows passing additional parameters to
> > +``runchecks``. You can for instance use::
> > +
> > +	make C=2 CF="--run:checkpatch --fix-inplace"
> > +
> > +to run only the ``checkpatch`` checker, and to have checkpatch trying to fix
> 
> 
> trying -> try
> 
> > +issues it finds - *make sure you have a clean git tree and carefully review
> > +the output afterwards!* Combine this with selectively enabling of types of
> > +errors via changes under ``checker checkpatch`` to the local
> > +``runchecks.cfg``, and you can focus on fixing up errors subsystem or
> > +driver by driver on a type by type basis.
> > +
> > +By default runchecks will skip all files if a ``runchecks.cfg`` file cannot
> > +be found in the directory of the file or in the tree above.  This is to
> > +allow builds with ``C=2`` to pass even for subsystems that has not yet done
> > +anything to rein in checker errors. At some point when all subsystems and
> > +drivers either have fixed all checker errors or added proper
> > +``runchecks.cfg`` files, this can be changed.
> > +
> > +To force runchecks to run a full run in directories/trees where runchecks
> > +does not find  a ``runchecks.cfg`` file as well, use::
> > +
> > +	make C=2 CF="-f"
> > +
> > +If you like to see all the warnings and errors produced by the checkers, ignoring
> > +any runchecks.cfg files except the one under ``scripts``, you can use::
> > +
> > +	make C=2 CF="-n"
> > +
> > +or for a specific module directory::
> > +
> > +	make C=2 M=drivers/infiniband/core CF="--color -n -w"
> > +
> > +with the -w option to ``runchecks`` to suppress errors from any of the
> > +checkers and just continue on, and the ``--color`` option to present errors
> > +with colors where supported.
> > +
> > +Ever tightening checker rules
> > +-----------------------------
> > +
> > +Commit the changes to the relevant ``runchecks.cfg`` together with the code
> > +changes that fixes a particular type of issue, this will allow automatic
> > +checker running by default. This way we can ensure that new errors of that
> > +particular type do not inadvertently sneak in again! This can be done at
> > +any subsystem or module maintainer's discretion and at the right time
> > +without having to do it all at the same time.
> > +
> > +Before submitting your changes, verify that a full "make C=2" passes with no
> > +errors.
> > +
> > +Extending and improving checker support in ``runchecks``
> > +--------------------------------------------------------
> > +
> > +The runchecks program has been written with extensibility in mind.
> > +If the checker starts it's reporting lines with filename:lineno, there's a
> 
> it's -> its
> 
> > +good chance that a new checker can simply be added by adding::
> > +
> > +	checker mychecker path_to_mychecker
> > +
> > +to ``scripts/runchecks.cfg`` and suitable ``typedef`` expressions to provide
> > +selective suppressions of output, however it is likely that some quirks are
> > +needed to make the new checker behave similar to the others, and to support
> 
> similar -> similarly
> 
> julia

Thanks for the review - I'll certainly fix these for v3.

Knut

> 
> > +the full set of features, such as the ``--list`` option. This is done by
> > +implementing a new subclass of the Checker class in ``runchecks``. This is the way
> > +all the available default supported checkers are implemented, and those
> > +relatively lean implementations could serve as examples.
> > diff --git a/Documentation/dev-tools/sparse.rst b/Documentation/dev-tools/sparse.rst
> > index 78aa00a..e3e8b27 100644
> > --- a/Documentation/dev-tools/sparse.rst
> > +++ b/Documentation/dev-tools/sparse.rst
> > @@ -101,5 +101,31 @@ recompiled, or use "make C=2" to run sparse on the files whether
> they need to
> >  be recompiled or not.  The latter is a fast way to check the whole tree if you
> >  have already built it.
> >
> > -The optional make variable CF can be used to pass arguments to sparse.  The
> > -build system passes -Wbitwise to sparse automatically.
> > +The "make C={1,2}" form of kernel make indirectly calls sparse via "runchecks",
> > +which dependent on configuration and command line options may dispatch calls to
> > +other checkers in addition to sparse. Details on how this works is covered
> > +in Documentation/dev-tools/runchecks.rst .
> > +
> > +The optional make variable CF can be used to pass arguments to runchecks for dispatch
> > +to sparse. If sparse is the only tool enabled, any option not recognized by
> > +runchecks will be forwarded to sparse. If more than one tool is active, you must
> > +add the parameters you want sparse to get as a comma separated list prefixed by
> > +``--to-sparse:``. If you want sparse to be the only checker run, and you want
> > +some nice colored output, you can specify this as::
> > +
> > +	make C=2 CF="--run:sparse --color"
> > +
> > +This will cause sparse to be called for all files which are supported by a valid
> > +runchecks configuration (again see Documentation/dev-tools/runchecks.rst for
> > +details). If you want to run sparse on all files and ignore any missing
> > +configuration files(s), just add ``-n`` to the list of options passed to
> > +runchecks. This will cause runchecks to call sparse with all errors enabled for
> > +all files even if no valid configuration is found in the tree for the source files.
> > +
> > +By default "runchecks" is set to enable all sparse errors, but you can
> > +configure what checks to be applied by sparse on a per file or per subsystem
> > +basis. With the above invocation, make will fail and stop on the first file
> > +encountered with sparse errors or warnings in it. If you want to continue
> > +anyway, you can use::
> > +
> > +	make C=2 CF="--run:sparse --color -w"
> > diff --git a/Documentation/kbuild/kbuild.txt b/Documentation/kbuild/kbuild.txt
> > index ac2363e..260e688 100644
> > --- a/Documentation/kbuild/kbuild.txt
> > +++ b/Documentation/kbuild/kbuild.txt
> > @@ -103,10 +103,13 @@ CROSS_COMPILE is also used for ccache in some setups.
> >
> >  CF
> >  --------------------------------------------------
> > -Additional options for sparse.
> > -CF is often used on the command-line like this:
> > +Additional options for runchecks, the generic checker runner.
> > +CF is often used on the command-line for instance like this:
> >
> > -    make CF=-Wbitwise C=2
> > +    make C=2 CF="--run:sparse --color -w"
> > +
> > +to run the sparse tool only, and to use colored output and continue on warnings
> > +or errors.
> >
> >  INSTALL_PATH
> >  --------------------------------------------------
> > --
> > git-series 0.9.1
> >

Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Knut Omang]

On Sat, 2017-12-16 at 07:21 -0800, Joe Perches wrote:
> On Sat, 2017-12-16 at 15:42 +0100, Knut Omang wrote:
> > This patch series implements features to make it easier to run checkers on the
> > entire kernel as part of automatic and developer testing.
> 
> This seems like a useful and reasonable series, thanks.

thanks, appreciated,

> Do please take Julia's grammar updates.

will do,

> How is this series to be applied?

I am open for suggestions.

Let's leave patch #3 out for now.

It's safe to apply #1 (and #2) alone.

Patches #4 and #5 only have value once patch #1 is accepted,
and I included them in the set for documentation purposes.

They don't break anything by getting into the RDMA or RDS subsystem 
without patch #1, alternatively they can go together with the set of cleanup patches I
have prepared for each of RDMA and RDS after #1 is in.

Thanks,
Knut

Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Joe Perches]

On Sat, 2017-12-16 at 17:27 +0100, Knut Omang wrote:
> On Sat, 2017-12-16 at 07:21 -0800, Joe Perches wrote:
> > On Sat, 2017-12-16 at 15:42 +0100, Knut Omang wrote:
> > > This patch series implements features to make it easier to run checkers on the
> > > entire kernel as part of automatic and developer testing.
> > 
> > This seems like a useful and reasonable series, thanks.
> 
> thanks, appreciated,
> 
> > Do please take Julia's grammar updates.
> 
> will do,
> 
> > How is this series to be applied?
> 
> I am open for suggestions.
> 
> Let's leave patch #3 out for now.
> 
> It's safe to apply #1 (and #2) alone.

I'd just as soon combine patch #1 and an updated 
patch #2 into a single patch and get that applied
sooner than later.

> They don't break anything by getting into the RDMA or RDS subsystem 
> without patch #1, alternatively they can go together with the set of cleanup patches I
> have prepared for each of RDMA and RDS after #1 is in.

right.

Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Knut Omang]

On Sat, 2017-12-16 at 09:00 -0800, Joe Perches wrote:
> On Sat, 2017-12-16 at 17:27 +0100, Knut Omang wrote:
> > On Sat, 2017-12-16 at 07:21 -0800, Joe Perches wrote:
> > > On Sat, 2017-12-16 at 15:42 +0100, Knut Omang wrote:
> > > > This patch series implements features to make it easier to run checkers on the
> > > > entire kernel as part of automatic and developer testing.
> > > 
> > > This seems like a useful and reasonable series, thanks.
> > 
> > thanks, appreciated,
> > 
> > > Do please take Julia's grammar updates.
> > 
> > will do,
> > 
> > > How is this series to be applied?
> > 
> > I am open for suggestions.
> > 
> > Let's leave patch #3 out for now.
> > 
> > It's safe to apply #1 (and #2) alone.
> 
> I'd just as soon combine patch #1 and an updated 
> patch #2 into a single patch and get that applied
> sooner than later.

Ok, will do that.

> > They don't break anything by getting into the RDMA or RDS subsystem 
> > without patch #1, alternatively they can go together with the set of cleanup patches I
> > have prepared for each of RDMA and RDS after #1 is in.
> 
> right.

Thanks,
Knut

Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Stephen Hemminger]

On Sat, 16 Dec 2017 15:42:25 +0100
Knut Omang <knut.omang@oracle.com> wrote:

> This patch series implements features to make it easier to run checkers on the
> entire kernel as part of automatic and developer testing.
> 
> This is done by replacing the sparse specific setup for the C={1,2} variable
> in the makefiles with setup for running scripts/runchecks, a new program that
> can run any number of different "checkers". The behaviour of runchecks is
> defined by simple "global" configuration in scripts/runchecks.cfg which can be
> extended by local configuration applying to individual files, directories or
> subtrees in the source.
> 
> It also fixes a minor issue with "checkpatch --fix-inplace" found during testing
> (patch #3).
> 
> The runchecks.cfg files are parsed according to this minimal language:
> 
>        # comments
>        # "Global configuration in scripts/runchecks.cfg:
>        checker <name>
>        typedef NAME regex
>        run <list of checkers or "all"
> 
>        # "local" configuration:
>        line_len <n>
>        except checkpatch_type [files ...]
>        pervasive checkpatch_type1 [checkpatch_type2 ...]
> 
> With "make C=2" runchecks first parse the file scripts/runchecks.cfg, then
> look for a file named 'runchecks.cfg' in the same directory as the source file.
> If that file exists, it will be parsed and it's local configuration applied to
> allow suppression on a per checker, per check and per file basis.
> If a "local" configuration does not exist, either in the source directory or
> above, make will simply silently ignore the file.
> 
> The idea is that the community can work to add runchecks.cfg files to
> directories, serving both as documentation and as a way for subsystem
> maintainers to enforce policies and individual tastes as well as TODOs and/or
> priorities, to make it easier for newcomers to contribute in this area. By
> ignoring directories/subtrees without such files, automation can start right
> away as it is trivially possible to run errorless with C=2 for the entire
> kernel.
> 
> For the checker maintainers this should be a benefit as well: new
> or improved checks would generate new errors/warnings. With automatic testing
> for the checkers, these new checks would generate error reports and cause
> builds to fail. Adding the new check a 'pervasive' option at the top level or
> even for specific files, marked with a "new check - please consider fixing" comment
> or similar would make those builds pass while documenting and making the new check
> more visible.
> 
> The patches includes a documentation file with some more details.
> 
> This patch set has evolved from an earlier shell script implementation I made
> as only a wrapper script around checkpatch. That version have been used for a
> number of years on a driver project I worked on where we had automatic checkin
> regression testing. I extended that to also run checkpatch to avoid having to
> clean up frequent unintended whitespace changes and style violations from others...
> 
> I have also tested this version on some directories I am familiar with.  The
> result of that work is available in two patch sets of 10 and 11 patches, but we
> agreed that it would be better to post them as separate patch sets later.
> 
> Those patch sets illustrates how I picture the "flow" from just "reining in" the
> checkpatch detections to actually fixing classes of checkpatch issues one by
> one, while updating the checkpatch.cfg file(s) to have 0 errors or warnings at
> any commit boundary.
> 
> The combined set is available here:
> 
>    git://github.com/knuto/linux.git  branch runchecks
> 
> I only include version 0 of runchecks.cfg in the two directories I
> worked on here as the final two patches. These files both documents where
> the issues are in those two directories, and can be used by the maintainer
> to indicate to potential helpers what to focus on as I have tried to
> illustrate by means of comments.
> 
> Changes from v1:
> -----------------
> Based on feedback, the implementation is completely rewritten and extended.
> Instead of patches to checkpatch, and a sole checkpatch focus, it is now a
> generic solution implemented in python, for any type of checkers, extendable
> through some basic generic functionality, and for special needs by subclassing
> the Checker class in the implementation.
> 
> This implementation fully supports checkpatch, sparse and
> checkdoc == kernel-doc -none, and also has been tested with coccicheck.
> To facilitate the same mechanism of using check types to filter what
> checks to be suppressed, I introduced the concept of "typedefs" which allows
> runchecks to effectively augment the check type space of the checker in cases
> where types either are not available at all (checkdoc) or where only a few
> can be filtered out (sparse)
> 
> With this in place it also became trivial to make the look and feel similar
> for sparse and checkdoc as for checkpatch, with some optional color support
> too, to make fixing issues in the code, the goal of this whole exercise,
> much more pleasant IMHO :-)
> 
> Thanks,
> Knut
> 
> Knut Omang (5):
>   runchecks: Generalize make C={1,2} to support multiple checkers
>   Documentation: Add doc for runchecks, a checker runner
>   checkpatch: Improve --fix-inplace for TABSTOP
>   rds: Add runchecks.cfg for net/rds
>   RDMA/core: Add runchecks.cfg for drivers/infiniband/core
> 
>  Documentation/dev-tools/coccinelle.rst |  12 +-
>  Documentation/dev-tools/index.rst      |   1 +-
>  Documentation/dev-tools/runchecks.rst  | 215 ++++++++-
>  Documentation/dev-tools/sparse.rst     |  30 +-
>  Documentation/kbuild/kbuild.txt        |   9 +-
>  Makefile                               |  23 +-
>  drivers/infiniband/core/runchecks.cfg  |  83 +++-
>  net/rds/runchecks.cfg                  |  76 +++-
>  scripts/Makefile.build                 |   4 +-
>  scripts/checkpatch.pl                  |   2 +-
>  scripts/runchecks                      | 734 ++++++++++++++++++++++++++-
>  scripts/runchecks.cfg                  |  63 ++-
>  scripts/runchecks_help.txt             |  43 ++-
>  13 files changed, 1274 insertions(+), 21 deletions(-)
>  create mode 100644 Documentation/dev-tools/runchecks.rst
>  create mode 100644 drivers/infiniband/core/runchecks.cfg
>  create mode 100644 net/rds/runchecks.cfg
>  create mode 100755 scripts/runchecks
>  create mode 100644 scripts/runchecks.cfg
>  create mode 100644 scripts/runchecks_help.txt
> 
> base-commit: ae64f9bd1d3621b5e60d7363bc20afb46aede215

I like the ability to add more checkers and keep then in the main
upstream tree. But adding overrides for specific subsystems goes against
the policy that all subsystems should be treated equally.

There was discussion at Kernel Summit about how the different
subsystems already have different rules. This appears to be a
way to make that worse.

Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Joe Perches]

On Sat, 2017-12-16 at 09:47 -0800, Stephen Hemminger wrote:
> On Sat, 16 Dec 2017 15:42:25 +0100
> Knut Omang <knut.omang@oracle.com> wrote:
> 
> > This patch series implements features to make it easier to run checkers on the
> > entire kernel as part of automatic and developer testing.
> > 
> > This is done by replacing the sparse specific setup for the C={1,2} variable
> > in the makefiles with setup for running scripts/runchecks, a new program that
> > can run any number of different "checkers". The behaviour of runchecks is
> > defined by simple "global" configuration in scripts/runchecks.cfg which can be
> > extended by local configuration applying to individual files, directories or
> > subtrees in the source.
[]
> I like the ability to add more checkers and keep then in the main
> upstream tree. But adding overrides for specific subsystems goes against
> the policy that all subsystems should be treated equally.
> 
> There was discussion at Kernel Summit about how the different
> subsystems already have different rules. This appears to be a
> way to make that worse.

I think that's OK and somewhat reasonable.

What is perhaps unreasonable is requiring subsystems with
a local specific style to change to some universal style.

see comments like:

https://lkml.org/lkml/2017/12/11/689

Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Knut Omang]

On Sat, 2017-12-16 at 09:47 -0800, Stephen Hemminger wrote:
> On Sat, 16 Dec 2017 15:42:25 +0100
> Knut Omang <knut.omang@oracle.com> wrote:
> 
> > This patch series implements features to make it easier to run checkers on the
> > entire kernel as part of automatic and developer testing.
> > 
> > This is done by replacing the sparse specific setup for the C={1,2} variable
> > in the makefiles with setup for running scripts/runchecks, a new program that
> > can run any number of different "checkers". The behaviour of runchecks is
> > defined by simple "global" configuration in scripts/runchecks.cfg which can be
> > extended by local configuration applying to individual files, directories or
> > subtrees in the source.
> > 
> > It also fixes a minor issue with "checkpatch --fix-inplace" found during testing
> > (patch #3).
> > 
> > The runchecks.cfg files are parsed according to this minimal language:
> > 
> >        # comments
> >        # "Global configuration in scripts/runchecks.cfg:
> >        checker <name>
> >        typedef NAME regex
> >        run <list of checkers or "all"
> > 
> >        # "local" configuration:
> >        line_len <n>
> >        except checkpatch_type [files ...]
> >        pervasive checkpatch_type1 [checkpatch_type2 ...]
> > 
> > With "make C=2" runchecks first parse the file scripts/runchecks.cfg, then
> > look for a file named 'runchecks.cfg' in the same directory as the source file.
> > If that file exists, it will be parsed and it's local configuration applied to
> > allow suppression on a per checker, per check and per file basis.
> > If a "local" configuration does not exist, either in the source directory or
> > above, make will simply silently ignore the file.
> > 
> > The idea is that the community can work to add runchecks.cfg files to
> > directories, serving both as documentation and as a way for subsystem
> > maintainers to enforce policies and individual tastes as well as TODOs and/or
> > priorities, to make it easier for newcomers to contribute in this area. By
> > ignoring directories/subtrees without such files, automation can start right
> > away as it is trivially possible to run errorless with C=2 for the entire
> > kernel.
> > 
> > For the checker maintainers this should be a benefit as well: new
> > or improved checks would generate new errors/warnings. With automatic testing
> > for the checkers, these new checks would generate error reports and cause
> > builds to fail. Adding the new check a 'pervasive' option at the top level or
> > even for specific files, marked with a "new check - please consider fixing" comment
> > or similar would make those builds pass while documenting and making the new check
> > more visible.
> > 
> > The patches includes a documentation file with some more details.
> > 
> > This patch set has evolved from an earlier shell script implementation I made
> > as only a wrapper script around checkpatch. That version have been used for a
> > number of years on a driver project I worked on where we had automatic checkin
> > regression testing. I extended that to also run checkpatch to avoid having to
> > clean up frequent unintended whitespace changes and style violations from others...
> > 
> > I have also tested this version on some directories I am familiar with.  The
> > result of that work is available in two patch sets of 10 and 11 patches, but we
> > agreed that it would be better to post them as separate patch sets later.
> > 
> > Those patch sets illustrates how I picture the "flow" from just "reining in" the
> > checkpatch detections to actually fixing classes of checkpatch issues one by
> > one, while updating the checkpatch.cfg file(s) to have 0 errors or warnings at
> > any commit boundary.
> > 
> > The combined set is available here:
> > 
> >    git://github.com/knuto/linux.git  branch runchecks
> > 
> > I only include version 0 of runchecks.cfg in the two directories I
> > worked on here as the final two patches. These files both documents where
> > the issues are in those two directories, and can be used by the maintainer
> > to indicate to potential helpers what to focus on as I have tried to
> > illustrate by means of comments.
> > 
> > Changes from v1:
> > -----------------
> > Based on feedback, the implementation is completely rewritten and extended.
> > Instead of patches to checkpatch, and a sole checkpatch focus, it is now a
> > generic solution implemented in python, for any type of checkers, extendable
> > through some basic generic functionality, and for special needs by subclassing
> > the Checker class in the implementation.
> > 
> > This implementation fully supports checkpatch, sparse and
> > checkdoc == kernel-doc -none, and also has been tested with coccicheck.
> > To facilitate the same mechanism of using check types to filter what
> > checks to be suppressed, I introduced the concept of "typedefs" which allows
> > runchecks to effectively augment the check type space of the checker in cases
> > where types either are not available at all (checkdoc) or where only a few
> > can be filtered out (sparse)
> > 
> > With this in place it also became trivial to make the look and feel similar
> > for sparse and checkdoc as for checkpatch, with some optional color support
> > too, to make fixing issues in the code, the goal of this whole exercise,
> > much more pleasant IMHO :-)
> > 
> > Thanks,
> > Knut
> > 
> > Knut Omang (5):
> >   runchecks: Generalize make C={1,2} to support multiple checkers
> >   Documentation: Add doc for runchecks, a checker runner
> >   checkpatch: Improve --fix-inplace for TABSTOP
> >   rds: Add runchecks.cfg for net/rds
> >   RDMA/core: Add runchecks.cfg for drivers/infiniband/core
> > 
> >  Documentation/dev-tools/coccinelle.rst |  12 +-
> >  Documentation/dev-tools/index.rst      |   1 +-
> >  Documentation/dev-tools/runchecks.rst  | 215 ++++++++-
> >  Documentation/dev-tools/sparse.rst     |  30 +-
> >  Documentation/kbuild/kbuild.txt        |   9 +-
> >  Makefile                               |  23 +-
> >  drivers/infiniband/core/runchecks.cfg  |  83 +++-
> >  net/rds/runchecks.cfg                  |  76 +++-
> >  scripts/Makefile.build                 |   4 +-
> >  scripts/checkpatch.pl                  |   2 +-
> >  scripts/runchecks                      | 734 ++++++++++++++++++++++++++-
> >  scripts/runchecks.cfg                  |  63 ++-
> >  scripts/runchecks_help.txt             |  43 ++-
> >  13 files changed, 1274 insertions(+), 21 deletions(-)
> >  create mode 100644 Documentation/dev-tools/runchecks.rst
> >  create mode 100644 drivers/infiniband/core/runchecks.cfg
> >  create mode 100644 net/rds/runchecks.cfg
> >  create mode 100755 scripts/runchecks
> >  create mode 100644 scripts/runchecks.cfg
> >  create mode 100644 scripts/runchecks_help.txt
> > 
> > base-commit: ae64f9bd1d3621b5e60d7363bc20afb46aede215
> 
> I like the ability to add more checkers and keep then in the main
> upstream tree. But adding overrides for specific subsystems goes against
> the policy that all subsystems should be treated equally.

This is a tool to enable automated testing for as many checks as possible, as soon as
possible. Like any tool, it can be misused, but that's IMHO an orthogonal problem that I
think the maintainers will be more than capable of preventing.

Think of this as a tightening screw: We eliminate errors class by class or file by file,
and in the same commit narrows in the list of exceptions. That way we can fix issues piece
by piece while avoiding a lot of regressions in already clean parts.

> There was discussion at Kernel Summit about how the different
> subsystems already have different rules. This appears to be a
> way to make that worse.

IMHO this is a tool that should help maintainers implement the policies they desire.
But the tool itself does not dictate any such.

Thanks,
Knut

Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Jason Gunthorpe]

On Sun, Dec 17, 2017 at 03:14:10AM +0100, Knut Omang wrote:

> > I like the ability to add more checkers and keep then in the main
> > upstream tree. But adding overrides for specific subsystems goes against
> > the policy that all subsystems should be treated equally.
> 
> This is a tool to enable automated testing for as many checks as
> possible, as soon as possible. Like any tool, it can be misused, but
> that's IMHO an orthogonal problem that I think the maintainers will
> be more than capable of preventing.
> 
> Think of this as a tightening screw: We eliminate errors class by
> class or file by file, and in the same commit narrows in the list of
> exceptions. That way we can fix issues piece by piece while avoiding
> a lot of regressions in already clean parts.

Since you used drivers/infiniband as an example for this script..

I will say I agree with this idea.

It is not that we *want* infiniband to be different from the rest of
the kernel, it is that we have this historical situation where we
don't have a code base that already passes the various static checker
things.

I would like it very much if I could run 'make static checker' and see
no warnings. This helps me know that I when I accept patches I am not
introducing new problems to code that has already been cleaned up.

Today when we run checkers we get so many warnings it is too hard to
make any sense of it.

Being able to say File X is now clean for check XYZ seems very useful
and may motivate people to clean up the files they actualy care
about...

> > There was discussion at Kernel Summit about how the different
> > subsystems already have different rules. This appears to be a way
> > to make that worse.
> 
> IMHO this is a tool that should help maintainers implement the
> policies they desire.  But the tool itself does not dictate any
> such.

Yes, again, in infiniband we like to see checkpatch be good for new
submission, even if that clashes with surrounding code. For instance
we have a mixture of sizeof foo and sizeof(foo) styles in the same
file/function now.

I certainly don't want to tell people they need to follow some
different style from 10 years ago when they send patches.

Jason

Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Joe Perches]

On Sun, 2017-12-17 at 22:00 -0700, Jason Gunthorpe wrote:
> On Sun, Dec 17, 2017 at 03:14:10AM +0100, Knut Omang wrote:
> 
> > > I like the ability to add more checkers and keep then in the main
> > > upstream tree. But adding overrides for specific subsystems goes against
> > > the policy that all subsystems should be treated equally.
> > 
> > This is a tool to enable automated testing for as many checks as
> > possible, as soon as possible. Like any tool, it can be misused, but
> > that's IMHO an orthogonal problem that I think the maintainers will
> > be more than capable of preventing.
> > 
> > Think of this as a tightening screw: We eliminate errors class by
> > class or file by file, and in the same commit narrows in the list of
> > exceptions. That way we can fix issues piece by piece while avoiding
> > a lot of regressions in already clean parts.
> 
> Since you used drivers/infiniband as an example for this script..
> 
> I will say I agree with this idea.
> 
> It is not that we *want* infiniband to be different from the rest of
> the kernel, it is that we have this historical situation where we
> don't have a code base that already passes the various static checker
> things.
> 
> I would like it very much if I could run 'make static checker' and see
> no warnings. This helps me know that I when I accept patches I am not
> introducing new problems to code that has already been cleaned up.
> 
> Today when we run checkers we get so many warnings it is too hard to
> make any sense of it.

Here is a list of the checkpatch messages for drivers/infiniband
sorted by type.

Many of these might be corrected by using

$ ./scripts/checkpatch.pl -f --fix-inplace --types=<TYPE> \
  $(git ls-files drivers/infiniband/)

   5243 CHECK:CAMELCASE
   4487 WARNING:LONG_LINE
   1755 CHECK:PARENTHESIS_ALIGNMENT
   1664 CHECK:SPACING
    910 WARNING:FUNCTION_ARGUMENTS
    742 CHECK:OPEN_ENDED_LINE
    685 CHECK:BRACES
    643 CHECK:UNNECESSARY_PARENTHESES
    478 WARNING:SIZEOF_PARENTHESIS
    361 WARNING:UNSPECIFIED_INT
    342 WARNING:LONG_LINE_COMMENT
    338 ERROR:SPACING
    338 CHECK:LINE_SPACING
    306 WARNING:SPLIT_STRING
    278 WARNING:SPACING
    242 WARNING:SYMBOLIC_PERMS
    194 WARNING:BLOCK_COMMENT_STYLE
    175 CHECK:BIT_MACRO
    158 WARNING:SPACE_BEFORE_TAB
    154 WARNING:LINE_SPACING
    139 CHECK:MACRO_ARG_REUSE
    133 CHECK:UNCOMMENTED_DEFINITION
    122 CHECK:AVOID_BUG
    103 CHECK:COMPARISON_TO_NULL
    101 WARNING:ENOSYS
     89 WARNING:BRACES
     78 WARNING:PREFER_PR_LEVEL
     74 WARNING:MULTILINE_DEREFERENCE
     59 CHECK:TYPO_SPELLING
     52 WARNING:EMBEDDED_FUNCTION_NAME
     52 CHECK:MULTIPLE_ASSIGNMENTS
     50 CHECK:PREFER_KERNEL_TYPES
     45 WARNING:RETURN_VOID
     39 WARNING:UNNECESSARY_ELSE
     38 ERROR:POINTER_LOCATION
     37 WARNING:ALLOC_WITH_MULTIPLY
     36 CHECK:ALLOC_SIZEOF_STRUCT
     35 CHECK:AVOID_EXTERNS
     34 WARNING:PRINTK_WITHOUT_KERN_LEVEL
     33 ERROR:CODE_INDENT
     32 WARNING:PREFER_PACKED
     32 CHECK:LOGICAL_CONTINUATIONS
     29 WARNING:MEMORY_BARRIER
     29 WARNING:LEADING_SPACE
     28 WARNING:DEEP_INDENTATION
     27 CHECK:USLEEP_RANGE
     23 WARNING:SUSPECT_CODE_INDENT
     23 ERROR:TRAILING_STATEMENTS
     21 WARNING:LONG_LINE_STRING
     20 WARNING:CONSIDER_KSTRTO
     18 WARNING:CONSTANT_COMPARISON
     18 ERROR:OPEN_BRACE
     15 WARNING:QUOTED_WHITESPACE_BEFORE_NEWLINE
     14 WARNING:VOLATILE
     14 ERROR:SWITCH_CASE_INDENT_LEVEL
     11 WARNING:OOM_MESSAGE
     11 WARNING:INCLUDE_LINUX
     10 WARNING:SSCANF_TO_KSTRTO
     10 WARNING:INDENTED_LABEL
      9 ERROR:GLOBAL_INITIALISERS
      9 ERROR:COMPLEX_MACRO
      9 ERROR:ASSIGN_IN_IF
      8 WARNING:UNNECESSARY_BREAK
      6 WARNING:PRINTF_L
      6 WARNING:MISORDERED_TYPE
      6 ERROR:INITIALISED_STATIC
      5 WARNING:TABSTOP
      5 WARNING:SINGLE_STATEMENT_DO_WHILE_MACRO
      5 WARNING:NAKED_SSCANF
      4 WARNING:NEEDLESS_IF
      4 ERROR:RETURN_PARENTHESES
      4 CHECK:BOOL_COMPARISON
      3 WARNING:TRAILING_SEMICOLON
      3 WARNING:STATIC_CONST_CHAR_ARRAY
      3 ERROR:TRAILING_WHITESPACE
      2 WARNING:UNNECESSARY_PARENTHESES
      2 WARNING:MISSING_SPACE
      2 WARNING:LOGGING_CONTINUATION
      2 CHECK:ARCH_DEFINES
      1 WARNING:TYPECAST_INT_CONSTANT
      1 WARNING:PREFER_DEV_LEVEL
      1 WARNING:NR_CPUS
      1 WARNING:NEW_TYPEDEFS
      1 WARNING:MINMAX
      1 WARNING:MACRO_WITH_FLOW_CONTROL
      1 WARNING:LINE_CONTINUATIONS
      1 WARNING:DO_WHILE_MACRO_WITH_TRAILING_SEMICOLON
      1 WARNING:DEFAULT_NO_BREAK
      1 WARNING:CONST_STRUCT
      1 WARNING:CONSIDER_COMPLETION
      1 ERROR:WHILE_AFTER_BRACE
      1 ERROR:ELSE_AFTER_BRACE
      1 CHECK:REDUNDANT_CODE

Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Knut Omang]

On Sun, 2017-12-17 at 22:00 -0800, Joe Perches wrote:
> On Sun, 2017-12-17 at 22:00 -0700, Jason Gunthorpe wrote:
> > On Sun, Dec 17, 2017 at 03:14:10AM +0100, Knut Omang wrote:
> > 
> > > > I like the ability to add more checkers and keep then in the main
> > > > upstream tree. But adding overrides for specific subsystems goes against
> > > > the policy that all subsystems should be treated equally.
> > > 
> > > This is a tool to enable automated testing for as many checks as
> > > possible, as soon as possible. Like any tool, it can be misused, but
> > > that's IMHO an orthogonal problem that I think the maintainers will
> > > be more than capable of preventing.
> > > 
> > > Think of this as a tightening screw: We eliminate errors class by
> > > class or file by file, and in the same commit narrows in the list of
> > > exceptions. That way we can fix issues piece by piece while avoiding
> > > a lot of regressions in already clean parts.
> > 
> > Since you used drivers/infiniband as an example for this script..
> > 
> > I will say I agree with this idea.
> > 
> > It is not that we *want* infiniband to be different from the rest of
> > the kernel, it is that we have this historical situation where we
> > don't have a code base that already passes the various static checker
> > things.
> > 
> > I would like it very much if I could run 'make static checker' and see
> > no warnings. This helps me know that I when I accept patches I am not
> > introducing new problems to code that has already been cleaned up.
> > 
> > Today when we run checkers we get so many warnings it is too hard to
> > make any sense of it.
> 
> Here is a list of the checkpatch messages for drivers/infiniband
> sorted by type.
> 
> Many of these might be corrected by using
> 
> $ ./scripts/checkpatch.pl -f --fix-inplace --types=<TYPE> \
>   $(git ls-files drivers/infiniband/)

Yes, and I already did that work piece by piece for individual types,
just to test the runchecks tool, and want to post that set once the 
runchecks script and Makefile changes itself are in,

see:

https://github.com/torvalds/linux/compare/master...knuto:runchecks

What I personally like about this approach is that with the runchecks.cfg file
one can focus easily on one or two check types at a time, create a commit out of that
and at the same time "tighten" runchecks.cfg - changes in that file then serves as 
documentation for what got changed and use maintainers can use comments to indicate 
why the remaining exceptions are still there - and there's a one-stop for anyone 
wanting to contribute to checkpatch/sparse/doc fixes.

And it will be possible to run bisect with make C=2 and always have 0 errors.

Thanks,
Knut
> 
>    5243 CHECK:CAMELCASE
>    4487 WARNING:LONG_LINE
>    1755 CHECK:PARENTHESIS_ALIGNMENT
>    1664 CHECK:SPACING
>     910 WARNING:FUNCTION_ARGUMENTS
>     742 CHECK:OPEN_ENDED_LINE
>     685 CHECK:BRACES
>     643 CHECK:UNNECESSARY_PARENTHESES
>     478 WARNING:SIZEOF_PARENTHESIS
>     361 WARNING:UNSPECIFIED_INT
>     342 WARNING:LONG_LINE_COMMENT
>     338 ERROR:SPACING
>     338 CHECK:LINE_SPACING
>     306 WARNING:SPLIT_STRING
>     278 WARNING:SPACING
>     242 WARNING:SYMBOLIC_PERMS
>     194 WARNING:BLOCK_COMMENT_STYLE
>     175 CHECK:BIT_MACRO
>     158 WARNING:SPACE_BEFORE_TAB
>     154 WARNING:LINE_SPACING
>     139 CHECK:MACRO_ARG_REUSE
>     133 CHECK:UNCOMMENTED_DEFINITION
>     122 CHECK:AVOID_BUG
>     103 CHECK:COMPARISON_TO_NULL
>     101 WARNING:ENOSYS
>      89 WARNING:BRACES
>      78 WARNING:PREFER_PR_LEVEL
>      74 WARNING:MULTILINE_DEREFERENCE
>      59 CHECK:TYPO_SPELLING
>      52 WARNING:EMBEDDED_FUNCTION_NAME
>      52 CHECK:MULTIPLE_ASSIGNMENTS
>      50 CHECK:PREFER_KERNEL_TYPES
>      45 WARNING:RETURN_VOID
>      39 WARNING:UNNECESSARY_ELSE
>      38 ERROR:POINTER_LOCATION
>      37 WARNING:ALLOC_WITH_MULTIPLY
>      36 CHECK:ALLOC_SIZEOF_STRUCT
>      35 CHECK:AVOID_EXTERNS
>      34 WARNING:PRINTK_WITHOUT_KERN_LEVEL
>      33 ERROR:CODE_INDENT
>      32 WARNING:PREFER_PACKED
>      32 CHECK:LOGICAL_CONTINUATIONS
>      29 WARNING:MEMORY_BARRIER
>      29 WARNING:LEADING_SPACE
>      28 WARNING:DEEP_INDENTATION
>      27 CHECK:USLEEP_RANGE
>      23 WARNING:SUSPECT_CODE_INDENT
>      23 ERROR:TRAILING_STATEMENTS
>      21 WARNING:LONG_LINE_STRING
>      20 WARNING:CONSIDER_KSTRTO
>      18 WARNING:CONSTANT_COMPARISON
>      18 ERROR:OPEN_BRACE
>      15 WARNING:QUOTED_WHITESPACE_BEFORE_NEWLINE
>      14 WARNING:VOLATILE
>      14 ERROR:SWITCH_CASE_INDENT_LEVEL
>      11 WARNING:OOM_MESSAGE
>      11 WARNING:INCLUDE_LINUX
>      10 WARNING:SSCANF_TO_KSTRTO
>      10 WARNING:INDENTED_LABEL
>       9 ERROR:GLOBAL_INITIALISERS
>       9 ERROR:COMPLEX_MACRO
>       9 ERROR:ASSIGN_IN_IF
>       8 WARNING:UNNECESSARY_BREAK
>       6 WARNING:PRINTF_L
>       6 WARNING:MISORDERED_TYPE
>       6 ERROR:INITIALISED_STATIC
>       5 WARNING:TABSTOP
>       5 WARNING:SINGLE_STATEMENT_DO_WHILE_MACRO
>       5 WARNING:NAKED_SSCANF
>       4 WARNING:NEEDLESS_IF
>       4 ERROR:RETURN_PARENTHESES
>       4 CHECK:BOOL_COMPARISON
>       3 WARNING:TRAILING_SEMICOLON
>       3 WARNING:STATIC_CONST_CHAR_ARRAY
>       3 ERROR:TRAILING_WHITESPACE
>       2 WARNING:UNNECESSARY_PARENTHESES
>       2 WARNING:MISSING_SPACE
>       2 WARNING:LOGGING_CONTINUATION
>       2 CHECK:ARCH_DEFINES
>       1 WARNING:TYPECAST_INT_CONSTANT
>       1 WARNING:PREFER_DEV_LEVEL
>       1 WARNING:NR_CPUS
>       1 WARNING:NEW_TYPEDEFS
>       1 WARNING:MINMAX
>       1 WARNING:MACRO_WITH_FLOW_CONTROL
>       1 WARNING:LINE_CONTINUATIONS
>       1 WARNING:DO_WHILE_MACRO_WITH_TRAILING_SEMICOLON
>       1 WARNING:DEFAULT_NO_BREAK
>       1 WARNING:CONST_STRUCT
>       1 WARNING:CONSIDER_COMPLETION
>       1 ERROR:WHILE_AFTER_BRACE
>       1 ERROR:ELSE_AFTER_BRACE
>       1 CHECK:REDUNDANT_CODE
> 

Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Knut Omang]

On Sun, 2017-12-17 at 22:00 -0700, Jason Gunthorpe wrote:
> On Sun, Dec 17, 2017 at 03:14:10AM +0100, Knut Omang wrote:
> 
> > > I like the ability to add more checkers and keep then in the main
> > > upstream tree. But adding overrides for specific subsystems goes against
> > > the policy that all subsystems should be treated equally.
> > 
> > This is a tool to enable automated testing for as many checks as
> > possible, as soon as possible. Like any tool, it can be misused, but
> > that's IMHO an orthogonal problem that I think the maintainers will
> > be more than capable of preventing.
> > 
> > Think of this as a tightening screw: We eliminate errors class by
> > class or file by file, and in the same commit narrows in the list of
> > exceptions. That way we can fix issues piece by piece while avoiding
> > a lot of regressions in already clean parts.
> 
> Since you used drivers/infiniband as an example for this script..
> 
> I will say I agree with this idea.

> It is not that we *want* infiniband to be different from the rest of
> the kernel, it is that we have this historical situation where we
> don't have a code base that already passes the various static checker
> things.

I poked around trying make M= in different parts of the kernel to get some 
"mileage" and to get as much examples as possible, and it appears most of 
the kernel has issues of sorts. Also Joe and others keep adding more checks 
as well, and we'd want that process to coexist with the need for automatic 
regression testing in this area.

> I would like it very much if I could run 'make static checker' and see
> no warnings. 

which is just what is the result with 'make C=2 M=drivers/infiniband/core' 
and the patches #1 and #5 in this set in case not everyone got the point,

> This helps me know that I when I accept patches I am not
> introducing new problems to code that has already been cleaned up.
> 
> Today when we run checkers we get so many warnings it is too hard to
> make any sense of it.
> 
> Being able to say File X is now clean for check XYZ seems very useful
> and may motivate people to clean up the files they actualy care
> about...
> 
> > > There was discussion at Kernel Summit about how the different
> > > subsystems already have different rules. This appears to be a way
> > > to make that worse.
> > 
> > IMHO this is a tool that should help maintainers implement the
> > policies they desire.  But the tool itself does not dictate any
> > such.
> 
> Yes, again, in infiniband we like to see checkpatch be good for new
> submission, even if that clashes with surrounding code. For instance
> we have a mixture of sizeof foo and sizeof(foo) styles in the same
> file/function now.

That's one of the fixes that the excellent --fix-inplace handles so 
one of my patches here

https://github.com/torvalds/linux/compare/master...knuto:runchecks

fixes those in drivers/infiniband/core.

> I certainly don't want to tell people they need to follow some
> different style from 10 years ago when they send patches.
> 

Thanks,
Knut

> Jason

Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Joe Perches]

On Mon, 2017-12-18 at 14:05 +0100, Knut Omang wrote:
> > Here is a list of the checkpatch messages for drivers/infiniband
> > sorted by type.
> > 
> > Many of these might be corrected by using
> > 
> > $ ./scripts/checkpatch.pl -f --fix-inplace --types=<TYPE> \
> >   $(git ls-files drivers/infiniband/)
> 
> Yes, and I already did that work piece by piece for individual types,
> just to test the runchecks tool, and want to post that set once the 
> runchecks script and Makefile changes itself are in,

I think those are independent of any runcheck tool changes and
could be posted now.  In general, don't keep patches in a local
tree waiting on some other unrelated patch.

Just fyi:

There is a script that helps automate checkpatch "by-type" conversions
with compilation, .o difference checking, and git commit editing.

https://lkml.org/lkml/2014/7/11/794

Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Knut Omang]

On Mon, 2017-12-18 at 07:30 -0800, Joe Perches wrote:
> On Mon, 2017-12-18 at 14:05 +0100, Knut Omang wrote:
> > > Here is a list of the checkpatch messages for drivers/infiniband
> > > sorted by type.
> > > 
> > > Many of these might be corrected by using
> > > 
> > > $ ./scripts/checkpatch.pl -f --fix-inplace --types=<TYPE> \
> > >   $(git ls-files drivers/infiniband/)
> > 
> > Yes, and I already did that work piece by piece for individual types,
> > just to test the runchecks tool, and want to post that set once the 
> > runchecks script and Makefile changes itself are in,
> 
> I think those are independent of any runcheck tool changes and
> could be posted now.  In general, don't keep patches in a local
> tree waiting on some other unrelated patch.

It becomes related in that the runchecks.cfg file is updated 
in all the patches to keep 'make C=2' run with 0 errors while 
enabling more checks. I think they serve well as examples of 
how a workflow with runchecks could be.

> Just fyi:
> 
> There is a script that helps automate checkpatch "by-type" conversions
> with compilation, .o difference checking, and git commit editing.
> 
> https://lkml.org/lkml/2014/7/11/794

oh - good to know - seems it would have been a good help
during my little exercise..

Thanks,
Knut

Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Jason Gunthorpe]

On Sun, Dec 17, 2017 at 10:00:17PM -0800, Joe Perches wrote:

> > Today when we run checkers we get so many warnings it is too hard to
> > make any sense of it.
> 
> Here is a list of the checkpatch messages for drivers/infiniband
> sorted by type.
> 
> Many of these might be corrected by using
> 
> $ ./scripts/checkpatch.pl -f --fix-inplace --types=<TYPE> \
>   $(git ls-files drivers/infiniband/)

How many of these do you think it is worth to fix?

We do get a steady trickle of changes in this topic every cycle.

Is it better to just do a big number of them all at once? Do you have
an idea how disruptive this kind of work is to the whole patch flow
eg new patches no longer applying to for-next, backports no longer
applying, merge conflicts?

Jason

Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Jason Gunthorpe]

On Mon, Dec 18, 2017 at 02:05:15PM +0100, Knut Omang wrote:
 
> https://github.com/torvalds/linux/compare/master...knuto:runchecks

Several of these to rdma/core do not look so big, you should think
about sending them..

Jason

Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Joe Perches]

On Mon, 2017-12-18 at 10:46 -0700, Jason Gunthorpe wrote:
> On Sun, Dec 17, 2017 at 10:00:17PM -0800, Joe Perches wrote:
> 
> > > Today when we run checkers we get so many warnings it is too hard to
> > > make any sense of it.
> > 
> > Here is a list of the checkpatch messages for drivers/infiniband
> > sorted by type.
> > 
> > Many of these might be corrected by using
> > 
> > $ ./scripts/checkpatch.pl -f --fix-inplace --types=<TYPE> \
> >   $(git ls-files drivers/infiniband/)
> 
> How many of these do you think it is worth to fix?
> 
> We do get a steady trickle of changes in this topic every cycle.
> 
> Is it better to just do a big number of them all at once?

I think so.

> Do you have
> an idea how disruptive this kind of work is to the whole patch flow
> eg new patches no longer applying to for-next, backports no longer
> applying, merge conflicts?

Some do complain about backport patch purity.

I think that difficulty is overstated, but then
again, I don't do backports very often.

I think the best time for any rather wholesale
change is immediately after an rc-1 so overall
in-flight patch conflict volume is minimized.

Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Bart Van Assche]

On Mon, 2017-12-18 at 10:46 -0700, Jason Gunthorpe wrote:
> On Sun, Dec 17, 2017 at 10:00:17PM -0800, Joe Perches wrote:
> 
> > > Today when we run checkers we get so many warnings it is too hard to
> > > make any sense of it.
> > 
> > Here is a list of the checkpatch messages for drivers/infiniband
> > sorted by type.
> > 
> > Many of these might be corrected by using
> > 
> > $ ./scripts/checkpatch.pl -f --fix-inplace --types=<TYPE> \
> >   $(git ls-files drivers/infiniband/)
> 
> How many of these do you think it is worth to fix?
> 
> We do get a steady trickle of changes in this topic every cycle.
> 
> Is it better to just do a big number of them all at once? Do you have
> an idea how disruptive this kind of work is to the whole patch flow
> eg new patches no longer applying to for-next, backports no longer
> applying, merge conflicts?

In my opinion patches that only change the coding style and do not change any
functionality are annoying. Before posting a patch that fixes a bug the change
history (git log -p) has to be cheched to figure out which patch introduced
the bug. Patches that only change coding style pollute the change history.

Bart.

Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Knut Omang]

On Mon, 2017-12-18 at 17:56 +0000, Bart Van Assche wrote:
> On Mon, 2017-12-18 at 10:46 -0700, Jason Gunthorpe wrote:
> > On Sun, Dec 17, 2017 at 10:00:17PM -0800, Joe Perches wrote:
> > 
> > > > Today when we run checkers we get so many warnings it is too hard to
> > > > make any sense of it.
> > > 
> > > Here is a list of the checkpatch messages for drivers/infiniband
> > > sorted by type.
> > > 
> > > Many of these might be corrected by using
> > > 
> > > $ ./scripts/checkpatch.pl -f --fix-inplace --types=<TYPE> \
> > >   $(git ls-files drivers/infiniband/)
> > 
> > How many of these do you think it is worth to fix?
> > 
> > We do get a steady trickle of changes in this topic every cycle.
> > 
> > Is it better to just do a big number of them all at once? Do you have
> > an idea how disruptive this kind of work is to the whole patch flow
> > eg new patches no longer applying to for-next, backports no longer
> > applying, merge conflicts?
> 
> In my opinion patches that only change the coding style and do not change any
> functionality are annoying. Before posting a patch that fixes a bug the change
> history (git log -p) has to be cheched to figure out which patch introduced
> the bug. Patches that only change coding style pollute the change history.

I agree with you - the problem is that style issues should not have existed. 
But when they do it becomes a problem to remove them and a problem to 
keep them - for instance us who try to be compliant by having style helpers 
in our editor, we end up having to manually revert old style mistakes back in
to avoid making unrelated whitespace changes or similar.

I think what we get with this patch set is that there's a way to rein in the
issues and to enable some regression testing without enforcing a lot of work 
or annoying history issues *right away*. That way this problem can be tackled when
appropriate for the subsystem in question, and the reason for holding back on 
some changes can be documented in the local runchecks.cfg file, focusing
willing helping hands on the issues considered most important for that 
subsystem.

Thanks,
Knut

Re: [PATCH v2 0/5] Support for generalized use of make C={1,2} via a wrapper program

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 2032 bytes --]

On Mon, Dec 18, 2017 at 07:39:50PM +0100, Knut Omang wrote:
> On Mon, 2017-12-18 at 17:56 +0000, Bart Van Assche wrote:
> > On Mon, 2017-12-18 at 10:46 -0700, Jason Gunthorpe wrote:
> > > On Sun, Dec 17, 2017 at 10:00:17PM -0800, Joe Perches wrote:
> > >
> > > > > Today when we run checkers we get so many warnings it is too hard to
> > > > > make any sense of it.
> > > >
> > > > Here is a list of the checkpatch messages for drivers/infiniband
> > > > sorted by type.
> > > >
> > > > Many of these might be corrected by using
> > > >
> > > > $ ./scripts/checkpatch.pl -f --fix-inplace --types=<TYPE> \
> > > >   $(git ls-files drivers/infiniband/)
> > >
> > > How many of these do you think it is worth to fix?
> > >
> > > We do get a steady trickle of changes in this topic every cycle.
> > >
> > > Is it better to just do a big number of them all at once? Do you have
> > > an idea how disruptive this kind of work is to the whole patch flow
> > > eg new patches no longer applying to for-next, backports no longer
> > > applying, merge conflicts?
> >
> > In my opinion patches that only change the coding style and do not change any
> > functionality are annoying. Before posting a patch that fixes a bug the change
> > history (git log -p) has to be cheched to figure out which patch introduced
> > the bug. Patches that only change coding style pollute the change history.
>
> I agree with you - the problem is that style issues should not have existed.
> But when they do it becomes a problem to remove them and a problem to
> keep them - for instance us who try to be compliant by having style helpers
> in our editor, we end up having to manually revert old style mistakes back in
> to avoid making unrelated whitespace changes or similar.

If the checkpatch.pl complains about coding style for the new patch in
newly added code, I'm asking from the author to prepare cleanup patch so
it will be applied before actual patch.

In case, complains are for code which patch are not touching, I'm
submitting it as is.

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]