Re: checkkpatch (in)sanity ?

From	"Levin, Alexander" <alexander.levin@verizon.com>
Newsgroups	linux.kernel
Subject	Re: checkkpatch (in)sanity ?
Date	2016-08-28 03:10 +0200
Message-ID	<saWLL-3rM-1@gated-at.bofh.it> (permalink)
References	<saSI9-NY-9@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Sat, Aug 27, 2016 at 04:40:52PM -0400, Joe Perches wrote:
> On Fri, Aug 26, 2016 at 01:26:35PM +0200, Greg KH wrote:
> > On Fri, Aug 26, 2016 at 12:46:51AM -0400, Levin, Alexander wrote:
> >>    - Making checkpatch check for (some) of the stable kernel rules
> >>    (and possibly recommend adding the stable@ tag in certain cases?).
> >>      - Depends on: making checkpatch sane again.>This sounds interesting.  What do you mean by "sane"?
> 
> Sasha, can you expand your thoughts here please?

Sure. I have 2.5 concerns about the state of checkpatch:

On Sat, Aug 27, 2016 at 04:40:52PM -0400, Joe Perches wrote: 
> Most all of the trivial spacing stuff can easily be
> ignored either by a human determining what's important
> or by using command line options like --ignore=spacing

1.
This is the wrong default. By default checkpatch shouldn't be showing trivial
issues that encourage folks to try and work around them and as a result
produce worse code.

Look at the 80 character limit warning for example, what good does it do? It
encourages people to do even stupider things to work around it and results in
a bunch of "fix checkpatch warning" that touch existing code just to make the
result harder to read and make 'git blame' harder to work with.

By default you should only get the most critical warnings we have in the
kernel like missing S-O-B or corrupt patch.

2. A "who wrote these rules?": there seems to be a disconnect between the rules
checkpatch is trying to enforce and the accepted coding style enforced by
maintainers. 

Do a git-format-patch on all of the commits Linus authored in the past year or
two and see how many of them fail checkpatch (or do the same for any of the
commits that passed through and were accepted by the top maintainers),
according to checkpatch we need to make those guys stop touching the kernel.

3. This one is somewhat subjective: scripts/checkpatch.pl is a massive blob of
perl code that a fair amount of people don't know how to deal with. In 4.8 it's
6142 lines, making it the 124th largest source file in the kernel, well within
the top 1% of source files in the kernel.

This combination of size/language pushes people away from being involved in
what is supposed to be a central tool and gives them a reason to never use
it again after they see results they don't agree with (rather than fixing it).

-- 

Thanks,
Sasha

Thread

