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.
James
