On Fri, Jan 24, 2020 at 09:59:48AM -0800, James Bottomley wrote:
> On Fri, 2020-01-24 at 19:38 +0200, Andy Shevchenko wrote:
> > On Fri, Jan 24, 2020 at 08:39:02AM -0800, James Bottomley wrote:
> > > On Fri, 2020-01-24 at 18:07 +0200, Andy Shevchenko wrote:
> > > > Replace open coded single-linked list iteration loop with
> > > > for_each_console()
> > > > helper in use.
> > > > - while ((console = console_drivers) != NULL)
> > > > - unregister_console(console_drivers);
> > > > + for_each_console(console)
> > > > + unregister_console(console);
> > >
> > > This is wrong. The old formulation iterates correctly in the face
> > > of element removal. for_each_console is defined:
> > >
> > > #define for_each_console(con) \
> > > for (con = console_drivers; con != NULL; con = con->next)
> > >
> > > So it's not safe for any iteration that alters the list elements.
> >
> > Ah, I see. In this case we need to keep a pointer to the next
> > element. Though, the original code assumes that console_drivers after
> > unregistration will be promoted to the next element. Do we have this
> > assumption solid?
>
> Yes, the original code simply removes the head until the list is empty.
> That's a recognized way of emptying any list while letting the remove
> code take care of the locking ... it works because parisc doesn't have
> a braille console.
By the way, consider this code from register_console()
for_each_console(bcon)
if (bcon->flags & CON_BOOT)
unregister_console(bcon);
It works based on assumption that next pointer of the just unregistered console
is not damaged. So, My initial patch will work in the same way.
--
With Best Regards,
Andy Shevchenko
