Re: [RFC PATCH v2] Simply enable one to write code like:

On 10/1/24 17:21, Dmitry Torokhov wrote:

Hi Przemek,

On Tue, Oct 01, 2024 at 04:57:18PM +0200, Przemek Kitszel wrote:

quoted

int foo(struct my_drv *adapter)
{
	scoped_guard(spinlock, &adapter->some_spinlock)
		return adapter->spinlock_protected_var;
}

Could you change the subject to say something like:

"Adjust cond_guard() implementation to avoid potential warnings"

And then give detailed explanation in the body?

thanks, sure
(and apologies that I forgot to add any subject :F (this was just my
very first non-subject paragraph))

quoted

Current scoped_guard() implementation does not support that,
due to compiler complaining:
error: control reaches end of non-void function [-Werror=return-type]

One could argue that for such use case it would be better to use
guard(spinlock)(&adapter->some_spinlock), I disagree. I could also say
that coding with my proposed locking style is also very pleasant, as I'm
doing so for a few weeks already.

I'd drop this paragraph from the patch description (and moved past "---"
if you prefer to keep it for additional context.

I will think about that, especially given that since v2 this patch is
not only fixing "my case", but just it's regular hardening for static
analysis needs.

quoted

+#define DEFINE_CLASS_IS_CONDITIONAL(_name, _is_cond)	\

This is not supposed to be used outside of cleanup.h so probably
__DEFINE_CLASS_IS_CONDITIONAL()?

indeed

quoted

+#define __scoped_guard_labeled(_label, _name, args...)	\
+	if (0)						\
+		_label: ;				\
+	else						\
+		for (CLASS(_name, scope)(args);		\
+		     __guard_ptr(_name)(&scope) || !__is_cond_ptr(_name); \
+		     ({goto _label;}))

The "jump back" throws me a little, do you think if can be rewritten as:

	if (true)
		for (...)
	else
		_label: /* dummy */ ;

user code must be glued at the end, so there must be "if (0) label:"
however I figured that you could reorder for and else:

	for (
		CLASS(...);
		__guard_ptr(...) || __is_cond_ptr(...);
		({ goto label; })
	)
		if (0)
			label:
				break;
		else
			// actual user code glued here

and this jumps forward

quoted

  
  #define scoped_cond_guard(_name, _fail, args...) \
  	for (CLASS(_name, scope)(args), \

With your __is_cond_ptr() can this be made to warn or error if
scoped_cond_guard() is used with a non-conditional lock/class? As that
would make no sense.

good idea, thanks