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

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

[Guixin Liu]

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] checkpatch: trailing arithmetic after ACQUIRE_ERR bypasses ASSIGN_IN_IF
- [Medium] checkpatch: mixed compound assignment and ACQUIRE_ERR bypasses check
--

commit 82d84894b0a64b92c72367ea66d4270c5cb9cc86
Author: Guixin Liu <kanie@linux.alibaba.com>

checkpatch: don't flag ACQUIRE_ERR() assignments in if conditions

This commit updates checkpatch to skip the ASSIGN_IN_IF warning when the
assignments inside the condition are strictly from the ACQUIRE_ERR family of
macros. It attempts to ensure that plain assignments, compound assignments,
and mixed conditions still trigger the warning.

Link: https://lore.kernel.org/all/20260916020921.3480730-1-kanie@linux.alibaba.com/
Link: https://lore.kernel.org/linux-cxl/20250815010645.2980846-1-alison.schofield@intel.com/

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 3614cfe4dcbb4..598e3ed743bd9 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
[ ... ]
> @@ -5787,7 +5787,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*\()[^,)&|=]+/;

[Severity: Medium]
Does this regex allow trailing arithmetic to bypass the check?

Because the regex only checks the prefix of the right-hand side of the
assignment, a condition like if ((rc = ACQUIRE_ERR(...) + 1)) seems to start
with the allowed macro. This causes the negative lookahead to fail, preventing
it from being flagged as an other assignment.

This appears to contradict the commit message's claim about arithmetic on the
call result still triggering the check.

> +				$assign_in_if = !$has_assignment || $has_other_assignment;

[Severity: Medium]
Does this logic silently allow compound assignments when they are mixed with a
valid ACQUIRE_ERR() assignment?

An expression like if ((rc = ACQUIRE_ERR(...)) || (a += 1)) satisfies the
$has_assignment check, which disables the fail-safe. However, the compound
assignment does not trigger the $has_other_assignment check because it does
not match the strict equals sign in the regex.

This seems to contradict the commit message's statement that compound
assignments are not silently allowed.

> +			}
> +			if ($assign_in_if) {
>  				if (ERROR("ASSIGN_IN_IF",
>  					  "do not use assignment in if condition\n" . $herecurr) &&
>  				    $fix && $perl_version_ok) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924033923.4140210-1-kanie@linux.alibaba.com?part=1

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

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

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

[Alison Schofield]

On Thu, Sep 24, 2026 at 11:39:23AM +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>
> Cc: linux-cxl@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>


Reviewed-by: Alison Schofield <alison.schofield@intel.com>

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

[Jonathan Cameron]

On Thu, 24 Sep 2026 11:39:23 +0800
Guixin Liu <kanie@linux.alibaba.com> 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>
> Cc: linux-cxl@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
> 
I'm thoroughly in favour but seeing as I'd end up using an LLM
just to figure out what the actual code does no tags from me!

Jonathan