Thread (16 messages) 16 messages, 5 authors, 2024-10-07

Re: [PATCH v1] cleanup: adjust scoped_guard() to avoid potential warning

From: Andy Shevchenko <hidden>
Date: 2024-10-04 13:52:12
Also in: lkml

On Fri, Oct 04, 2024 at 11:33:08AM +0200, Peter Zijlstra wrote:
On Thu, Oct 03, 2024 at 08:51:46PM +0300, Andy Shevchenko wrote:
quoted
quoted
I would really like to understand why you don't like this; care to
elaborate Andy?
To me the idea of

int my_foo(...)
{
	NOT_my_foo_macro(...)
		return X;
}

is counter intuitive from C programming. Without knowing the magic behind the
scenes of NOT_my_foo_macro() I would eager to ask for adding a dead code like

int my_foo(...)
{
	NOT_my_foo_macro(...)
		return X;
	return 0;
}
Well, this is kernel coding, we don't really do (std) C anymore, and
using *anything* without knowing the magic behind it is asking for fail.
True in many cases, mostly for macros themselves, but in the functions
I would prefer to stay away from magic as possible.
Also, something like:

int my_foo()
{
	for (;;)
		return X;
}

or

int my_foo()
{
	do {
		return X;
	} while (0);
}

is perfectly valid C that no compiler should be complaining about. Yes
its a wee bit daft, but if you want to write it, that's fine.
Yes, the difference is that it's not hidden from the reader.
The code behind the macro is magic for the reader by default.
The point being that the compiler can determine there is no path not
hitting that return.

Apparently the current for loop is defeating the compiler, I see no
reason not to change it in such a way that the compiler is able to
determine wtf happens -- that can only help.
quoted
What I would agree on is

int my_foo(...)
{
	return NOT_my_foo_macro(..., X);
}
That just really won't work with things as they are ofcourse.
quoted
Or just using guard()().
Okay, thanks for sharing your view on this. Since you are the author
of the original code and seems fine with a change, I can't help myself
from withdrawing my NACK. OTOH, I am not going to give an Ack either.
That's always an option. You don't *have* to use the -- to you -- weird
form.
Yes, I am not convinced that using scoped_guard() (or any other macro)
in a described way is okay. Definitely, *I am* not going to do a such
until I understand the real benefit of it.

-- 
With Best Regards,
Andy Shevchenko

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help