linux-kernel - Re: [PATCH v2] 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: <68ed99a5b6308_1992810024@dwillia2-mobl4.notmuch>
Date: Mon, 13 Oct 2025 17:30:29 -0700
From: <dan.j.williams@...el.com>
To: <alison.schofield@...el.com>, Joe Perches <joe@...ches.com>, "Andy
 Whitcroft" <apw@...onical.com>, Dwaipayan Ray <dwaipayanray1@...il.com>,
	"Lukas Bulwahn" <lukas.bulwahn@...il.com>
CC: Alison Schofield <alison.schofield@...el.com>, 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 v2] checkpatch: allow an assignment in if condition for
 ACQUIRE_ERR()

alison.schofield@ wrote:
> From: Alison Schofield <alison.schofield@...el.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")

Works for me.

Tested-by: Dan Williams <dan.j.williams@...el.com>

I am going to be resubmitting the final (hopefully) version of the
PCI/TSM core implementation [1] which uses this compact style.

It takes the current version of the patch from:

    total: 4 errors, 2 warnings, 1074 lines checked

...to:

    total: 0 errors, 2 warnings, 1074 lines checked

Where one of those warnings I overlooked previously due to the other
errors. The final warning in that set is:

    WARNING: suspect code indent for conditional statements (0, 0)
    #380: FILE: drivers/pci/tsm.c:72:
    +DEFINE_FREE(tsm_remove, struct pci_tsm *, if (_T) tsm_remove(_T))
    [...]
    +static void pci_tsm_walk_fns(struct pci_dev *pdev,

Which is another cleanup.h usage enlightenment for checkpatch.pl that
would be nice to see.

[1]: http://lore.kernel.org/20250911235647.3248419-5-dan.j.williams@intel.com