On 2025-10-17 06:09, Thomas Gleixner wrote:

+/**
+ * __scoped_user_access_begin - Start the masked user access
+ * @_mode:	The mode of the access class (read, write, rw)
+ * @_uptr:	The pointer to access user space memory
+ * @_size:	Size of the access
+ * @_elbl:	Error label to goto when the access region is rejected.
+ *
+ * Internal helper for __scoped_masked_user_access(). Don't use directly
+ */

^ general comment about ending sentences with '.' across this patch
(nit).

+#define __scoped_user_access_begin(_mode, _uptr, _size, _elbl)		\
+({									\
+	typeof((_uptr)) ____ret;					\
+									\
+	if (can_do_masked_user_access()) {				\
+		____ret = masked_user_##_mode##_access_begin((_uptr));	\

I don't think the extra () are needed here, or is there something
special happening within this macro that requires it ?

+	} else {							\
+		____ret = _uptr;					\
+		if (!user_##_mode##_access_begin(_uptr, (_size)))	\

likewise around _size.

+*/
+#define __scoped_masked_user_access(_mode, _uptr, _size, _elbl)					\
+for (bool ____stop = false; !____stop; ____stop = true)						\
+	for (typeof((_uptr)) _tmpptr = __scoped_user_access_begin(_mode, _uptr, _size, _elbl);	\

The extra () around _uptr seems useless.

+	     !____stop; ____stop = true)							\
+		for (CLASS(masked_user_##_mode##_access, scope) (_tmpptr); !____stop;		\

Removing the space before (_tmpptr) would make it clearer that it
behaves as arguments to CLASS(masked_user_##_mode##_access, scope),
similarly to what is done in cleanup.h:scoped_class().

Nesting those constructs will cause variables to be hidden by inner
definitions. I recommend using __UNIQUE_ID() to make sure the "stop" and
"tmpptr" variables don't clash with external ones rather than trying to
solve the issue with a random amount of leading underscores.

+		     ____stop = true)					\
+			/* Force modified pointer usage within the scope */			\
+			for (const typeof((_uptr)) _uptr = _tmpptr; !____stop; ____stop = true)	\

I'm puzzled that it does not trigger compiler warnings as it shadows
_uptr if _uptr is a variable defined outside of this scope.

+				if (1)
+

^ can be removed (as pointed out by someone else already).

[...]

+#define scoped_masked_user_read_access_size(_usrc, _size, _elbl)		\
+	__scoped_masked_user_access(read, (_usrc), (_size), _elbl)

Useless () around _usrc and _size.

+#define scoped_masked_user_read_access(_usrc, _elbl)				\
+	scoped_masked_user_read_access_size((_usrc), sizeof(*(_usrc)), _elbl)

() around the first argument are useless.

+#define scoped_masked_user_write_access_size(_udst, _size, _elbl)		\
+	__scoped_masked_user_access(write, (_udst),  (_size), _elbl)

Useless () around _udst and _size.

+ */
+#define scoped_masked_user_write_access(_udst, _elbl)				\
+	scoped_masked_user_write_access_size((_udst), sizeof(*(_udst)), _elbl)

() around the first argument are useless.

+#define scoped_masked_user_rw_access_size(_uptr, _size, _elbl)			\
+	__scoped_masked_user_access(rw, (_uptr), (_size), _elbl)

Useless () around _uptr and _size.

+#define scoped_masked_user_rw_access(_uptr, _elbl)				\
+	scoped_masked_user_rw_access_size((_uptr), sizeof(*(_uptr)), _elbl)

() around the first argument are useless.

Thanks,

Mathieu


-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com