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

[sashiko-bot@kernel.org]

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