linux-kernel - Re: Software evolution around “checkpatch.pl”?

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]

Date:   Sat, 10 Feb 2018 09:42:57 -0800
From:   Joe Perches <joe@...ches.com>
To:     Jonathan Cameron <jic23@...23.retrosnub.co.uk>
Cc:     SF Markus Elfring <elfring@...rs.sourceforge.net>,
        linux-iio@...r.kernel.org, linux-input@...r.kernel.org,
        Jonathan Cameron <jic23@...nel.org>,
        Hartmut Knaack <knaack.h@....de>,
        Jiri Kosina <jikos@...nel.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        LKML <linux-kernel@...r.kernel.org>,
        kernel-janitors@...r.kernel.org
Subject: Re: Software evolution around
 “checkpatch.pl”?

On Sat, 2018-02-10 at 15:57 +0000, Jonathan Cameron wrote:
> On Sat, 10 Feb 2018 06:59:43 -0800
> Joe Perches <joe@...ches.com> wrote:
> 
> > On Sat, 2018-02-10 at 14:53 +0000, Jonathan Cameron wrote:
> > > While it would be great to improve checkpatches false
> > > positive rate, it's very nature as a string matcher makes
> > > this hard.  
> > 
> > true.
> > 
> > what are the false positives you see?
> > 
> 
> This particular case is only 'sort of' a false positive
> in the warning that a message printed on a memory allocation
> failure 'may' not add any information over the generic case.

Right.  So it's not a 'false positive' at all.
Are there any actual 'false positives' you know of?

> Very hard to judge on whether it is useful to know more than
> an allocation failed somewhere or not.
> 
> Message makes this clear:
> > “WARNING: Possible unnecessary 'out of memory' message”
> > (from the script “checkpatch.pl”)  
> 
> We also have the balance between any changes to existing code
> adding 'some' maintenance overhead vs changing this stuff
> in a new driver - which is what checkpatch is really intended
> for.

There's almost zero maintenance overhead here.
The time it takes for the back and forth
replies is likely larger.

> So I think checkpatch is striking the right balance here in
> how it warns.  Obviously if it could assess the text
> and come to an informed decision that would be great but
> we are some way from that ;)

The 'informed' bit is difficult as it is mostly
a political problem.

This particular message really is unnecessary as
the generic dump_stack on any normal allocation
(ie: without __GFP_WARN) already emits location
specific information.

Removing these messages can help make the kernel
image smaller and thereby help make these OOM
messages a tiny bit less likely.

I just wish Markus would improve his consistently
terrible commit messages that just restate the
action being done and detail _why_ a particular
thing _should_ be done.

His acceptance rate would improve as many of these
back and forth replies for what trivialities he
posts as patches would be minimized.