Re: [patch 2/16] list_head debugging

Jan Harkes
2002-06-03T13:55:35Z

On Sat, Jun 01, 2002 at 01:40:03AM -0700, Andrew Morton wrote:
> The patch nulls out the dangling pointers so we get a nice oops at the
> site of the buggy code.
...
> static __inline__ void list_del(struct list_head *entry)
> {
> __list_del(entry->prev, entry->next);
> + /*
> + * This is debug. Remove it when the kernel has no bugs ;)
> + */
> + entry->next = 0;
> + entry->prev = 0;
> }

We've had this before, and it breaks some code that removes items from
lists as follows,

list_for_each(p, list)
if (condition)
list_del(p);

These would have to either use __list_del, or need to do,

for(p = list.next; p != &list;) {
struct list_head *n = p->next;
if (condition)
list_del(p);
p = n;
}

I'm not sure how many places are using the list_for_each/list_del
construction, but there were a couple when this was in the tree
previously. Converting most places that use list_del to use
list_del_init should fix the same bugs, but not cause problems for
existing code.

Just did a grep for list_del and didn't find any obvious places where we
are doing the above construction, except for drivers/isdn/capi/capilib.c
and maybe drivers/hotplug/pcihp_skeleton.c, but it could be hidden in
many more places by a macro or function call (or a larger loop than my 3
line context was showing). But there are not that many places where
we're calling list_del while not immediately re-initializing or adding
the unlinked list_head to another list.

You could probably also add list_move

list_move(entry, head)
{
if (!list_empty(entry))
__list_del(entry->prev, entry->next);
list_add(entry, head);
}

And just delete list_del completely, because all existing places where
list_del is currently used should probably use either list_del_init, or
list_move.

Jan