linux-kernel - Re: [PATCH v2 0/2] checkpatch: Add support for checkpatch-ignore note

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: <763bd905-6f1a-42a9-9f81-acecd98131a4@oss.qualcomm.com>
Date: Mon, 24 Feb 2025 11:09:02 -0800
From: Jeff Johnson <jeff.johnson@....qualcomm.com>
To: Brendan Jackman <jackmanb@...gle.com>, Andy Whitcroft
 <apw@...onical.com>,
        Joe Perches <joe@...ches.com>, Dwaipayan Ray <dwaipayanray1@...il.com>,
        Lukas Bulwahn <lukas.bulwahn@...il.com>,
        Jonathan Corbet <corbet@....net>,
        Konstantin Ryabitsev <konstantin@...uxfoundation.org>
Cc: linux-kernel@...r.kernel.org, workflows@...r.kernel.org,
        linux-doc@...r.kernel.org
Subject: Re: [PATCH v2 0/2] checkpatch: Add support for checkpatch-ignore note

On 1/15/2025 7:33 AM, Brendan Jackman wrote:
> Checkpatch sometimes has false positives. This makes it less useful for
> automatic usage: tools like b4 [0] can run checkpatch on all of your
> patches and give you a quick overview. When iterating on a branch, it's
> tiresome to manually re-check that any errors are known false positives.
> 
> This patch adds a feature to record in the patch "graveyard" (after the
> "---" that a patch might produce a certain checkpatch error, and that
> this is an expected false positive.
> 
> Note, for Git users this will require some configuration changes to
> adopt (see documentation patch), and not all tools that wrap Checkpatch
> will automatically be able to take advantage. Changes are required for
> `b4 prep --check` to work [0], I'll send that separately if this patch
> is accepted.
> 
> [0] https://github.com/bjackman/b4/tree/checkpatch-ignore

While this addresses one issue with checkpatch, it doesn't solve what I
consider to be a bigger issue, namely to have a way for checkpatch to ignore
false positives (or deliberate use of non-compliant code) when run with the -f
flag.

I don't know how many times I've seen new kernel contributors try to "fix"
checkpatch issues as their first commit, and contribute broken code in the
process. This is especially true when trying to "fix" macros.

So IMO what would be more useful is to have some way to document these
overrides in the tree so that they could be honored both when processing
patches as well as when processing files.

Note that thanks to Kalle's work, the wireless/ath drivers have their own way
of doing this, but of course that only works if you run the scripts.
checkpatch run normally would still flag the issues.

more surgical:
<https://github.com/qca/qca-swiss-army-knife/blob/master/tools/scripts/ath12k/ath12k-check>

less surgical:
<https://github.com/qca/qca-swiss-army-knife/blob/master/tools/scripts/ath11k/ath11k-check>
<https://github.com/qca/qca-swiss-army-knife/blob/master/tools/scripts/ath10k/ath10k-check>

/jeff