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

[Joe Perches <joe@perches.com>]

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) {
>