From: Guixin Liu <kanie@linux.alibaba.com>
To: Andy Whitcroft <apw@canonical.com>, Joe Perches <joe@perches.com>,
Jonathan Cameron <jic23@kernel.org>,
Alison Schofield <alison.schofield@intel.com>
Cc: linux-kernel@vger.kernel.org, linux-cxl@vger.kernel.org
Subject: [PATCH v2] checkpatch: don't flag ACQUIRE_ERR() assignments in if conditions
Date: Thu, 24 Sep 2026 11:39:23 +0800 [thread overview]
Message-ID: <20260924033923.4140210-1-kanie@linux.alibaba.com> (raw)
ACQUIRE_ERR() and its wrappers, PM_RUNTIME_ACQUIRE_ERR() and
IIO_DEV_ACQUIRE_FAILED(), report whether a conditional cleanup.h guard
was acquired, and drivers consume the result directly in an if
condition:
if ((rc = ACQUIRE_ERR(mutex_intr, &lock)))
return rc;
That combined form is the established style at the 49 in-tree call
sites under drivers/cxl and drivers/pci/tsm.c, so ASSIGN_IN_IF fires
there only as a false positive, and every patch touching those lines
carries noise that reviewers have to wave off manually.
Skip the check only when every assignment in the condition assigns the
result of such a call, matched by the *_ACQUIRE_ERR() /
*_ACQUIRE_FAILED() naming convention of its wrappers. Plain
assignments, mixed conditions and near-miss identifiers still get
flagged.
Suggested-by: Alison Schofield <alison.schofield@intel.com>
Cc: linux-cxl@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
Changes since v1 [1]:
- Check each assignment in the condition instead of the condition as a
whole, so a mixed condition such as
"if ((rc = regular_function()) || (ret = ACQUIRE_ERR(...)))" keeps
getting flagged (Alison Schofield).
- Match the assigned-to expression with $Lval, as Joe Perches
suggested on Alison's earlier attempt at this exception [2].
- Cc linux-cxl, where most users of this form live.
Two deviations from [2]: the assigned expression capture excludes '=',
so an '==' comparison in a mixed condition is not mistaken for a bad
assignment, and the check requires an assignment the $Lval match can
see, so compound assignments like '+=', which it cannot see, are not
silently allowed.
Testing: ran checkpatch on a test file with the allowed forms (bare
macro and both wrappers, member and array element lhs, multiple
ACQUIRE family assignments in one condition, an '==' comparison
alongside, no spaces around '=') and with 14 cases that must still
trigger (plain assignments, near-miss identifiers, arithmetic on the
call result, ACQUIRE_ERR in a comment, indirect assignment, mixed
conditions in both orders). All allowed forms pass, all 14 still
trigger. The 49 in-tree call sites under drivers/cxl and
drivers/pci/tsm.c no longer report ASSIGN_IN_IF, and checkpatch output
is unchanged for fs/namei.c, kernel/sched/core.c, mm/vmscan.c and
net/core/dev.c.
[1] https://lore.kernel.org/all/20260916020921.3480730-1-kanie@linux.alibaba.com/
[2] https://lore.kernel.org/linux-cxl/20250815010645.2980846-1-alison.schofield@intel.com/
---
scripts/checkpatch.pl | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 8a7787d228a6..eee06ac6737f 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5777,7 +5777,21 @@ sub process {
my ($s, $c) = ($stat, $cond);
my $fixed_assign_in_if = 0;
+ # ACQUIRE_ERR() and its wrappers, e.g. PM_RUNTIME_ACQUIRE_ERR()
+ # and IIO_DEV_ACQUIRE_FAILED(), are meant to be evaluated in an
+ # if condition, with the error assigned in the condition:
+ # if ((rc = ACQUIRE_ERR(name, &lock)))
+ # Allow that only when every assignment in the condition assigns
+ # the result of such a call, so that a mixed condition keeps
+ # getting flagged:
+ # if ((rc = regular_function()) || (ret = ACQUIRE_ERR(name, &lock)))
+ my $assign_in_if = 0;
if ($c =~ /\bif\s*\(.*[^<>!=]=[^=].*/s) {
+ my $has_assignment = $c =~ /\b$Lval\s*=\s*[^,)&|=]+/;
+ my $has_other_assignment = $c =~ /\b$Lval\s*=\s*(?!\s*\w*ACQUIRE_(?:ERR|FAILED)\s*\()[^,)&|=]+/;
+ $assign_in_if = !$has_assignment || $has_other_assignment;
+ }
+ if ($assign_in_if) {
if (ERROR("ASSIGN_IN_IF",
"do not use assignment in if condition\n" . $herecurr) &&
$fix && $perl_version_ok) {
--
2.43.7
next reply other threads:[~2026-09-24 3:39 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-24 3:39 Guixin Liu [this message]
2026-09-24 17:09 ` Joe Perches
2026-09-24 20:14 ` Alison Schofield
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260924033923.4140210-1-kanie@linux.alibaba.com \
--to=kanie@linux.alibaba.com \
--cc=alison.schofield@intel.com \
--cc=apw@canonical.com \
--cc=jic23@kernel.org \
--cc=joe@perches.com \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®