linux-kernel - Re: [PATCH v0] [RFC] cleanup: Unify DEFINE_LOCK_GUARD_0 and DEFINE_LOCK_GUARD

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: <4513F5D8-A3DF-4477-B54D-D9D614DDC2AF@gmail.com>
Date: Tue, 17 Jun 2025 16:13:26 +0800
From: Jemmy Wong <jemmywong512@...il.com>
To: Dan Williams <dan.j.williams@...el.com>
Cc: Jemmy <jemmywong512@...il.com>,
 Peter Zijlstra <peterz@...radead.org>,
 Thomas Gleixner <tglx@...utronix.de>,
 Christian Brauner <brauner@...nel.org>,
 Jonathan Cameron <Jonathan.Cameron@...wei.com>,
 Al Viro <viro@...iv.linux.org.uk>,
 Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
 David Lechner <dlechner@...libre.com>,
 Przemek Kitszel <przemyslaw.kitszel@...el.com>,
 Ingo Molnar <mingo@...nel.org>,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v0] [RFC] cleanup: Unify DEFINE_LOCK_GUARD_0 and
 DEFINE_LOCK_GUARD_1

Hi Dan,

> On Jun 17, 2025, at 2:54 AM, Dan Williams <dan.j.williams@...el.com> wrote:
> 
> Jemmy Wong wrote:
>> Hi,
>> 
>> This patch consolidates the DEFINE_LOCK_GUARD_0 and DEFINE_LOCK_GUARD_1
>> macros into a single, unified 'DEFINE_LOCK_GUARD' macro to provide
>> a consistent and simplified API for lock guard definitions.
>> 
>> API changes:
>> From    DEFINE_LOCK_GUARD_0(name, lock, unlock, ...)
>> to      DEFINE_LOCK_GUARD(name, *void*, lock, unlock, ...)
>> 
>> From    DEFINE_LOCK_GUARD_1(name, type, lock, unlock, ...)
>> to      DEFINE_LOCK_GUARD(name, type, lock, unlock, ...)
>> 
>> From    CLASS(name, var)(args...)
>> to      CLASS(name, var, args...)
>> 
>> From    guard(name)(args)
>> to      guard(name, args)
> 
> No, I think this organization is instructive for understanding how these
> helpers work. I.e.  that the macro is instantiating a function with an
> automatic variable result, and the arguments to that function arrive in
> @args. This becomes even more important to understand with the ACQUIRE()
> and ACQUIRE_ERR() proposal that instantiate different functions to
> retrieve other properties of the automatic variable result.
> 
>> No change:
>> scoped_guard(name, args...)
>> scoped_cond_guard(name, fail, args...)
> 
> Effectively these are not returning an automatic variable result to the
> current scope and the different calling convention is consistent with
> that difference.

I have some concerns about this point.

Both guard and scoped_guard use CLASS(...) to instantiate an anonymous automatic variable, 
differing only in their scope, while CLASS instantiates a named automatic variable.

To ensure consistency, I believe guard, scoped_guard and CLASS should share the same calling convention.
This change would unify their syntax to: 
CLASS(name, var, args...), guard(name, args...) and scoped_guard(name, args...), 
improving consistency and clarity.

In most cases, an anonymous automatic guard variable suffices to manage object lifecycles, 
aligning with object-oriented programming languages that inherently support automatic lifecycle management 
(constructing objects at initialization and destructing them upon scope exit). 

For example, anonymous object instantiation in other languages includes:
C++: 		People("Alice", 30)
Python: 	People("Alice", 30)
C#: 		new People("Alice", 30)
PHP: 		new People("Alice", 30)
Jave: 		new People("Alice", 30)
JaveScript:	new People("Alice", 30)

This unified syntax would make the API more intuitive and consistent with established programming practices.

>> ---
>> 
>> Deailted changes:
>> 
>> - DEFINE_CLASS(_name, _type, _exit, _init, _init_args...)
>> The void type for _init_args is not required when the constructor takes no arguments,
>> as an int argc is implicitly inserted as the first argument. (int argc, void) is an error.
>> 
>> This patch includes only the core changes.
>> Follow-up patches will be submitted once the approach is accepted.
> 
> Appreciate the RFC first to avoid the thrash while deciding on the
> format change, but it is a nak from me.

Best,
Jemmy