Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752866AbeAGKMe (ORCPT + 1 other); Sun, 7 Jan 2018 05:12:34 -0500 Received: from bombadil.infradead.org ([65.50.211.133]:55771 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752232AbeAGKMa (ORCPT ); Sun, 7 Jan 2018 05:12:30 -0500 Date: Sun, 7 Jan 2018 08:12:15 -0200 From: Mauro Carvalho Chehab To: Knut Omang Cc: Jani Nikula , linux-kernel@vger.kernel.org, Nicolas Palix , Masahiro Yamada , John Haxby , linux-doc@vger.kernel.org, Jonathan Corbet , Gilles Muller , Michal Marek , =?UTF-8?B?TWlja2HDq2wgU2FsYcO8bg==?= , "Paul E. McKenney" , Julia Lawall , =?UTF-8?B?SMOla29u?= Bugge , =?UTF-8?B?w4VzbXVu?= =?UTF-8?B?ZCDDmHN0dm9sZA==?= , Matthew Wilcox , "Levin, Alexander (Sasha Levin)" , cocci@systeme.lip6.fr, linux-kbuild@vger.kernel.org Subject: Re: [PATCH v3 1/1] runchecks: Generalize make C={1,2} to support multiple checkers Message-ID: <20180107081215.29a31ea5@vento.lan> In-Reply-To: <1515181301.31439.724.camel@oracle.com> References: <5f292b7effba0efcf4855bff83b7b9313ac45895.1515072782.git-series.knut.omang@oracle.com> <874lo1aait.fsf@intel.com> <1515096931.31439.647.camel@oracle.com> <20180105160816.2e940aac@vento.lan> <1515181301.31439.724.camel@oracle.com> X-Mailer: Claws Mail 3.15.1-dirty (GTK+ 2.24.31; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: Em Fri, 05 Jan 2018 20:41:41 +0100 Knut Omang escreveu: > On Fri, 2018-01-05 at 16:08 -0200, Mauro Carvalho Chehab wrote: > > Em Thu, 04 Jan 2018 21:15:31 +0100 > > Knut Omang escreveu: > > > > > > I'm surprised the commit message and the provided documentation say > > > > nothing about using CHECK=foo on the command line. That already supports > > > > arbitrary checkers. > > > > > > The problem, highlighted by Jim Davis in > > > > > > https://lkml.org/lkml/2017/11/20/638 > > > > > > is that the current solution isn't flexible enough - that discussion > > > is what lead me to this reimplementation of what I originally intended > > > to be a checkpatch only solution. > > > > > > > How does this relate to that? Is this supposed to be > > > > a complete replacement? Or what? > > > > > > It has evolved into a complete replacement of the intention of CHECK. > > > > > > > 'make help' also references $CHECK, and this patch doesn't update the > > > > help text. > > > > > > I realize now that this needs to be handled in some way due to the way I split the > > > arguments with '--' - the intention was to keep it for bw compatibility. > > > > > > It would be good to know if people rely on using CHECK with C={1,2} for > > > anything beside the checkers supported by runchecks today > > > > I do. Here, I use: > > > > $ make ARCH=i386 CF=-D__CHECK_ENDIAN__ CONFIG_DEBUG_SECTION_MISMATCH=y C=1 W=1 > > CHECK='compile_checks' M=drivers/media > > > > Where "compile_checks" is actually a small script that calls both > > smatch and sparse: > > > > #!/bin/bash > > /devel/smatch/smatch -p=kernel $@ > > I suppose you here refer to this: > https://blogs.oracle.com/linuxkernel/smatch-static-analysis-tool-overview,-by-dan-carpenter > > Good idea! I'll have a look at how that plays with this. Yes. > > > /devel/sparse/sparse $@ > > > > So, I'm not sure why we need something else. > > The core functionality is the selective suppression logic and output unification > which makes checking with automated build tools more flexible and > applicable right away (not when every warning from every checker is fixed...) If the idea is to use it only/mostly with automated build tools, then the better would be to call it only when explicitly requested, e. g. something like C=3, in order to avoid breaking the usecase where one would run its own script. On my case, I use C=1 CHECK=compile_checks as part as my usual patch handling. For every patch I apply on media, I call make again, to be sure that no warning/building errors were added, not only with gcc but also with smatch and sparse. > > > That said, I didn't look > > on its code, but looking on its diffstat: > > > > Makefile | 23 +- > > scripts/Makefile.build | 4 +- > > scripts/runchecks | 734 ++++++++++++++++++++++++++- > > scripts/runchecks.cfg | 63 ++- > > scripts/runchecks_help.txt | 43 ++- > > > > Using a 734 lines python program just to do an exec on an external checker > > seems too much! > > Sure, if that was the case I would be the first to agree :-) > > Thanks, > Knut > > > Thanks, > > Mauro Thanks, Mauro