checkkpatch (in)sanity ? Joe Perches <joe@perches.com> - 2016-08-27 22:50 +0200
  Re: checkkpatch (in)sanity ? "Levin, Alexander" <alexander.levin@verizon.com> - 2016-08-28 03:10 +0200
    Re: checkkpatch (in)sanity ? Joe Perches <joe@perches.com> - 2016-08-28 03:50 +0200
      Re: checkkpatch (in)sanity ? Joe Perches <joe@perches.com> - 2016-08-28 04:30 +0200
      Re: checkkpatch (in)sanity ? "Levin, Alexander" <alexander.levin@verizon.com> - 2016-08-28 04:50 +0200
        Re: checkkpatch (in)sanity ? Joe Perches <joe@perches.com> - 2016-08-28 19:20 +0200
          Re: checkkpatch (in)sanity ? Greg KH <gregkh@linuxfoundation.org> - 2016-08-28 20:00 +0200
          Re: checkkpatch (in)sanity ? "Levin, Alexander" <alexander.levin@verizon.com> - 2016-08-29 00:40 +0200
            Re: checkkpatch (in)sanity ? Joe Perches <joe@perches.com> - 2016-08-29 01:30 +0200
              Re: checkkpatch (in)sanity ? "Levin, Alexander" <alexander.levin@verizon.com> - 2016-08-29 04:30 +0200
                Re: checkkpatch (in)sanity ? Christoph Hellwig <hch@infradead.org> - 2016-08-29 10:30 +0200
            Re: [Ksummit-discuss] checkkpatch (in)sanity ? Alexandre Belloni <alexandre.belloni@free-electrons.com> - 2016-08-29 09:20 +0200
              Re: [Ksummit-discuss] checkkpatch (in)sanity ? Arnd Bergmann <arnd@arndb.de> - 2016-08-29 11:10 +0200
                Re: [Ksummit-discuss] checkkpatch (in)sanity ? Joe Perches <joe@perches.com> - 2016-08-29 14:50 +0200
                Re: [Ksummit-discuss] checkkpatch (in)sanity ? Josh Triplett <josh@joshtriplett.org> - 2016-08-29 19:20 +0200
                Re: [Ksummit-discuss] checkkpatch (in)sanity ? Joe Perches <joe@perches.com> - 2016-08-29 19:50 +0200
          RE: [Ksummit-discuss] checkkpatch (in)sanity ? "Luck, Tony" <tony.luck@intel.com> - 2016-08-29 19:50 +0200
            Re: [Ksummit-discuss] checkkpatch (in)sanity ? Joe Perches <joe@perches.com> - 2016-08-29 20:10 +0200
              Re: [Ksummit-discuss] checkkpatch (in)sanity ? Joe Perches <joe@perches.com> - 2016-08-29 20:50 +0200
              Re: [Ksummit-discuss] checkkpatch (in)sanity ? Josh Triplett <josh@joshtriplett.org> - 2016-08-29 21:10 +0200
              Re: [Ksummit-discuss] checkkpatch (in)sanity ? Arnd Bergmann <arnd@arndb.de> - 2016-08-29 23:10 +0200
      Re: checkkpatch (in)sanity ? Kalle Valo <kvalo@codeaurora.org> - 2016-08-29 13:20 +0200
        Re: checkkpatch (in)sanity ? Joe Perches <joe@perches.com> - 2016-08-29 14:40 +0200
          Re: checkkpatch (in)sanity ? Kalle Valo <kvalo@codeaurora.org> - 2016-08-29 20:10 +0200
            Re: checkkpatch (in)sanity ? Joe Perches <joe@perches.com> - 2016-08-29 21:10 +0200
            Re: [Ksummit-discuss] checkkpatch (in)sanity ? Arnd Bergmann <arnd@arndb.de> - 2016-08-29 23:10 +0200
    Re: [Ksummit-discuss] checkkpatch (in)sanity ? Alexey Dobriyan <adobriyan@gmail.com> - 2016-08-28 10:00 +0200
      Re: [Ksummit-discuss] checkkpatch (in)sanity ? Julia Lawall <julia.lawall@lip6.fr> - 2016-08-28 12:00 +0200
        Re: [Ksummit-discuss] checkkpatch (in)sanity ? Joe Perches <joe@perches.com> - 2016-08-28 22:00 +0200
          Re: [Ksummit-discuss] checkkpatch (in)sanity ? Jiri Kosina <jikos@kernel.org> - 2016-08-28 22:40 +0200
          Re: [Ksummit-discuss] checkkpatch (in)sanity ? Dennis Kaarsemaker <dennis@kaarsemaker.net> - 2016-08-28 23:30 +0200
            Re: [Ksummit-discuss] checkkpatch (in)sanity ? Joe Perches <joe@perches.com> - 2016-08-29 00:00 +0200
  Re: [Ksummit-discuss] checkkpatch (in)sanity ? Dan Carpenter <dan.carpenter@oracle.com> - 2016-08-29 21:10 +0200
    Re: [Ksummit-discuss] checkkpatch (in)sanity ? Dan Carpenter <dan.carpenter@oracle.com> - 2016-08-29 21:20 +0200
      Re: [Ksummit-discuss] checkkpatch (in)sanity ? Joe Perches <joe@perches.com> - 2016-08-29 21:40 +0200
    Re: [Ksummit-discuss] checkkpatch (in)sanity ? Josh Triplett <josh@joshtriplett.org> - 2016-08-29 21:20 +0200
      Re: [Ksummit-discuss] checkkpatch (in)sanity ? Joe Perches <joe@perches.com> - 2016-08-29 21:30 +0200

csiph-web