mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] checkpatch: don't flag ACQUIRE_ERR() assignments in if conditions
@ 2026-09-24  3:39 Guixin Liu
  2026-09-24 17:09 ` Joe Perches
  0 siblings, 1 reply; 2+ messages in thread
From: Guixin Liu @ 2026-09-24  3:39 UTC (permalink / raw)
  To: Andy Whitcroft, Joe Perches, Jonathan Cameron, Alison Schofield
  Cc: linux-kernel, linux-cxl

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


^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-09-24 17:14 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-24  3:39 [PATCH v2] checkpatch: don't flag ACQUIRE_ERR() assignments in if conditions Guixin Liu
2026-09-24 17:09 ` Joe Perches

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®