On Mon, 2022-02-28 at 23:59 +0200, Mike Rapoport wrote:

On February 28, 2022 10:42:53 PM GMT+02:00, James Bottomley <
James.Bottomley@HansenPartnership.com> wrote:

quoted

On Mon, 2022-02-28 at 21:07 +0100, Christian König wrote:

[...]

quoted

quoted

quoted

I do wish we could actually poison the 'pos' value after the
loop somehow - but clearly the "might be uninitialized" I was
hoping for isn't the way to do it.

Anybody have any ideas?

I think we should look at the use cases why code is touching
(pos) after the loop.

Just from skimming over the patches to change this and experience
with the drivers/subsystems I help to maintain I think the
primary pattern looks something like this:

list_for_each_entry(entry, head, member) {
     if (some_condition_checking(entry))
         break;
}
do_something_with(entry);

Actually, we usually have a check to see if the loop found
anything, but in that case it should something like

if (list_entry_is_head(entry, head, member)) {
   return with error;
}
do_somethin_with(entry);

Suffice?  The list_entry_is_head() macro is designed to cope with
the bogus entry on head problem.

Won't suffice because the end goal of this work is to limit scope of
entry only to loop. Hence the need for additional variable.

Well, yes, but my objection is more to the size of churn than the
desire to do loop local.  I'm not even sure loop local is possible,
because it's always annoyed me that for (int i = 0; ...  in C++ defines
i in the outer scope not the loop scope, which is why I never use it.

However, if the desire is really to poison the loop variable then we
can do

#define list_for_each_entry(pos, head, member)				\
	for (pos = list_first_entry(head, typeof(*pos), member);	\
	     !list_entry_is_head(pos, head, member) && ((pos = NULL) == NULL;			\
	     pos = list_next_entry(pos, member))

Which would at least set pos to NULL when the loop completes.

Besides, there are no guarantees that people won't
do_something_with(entry) without the check or won't compare entry to
NULL to check if the loop finished with break or not.

I get the wider goal, but we have to patch the problem cases now and a
simple one-liner is better than a larger patch that may or may not work
if we ever achieve the local definition or value poisoning idea.  I'm
also fairly certain coccinelle can come up with a use without checking
for loop completion semantic patch which we can add to 0day.

James