Re: [PATCH v2] checkpatch: don't flag ACQUIRE_ERR() assignments in if conditions

[Alison Schofield <alison.schofield@intel.com>]

On Wed, Oct 28, 2026 at 10:21:51AM +0800, Guixin Liu wrote:
> Hi Joe,
>   Could you please apply this patch to 7.4?
> 
> Best Regards,
> Guixin Liu

Already applied by Andrew :)
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d25f15a76c7a7f0e20a2b4b36f5108ef115a6d1f


> 
> 在 2026/9/25 01:09, Joe Perches 写道:
> > On Thu, 2026-09-24 at 11:39 +0800, Guixin Liu wrote:
> >> 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](mailto:alison.schofield@intel.com)>
> >> Cc: [linux-cxl@vger.kernel.org](mailto:linux-cxl@vger.kernel.org)
> >> Assisted-by: LLM
> >> Signed-off-by: Guixin Liu <[kanie@linux.alibaba.com](mailto:kanie@linux.alibaba.com)>
> > Acked-by: Joe Perches <joe@perches.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/](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/](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) {
> >>
>