Hi Przemek,
On Tue, Oct 01, 2024 at 04:57:18PM +0200, Przemek Kitszel wrote:
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?
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.
+#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()?
+#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 */ ;
#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.