linux-kernel - Re: [Ksummit-discuss] checkkpatch (in)sanity ?

Open Source and information security mailing list archives

Message-ID: <alpine.DEB.2.10.1608281152080.3321@hadrien>
Date:   Sun, 28 Aug 2016 11:59:38 +0200 (CEST)
From:   Julia Lawall <julia.lawall@...6.fr>
To:     Alexey Dobriyan <adobriyan@...il.com>
cc:     "Levin, Alexander" <alexander.levin@...izon.com>,
        Joe Perches <joe@...ches.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        Sasha Levin <levinsasha928@...il.com>,
        "ksummit-discuss@...ts.linuxfoundation.org" 
        <ksummit-discuss@...ts.linuxfoundation.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [Ksummit-discuss] checkkpatch (in)sanity ?

On Sun, 28 Aug 2016, Alexey Dobriyan wrote:

> On Sat, Aug 27, 2016 at 09:06:13PM -0400, Levin, Alexander via Ksummit-discuss wrote:
> > 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).
>
> It is a textbook example of what's wrong with Perl. Instead of parsing
> C code like compilers do, the script is one big pile of regexes.
> It mostly works ("doing its job" in perlspeak) because people mostly
> follow the coding style.

Parsing is slow.  Perfect parsing is impossible due to configuration
options.  There is definitely a place for regexps in checking code.
Perhaps there is a better way to express the regexps, or to provide
regexps for integration into the checkpatch infrastructure.

> Regarding individual warnings: some are good (RETURN_VOID, DATE_TIME,
> USE_NEGATIVE_ERRNO), some are OK given kernel style of allocating memory
> but the rationale is bogus (UNNECESSARY_CASTS, linking to userspace
> example of malloc() returning "int"!).
>
> And then there is ALLOC_SIZEOF_STRUCT which advocates "kmalloc(sizeof(*p))".
>
> The problem is that c-h.pl generates noise in the commit history and
> makes git-blame less useful than it can be.

Could it be that this is a problem with git blame, rather than with
checkpatch?  Last year there was a discussion on this list about how there
is an option to git blame that will cause it to step through the history,
and not show only the most recent patch that has modified a given line.

julia

>
> I for one given up on it more or less since its introduction.
> _______________________________________________
> Ksummit-discuss mailing list
> Ksummit-discuss@...ts.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss
>

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