From: Alison Schofield <alison.schofield@intel.com>

New helpers, ACQUIRE() and ACQUIRE_ERR(), were recently introduced
and employed here [1] to clean up conditional locking paths.

That led to checkpatch ERRORS:
        ERROR: do not use assignment in if condition

on usages like this:
        ACQUIRE(rwsem_write_kill, rwsem)(&cxl_rwsem.region);
        if ((rc = ACQUIRE_ERR(rwsem_write_kill, &rwsem)))
                return rc;

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.

[1] Commit d03fcf50ba56 ("cxl: Convert to ACQUIRE() for conditional rwsem locking")

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

Changes in v2:
- Remove next that skipped other rules (Joe)
- Replace \w+ with $Lval for more precise pattern match (Joe)
- Only allow lines where all assignments are to ACQUIRE_ERR
- Tested many more allow and trigger conditions.
  Will share in reply to this post.
- Update commit message

 scripts/checkpatch.pl | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index e722dd6fa8ef..30435967d8c4 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5696,7 +5696,18 @@ sub process {
 			my ($s, $c) = ($stat, $cond);
 			my $fixed_assign_in_if = 0;
 
-			if ($c =~ /\bif\s*\(.*[^<>!=]=[^=].*/s) {
+			if ($c =~ /\bif\s*\((.*[^<>!=]=[^=].*)\)/s) {
+			    my $condition = $1;
+			    my $allow_assignment = 1;
+
+			    # Allow single ACQUIRE_ERR assignment, reject everything else
+			    while ($condition =~ /\b($Lval)\s*=\s*([^,)&|]+)/g) {
+				 if ($2 !~ /^\s*ACQUIRE_ERR\s*\(/) {
+					$allow_assignment = 0;
+					last;
+				}
+			    }
+			    if (!$allow_assignment) {
 				if (ERROR("ASSIGN_IN_IF",
 					  "do not use assignment in if condition\n" . $herecurr) &&
 				    $fix && $perl_version_ok) {
@@ -5721,6 +5732,7 @@ sub process {
 						fix_insert_line($fixlinenr + 1, $newline);
 						$fixed_assign_in_if = 1;
 					}
+				    }
 				}
 			}
 

base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
-- 
2.37.3

On Thu, Aug 14, 2025 at 06:06:43PM -0700, alison.schofield@intel.com wrote:
> From: Alison Schofield <alison.schofield@intel.com>

Appended is the file.c used in testing. It's annotated with what is
expected to pass thru vs trigger.


/*
 * Test file for checkpatch.pl ACQUIRE_ERR exception
 * This file contains various assignment-in-if patterns to verify
 * that the checkpatch modification works correctly.
 *
 * Usage: ./scripts/checkpatch.pl --file this_filename.c --strict 
 *
 * Expect: total: 16 errors, 3 warnings, 1 checks
 */

/*
 * Valid ACQUIRE_ERR() usage patterns that should be allowed
 * Should all pass,  no checkpatch errors
 */
int test_cases_that_should_pass(void)
{
	/* Basic ACQUIRE_ERR usage - should be allowed */
	if ((rc = ACQUIRE_ERR(rwsem_write_kill, &rwsem_ptr)))
		return rc;

	/* ACQUIRE_ERR with different variable names - should be allowed */
	if ((ret = ACQUIRE_ERR(mutex_lock, &obj->ptr)))
		goto error_exit;

	/* ACQUIRE_ERR with longer variable names - should be allowed */
	if ((return_code = ACQUIRE_ERR(spinlock_acquire, rwsem_ptr)))
		return return_code;

	/* ACQUIRE_ERR with struct member assignment - should be allowed */
	if ((obj->value = ACQUIRE_ERR(lock_type, &rwsem_ptr)))
		return -EBUSY;

	/* Different whitespace patterns - should all be allowed */
	if ((rc = ACQUIRE_ERR(lock_type, &ptr)))
		return rc;

	if ((rc = ACQUIRE_ERR(lock_type, &ptr)))
		return rc;

	if ((rc = ACQUIRE_ERR(lock_type, &ptr)))
		return rc;

	/* Multi-line ACQUIRE_ERR - should be allowed */
	if ((rc = ACQUIRE_ERR(very_long_lock_type_name,
			      &ptr)))
		return rc;

	/* Multiple ACQUIRE_ERR assignments - should be allowed */
	if ((rc = ACQUIRE_ERR(lock_type, &ptr)) || (ret = ACQUIRE_ERR(other_lock, &rwsem_ptr)))
		return rc;

	/* ACQUIRE_ERR in complex but all-ACQUIRE_ERR expression - should be allowed */
	if ((obj->value = ACQUIRE_ERR(lock1, &ptr)) && (rc = ACQUIRE_ERR(lock2, &rwsem_ptr)))
		return rc;

	/* Comparison operators - should be allowed */
	if (rc == ACQUIRE_ERR(lock_type, &rwsem_ptr))
		return rc;

	if (rc != ACQUIRE_ERR(lock_type, &rwsem_ptr))
		return rc;

	if (rc < ACQUIRE_ERR(lock_type, &rwsem_ptr))
		return rc;

	/* Function calls without assignment - should be allowed */
	if (ACQUIRE_ERR(lock_type, &rwsem_ptr))
		return -EBUSY;

	/* Regular conditionals - should be allowed */
	if (rc)
		return rc;

	if (obj && obj->value)
		return obj->value;

	return 0;
}

/*
 * Should all trigger ERROR or WARNING or CHECK
 *
 * Non-ACQUIRE_ERR assignments that should still be caught
 * And, cases that pass ASSIGN_IN_IF, but fail other rules
 */
int test_cases_that_should_fail(void)
{
	/* ERROR: do not use assignment in if condition */
	if ((rc = regular_function()))
		return rc;

	/* ERROR: do not use assignment in if condition */
	if ((value = obj->value))
		return value;

	/* ERROR: do not use assignment in if condition */
	if ((rc = value + 1))
		return rc;

	/* ERROR: do not use assignment in if condition */
	if ((rc = ACQUIRE(rwsem_write_kill, &rwsem_ptr)))
		return rc;

	/* ERROR: do not use assignment in if condition */
	if ((rc = some_ACQUIRE_ERR_helper_function()))
		return rc;

	/* ERROR: do not use assignment in if condition */
	ret = ACQUIRE_ERR(lock_type, &rwsem_ptr);
	if ((rc = ret))
		return rc;

	/* ERROR: do not use assignment in if condition */
	if ((rc = regular_function())) /* ACQUIRE_ERR pattern in comment */
		return rc;

	/* ERROR: do not use assignment in if condition */
	if ((rc = regular_function() + ACQUIRE_ERR(lock_type, &rwsem_ptr)))
		return rc;

	/* ERROR: do not use assignment in if condition */
	if ((rc = regular_function()) || (ret = ACQUIRE_ERR(lock_type, &rwsem_ptr)))
		return rc;

	/* ERROR: do not use assignment in if condition */
	if ((ret = ACQUIRE_ERR(lock_type, &rwsem_ptr)) || (rc = regular_function()))
		return rc;

	/* ERROR: do not use assignment in if condition */
	if ((rc = regular_function()) || (value = obj->value))
		return rc;

	/* ERROR: do not use assignment in if condition */
	if ((rc = regular_function()) && (value = 22))
		return rc;

	/* Next set should pass the ASSIGN_IN_IF check but fail other rules */

	/* ERROR: trailing whitespace */
	/* WARNING: please, no space before tabs */
	if ((rc = ACQUIRE_ERR(lock_type, &ptr)))   	
		return rc;

	/* ERROR: space required before the open brace '{' */
	/* WARNING: braces {} are not necessary for single statement blocks */
	if ((rc = ACQUIRE_ERR(lock_type, &ptr))){
		return rc;
	}

	/* CHECK: line length of 150 exceeds 100 columns */
	if ((rc = ACQUIRE_ERR(very_very_very_very_very_very_very_very_very_very_very_very_long_lock_type_name_that_exceeds_typical_line_limit, &ptr)))
		return rc;

	/* ERROR: space required after that ',' (ctx:VxO) */
	/* ERROR: space required before that '&' (ctx:OxV) */
	if ((rc = ACQUIRE_ERR(lock_type,&ptr)))
		return rc;
}