Re: [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

[Christian König]

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