linux-kernel - Re: [PATCH] checkpatch: allow an assignment in if condition for ACQUIRE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <9afd4c898a194cd576f1aaa08b72c67d4e538cc7.camel@perches.com>
Date: Wed, 13 Aug 2025 00:17:51 -0700
From: Joe Perches <joe@...ches.com>
To: alison.schofield@...el.com, Andy Whitcroft <apw@...onical.com>, 
 Dwaipayan Ray <dwaipayanray1@...il.com>, Lukas Bulwahn
 <lukas.bulwahn@...il.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Dan Williams
	 <dan.j.williams@...el.com>, linux-cxl@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] checkpatch: allow an assignment in if condition for
 ACQUIRE_ERR()

On Tue, 2025-08-12 at 17:38 -0700, alison.schofield@...el.com wrote:
> From: Alison Schofield <alison.schofield@...el.com>
> 
> New helpers, ACQUIRE() and ACQUIRE_ERR(), were recently introduced to
> clean up conditional locking paths [1].
[]
> That compact format was a deliberate choice by the authors. By making
> this a checkpatch exception, existing ERRORs are quieted, and future
> users of the macro will not be dissuaded by checkpatch from using the
> preferred compact format.

not stylish IMO.

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -5696,7 +5696,14 @@ sub process {
>  			my ($s, $c) = ($stat, $cond);
>  			my $fixed_assign_in_if = 0;
>  
> -			if ($c =~ /\bif\s*\(.*[^<>!=]=[^=].*/s) {
> +			if ($c =~ /\bif\s*\((.*[^<>!=]=[^=].*)\)/s) {
> +				my $expr = $1;
> +
> +				# Allow ACQUIRE_ERR() macro syntax
> +				if ($expr =~ /\w+\s*=\s*ACQUIRE_ERR\s*\(/) {
> +					next;
> +				}

nak.

Using next would not do any additional checks on this line.
Likely use basic block or a constuct like

			if ($c =~ /\bif\s*\((.*[^<>!=]=[^=].*)/s &&
			    $1 !~ /\bACQUIRE_ERR\b/) {


and perhaps the \w+ should be $Lval

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