linux-kernel - Re: "CodingStyle: Clarify and complete chapter 7" in docs-next

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1474540930.8253.9.camel@perches.com>
Date:   Thu, 22 Sep 2016 03:42:10 -0700
From:   Joe Perches <joe@...ches.com>
To:     Jean Delvare <jdelvare@...e.de>
Cc:     Julia Lawall <julia.lawall@...6.fr>,
        Al Viro <viro@...IV.linux.org.uk>,
        Ilya Dryomov <idryomov@...il.com>,
        Andy Whitcroft <apw@...onical.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Jonathan Corbet <corbet@....net>,
        Ceph Development <ceph-devel@...r.kernel.org>,
        Alex Elder <elder@...nel.org>, Sage Weil <sage@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>,
        kernel-janitors@...r.kernel.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        linux-doc@...r.kernel.org
Subject: Re: "CodingStyle: Clarify and complete chapter 7" in docs-next

On Thu, 2016-09-22 at 11:24 +0200, Jean Delvare wrote:
[]
> > The seriousness with which some beginners take these message
> > types though is troublesome,
[]
> You need to think in terms of actual use cases. Who uses checkpatch and
> why? I think there are 3 groups of users:
> * Beginners. They won't run the script by themselves, instead they will
>   submit a patch which infringes a lot of coding style rules, and the
>   maintainer will point them to checkpatch and ask for a resubmission
>   which makes checkpatch happy. Being beginners, they can only rely on
>   the script itself to only report things which need to be fixed, by
>   default.
> * Experienced developers. Who simply want to make sure they did not
>   overlook anything before they post their work for review. They have
>   the knowledge to decide if they want to ignore some of the warnings.
> * People with too much spare time, looking for anything they could
>   "contribute" to the kernel. They will use --subjective and piss off
>   every maintainer they can find.

I think you overlook the category of a beginner submitting
"my first kernel patch" which is a "coding style" defect of
some type.  The Eudyptula and Outreachy programs seem to
encourage these sorts of patches.

This is where "scripts/checkpatch.pl -f <file>" is most used.

I believe adding the --force option might be useful to
restrict cleanup-style-only patches outside of staging.

There's nothing wrong with cleanup style patches, it can be
good introduction to compiler/config tool & kernel setup.
 
> I would rather suggest:
> 
> ERROR -> MUST_FIX
> WARNING -> SHOULD_FIX
> CHECK -> MAY_FIX

MUST is much stronger language than I would prefer.

There are still about a quarter million ERRORs just for
spacing issues in the kernel tree.

Here are the top 10 ERROR checkpatch messages treewide as of
a few days ago,

$ grep ERROR checkpatch.short_sorted_20160917
 268308  ERROR:SPACING
  37340  ERROR:CODE_INDENT
  27678  ERROR:TRAILING_WHITESPACE
  21024  ERROR:COMPLEX_MACRO
  14048  ERROR:POINTER_LOCATION
  12207  ERROR:TRAILING_STATEMENTS
  11079  ERROR:OPEN_BRACE
   6802  ERROR:ASSIGN_IN_IF
   3940  ERROR:RETURN_PARENTHESES
   2322  ERROR:NON_OCTAL_PERMISSIONS

Maybe there could be some better classifications of the various
messages.

But there are about two million checkpatch messages overall in
the kernel tree.

That's a lot.