linux-kernel - Re: checkkpatch (in)sanity ?

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: <1472473855.3425.18.camel@perches.com>
Date:   Mon, 29 Aug 2016 05:30:55 -0700
From:   Joe Perches <joe@...ches.com>
To:     Kalle Valo <kvalo@...eaurora.org>
Cc:     "Levin, Alexander" <alexander.levin@...izon.com>,
        Sasha Levin <levinsasha928@...il.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        LKML <linux-kernel@...r.kernel.org>,
        "ksummit-discuss@...ts.linuxfoundation.org" 
        <ksummit-discuss@...ts.linuxfoundation.org>
Subject: Re: checkkpatch (in)sanity ?

On Mon, 2016-08-29 at 14:15 +0300, Kalle Valo wrote:
> I wish that checkpatch would have a way to enable/disable warnings per
> directory (or file). For example, there would be
> drivers/net/wireless/ath/ath10k/.checkpatch which would disable the
> warnings are not suitable for ath10k for one reason or another:
> 
> 'MSLEEP',
> 'USLEEP_RANGE',
> 'PRINTK_WITHOUT_KERN_LEVEL',
> 'NETWORKING_BLOCK_COMMENT_STYLE',
> 'BLOCK_COMMENT_STYLE',
> 'LINUX_VERSION_CODE',
> 'COMPLEX_MACRO',
> 'PREFER_DEV_LEVEL',
> 'PREFER_PR_LEVEL',
> 'COMPARISON_TO_NULL',
> 'BIT_MACRO',
> 'CONSTANT_COMPARISON',
> 'MACRO_WITH_FLOW_CONTROL'
> 
> Currently my workaround is to have a custom ath10k-check script[1] which
> runs checkpatch with those checks disabled. Oh, and it also filters out
> some of the warnings based on the symbol it is located in.
> 
> https://github.com/qca/qca-swiss-army-knife/blob/master/tools/scripts/ath10k/ath10k-check

Hey Kalle:

I looked at your script (which also does compilation
and sparse checking) I don't see how a .checkpatch_conf
hierarchy helps you much there as you've added all those
long symbol name long line avoidance bits.

Also, there'd be a lot of rework to the globals in
checkpatch for per-directory specific overrides if someone
fed it files in multiple directories like

checkpatch.pl <patchfile touching lib/kernel/include>

A couple btw's:

Why avoid the printk, sleep or macro tests?

And this for ath10k_core_register_work:

    ('ath10k_core_register_work', 'RETURN_VOID'),

and the code associated to it:

err:
	/* TODO: It's probably a good idea to release device from the driver
	 * but calling device_release_driver() here will cause a deadlock.
	 */
	return;
}

ia avoided a few times in the kernel by using a bare ";"
instead of "return;" before the function closing brace.

It's maybe unfortunate that gcc / c spec doesn't allow
jumping to a label just before the function close brace.