Re: [PATCH 0/3] using guard/__free in networking

Re: [PATCH 0/3] using guard/__free in networking

[Johannes Berg]

On Wed, 2024-03-27 at 21:25 +0100, Jakub Sitnicki wrote:
> On Tue, Mar 26, 2024 at 04:33 PM +01, Johannes Berg wrote:
> > > Is it also present in Rust or some such?
> > 
> > I have no idea. I _think_ Rust actually ties the data and the locks
> > together more?
> 
> That is right. Nicely explained here:
> 
> https://marabos.nl/atomics/basics.html#rusts-mutex

Right.

Thinking about that, we _could_ even add support for drop_guard()?

With the below patch to cleanup.h, you can write

void my_something(my_t *my)
{
...
	named_guard(lock, mutex)(&my->mutex);
...
	if (foo)
		return -EINVAL; // automatically unlocks
...
	// no need for lock any more
	drop_guard(lock);
...
	// do other things now unlocked
}


Is that too ugly? Maybe it's less Java-like and more Rust-like and
better for Jakub ;-)

In some sense that's nicer than scoped_guard() since it doesn't require
a new scope / indentation, but maybe Peter already thought about it and
rejected it :-)


Patch follows, though maybe that should be rolled into the 'base' CLASS
definition instead of defining another "_drop" one for named_guard()?

diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
index c2d09bc4f976..31298106c28b 100644
--- a/include/linux/cleanup.h
+++ b/include/linux/cleanup.h
@@ -106,7 +106,27 @@ typedef _type class_##_name##_t;					\
 static inline void class_##_name##_destructor(_type *p)			\
 { _type _T = *p; _exit; }						\
 static inline _type class_##_name##_constructor(_init_args)		\
-{ _type t = _init; return t; }
+{ _type t = _init; return t; }						\
+typedef struct class_##_name##_drop##_t {				\
+	class_##_name##_t obj;						\
+	void (*destructor)(struct class_##_name##_drop##_t *);		\
+} class_##_name##_drop##_t;						\
+static inline void							\
+class_##_name##_drop##_destructor(class_##_name##_drop##_t *p)		\
+{									\
+	if (p->obj)							\
+		class_##_name##_destructor(&p->obj);			\
+	p->obj = NULL;							\
+}									\
+static inline class_##_name##_drop##_t					\
+class_##_name##_drop##_constructor(_init_args)				\
+{									\
+	class_##_name##_drop##_t t = {					\
+		.obj = _init,						\
+		.destructor = class_##_name##_drop##_destructor,	\
+	};								\
+	return t;							\
+}
 
 #define EXTEND_CLASS(_name, ext, _init, _init_args...)			\
 typedef class_##_name##_t class_##_name##ext##_t;			\
@@ -163,6 +183,11 @@ static inline class_##_name##_t class_##_name##ext##_constructor(_init_args) \
 #define guard(_name) \
 	CLASS(_name, __UNIQUE_ID(guard))
 
+#define named_guard(_name, _class) \
+	CLASS(_class##_drop, _name)
+
+#define drop_guard(_name) do { _name.destructor(&_name); } while (0)
+
 #define __guard_ptr(_name) class_##_name##_lock_ptr
 
 #define scoped_guard(_name, args...)					\

Re: [PATCH 0/3] using guard/__free in networking

[Johannes Berg]

On Wed, 2024-03-27 at 22:28 +0100, Johannes Berg wrote:
> 
> +typedef struct class_##_name##_drop##_t {				\
> +	class_##_name##_t obj;						\
> +	void (*destructor)(struct class_##_name##_drop##_t *);		\
> +} class_##_name##_drop##_t;						\

No, I misread the compiler output, it does output a real destructor
function to push it into a stack variable with this...

So I guess it'd have to be

void my_something(my_t *my)
{
...
	named_guard(lock, mutex)(&my->mutex);
...
	if (foo)
		return -EINVAL; // automatically unlocks
...
	// no need for lock any more
	drop_guard(lock, mutex);
...
	// do other things now unlocked
}


instead, syntax-wise.


Which obviously simplifies the changes:


diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
index c2d09bc4f976..cf39a4a3f56f 100644
--- a/include/linux/cleanup.h
+++ b/include/linux/cleanup.h
@@ -163,6 +163,12 @@ static inline class_##_name##_t class_##_name##ext##_constructor(_init_args) \
 #define guard(_name) \
 	CLASS(_name, __UNIQUE_ID(guard))
 
+#define named_guard(_name, _class) \
+	CLASS(_class, _name)
+
+#define drop_guard(_name, _class) \
+	do { class_##_class##_destructor(&_name); _name = NULL; } while (0)
+
 #define __guard_ptr(_name) class_##_name##_lock_ptr
 
 #define scoped_guard(_name, args...)					\