Am 28.02.22 um 22:13 schrieb James Bottomley:
On Mon, 2022-02-28 at 21:56 +0100, Christian König wrote:
Am 28.02.22 um 21:42 schrieb James Bottomley:
On Mon, 2022-02-28 at 21:07 +0100, Christian König wrote:
Am 28.02.22 um 20:56 schrieb Linus Torvalds:
On Mon, Feb 28, 2022 at 4:19 AM Christian König
<christian.koenig@xxxxxxx> wrote:
[SNIP]
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.
That will work and is also what people already do.
The key problem is that we let people do the same thing over and
over again with slightly different implementations.
Out in the wild I've seen at least using a separate variable, using
a bool to indicate that something was found and just assuming that
the list has an entry.
The last case is bogus and basically what can break badly.
Yes, I understand that. I'm saying we should replace that bogus checks
of entry->something against some_value loop termination condition with
the list_entry_is_head() macro. That should be a one line and fairly
mechanical change rather than the explosion of code changes we seem to
have in the patch series.
Yes, exactly that's my thinking as well.
Christian.
James
