On Mon, Nov 25, 2024 at 09:51:43AM -0800, Linus Torvalds wrote:
> That said, I have a "lovely" suggestion. Instead of the "if(0)+goto"
> games, I think you can just do this:
>
> #define scoped_guard(_name, args...) \
> for (CLASS(_name, scope)(args), *_once = (void *)1; _once && \
> (__guard_ptr(_name)(&scope) || !__is_cond_ptr(_name)); \
> _once = NULL)
>
Right, so that's more or less what we used to have:
#define scoped_guard(_name, args...) \
for (CLASS(_name, scope)(args), *done = NULL; \
__guard_ptr(_name)(&scope) && !done; done = (void *)1)
But it turns out the compilers have a hard time dealing with this. From
commit fcc22ac5baf0 ("cleanup: Adjust scoped_guard() macros to avoid
potential warning"):
int foo(struct my_drv *adapter)
{
scoped_guard(spinlock, &adapter->some_spinlock)
return adapter->spinlock_protected_var;
}
Using that (old) form results in:
error: control reaches end of non-void function [-Werror=return-type]
Now obviously the above can also be written like:
int foo(struct my_drv *adapter)
{
guard(spinlock)(&adapter->some_spinlock);
return adapter->spinlock_protected_var;
}
But the point is to show the compilers get confused. Additionally Dan
Carpenter noted that smatch has a much easier time dealing with the new
form.
And the commit notes the generated code is actually slighly smaller with
e new form too (probably because the compiler is less confused about
control flow).
Except of course, now we get that dangling-else warning, there is no
winning this :-/
So I merged that patch because of the compilers getting less confused
and better code-gen, but if you prefer the old one we can definitely go
back. In which case we should go talk to compiler folks to figure out
how to make it not so confused.
