Messages in this thread | | | Date | Mon, 3 Jun 2002 09:55:35 -0400 | Subject | Re: [patch 2/16] list_head debugging | From | Jan Harkes <> |
| |
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
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |