Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Why do we need list_for_each_safe() in for deleting nodes in kernel linked list?

I'm learning how to use the kernel linked-list API from list.h.

I learned that I need to use list_for_each_safe() when deleting nodes off with list_del() instead of using list_for_each().

Code for list_for_each_safe():

#define list_for_each_safe(pos, n, head) \
    for (pos = (head)->next, n = pos->next; pos != (head); \
        pos = n, n = pos->next)

Code for list_for_each():

    for (pos = (head)->next; pos != (head); pos = pos->next)

I notice they both are very similar except that the _safe version takes an extra argument to be used as 'temporary storage' (stated here, list.h).

I understand when to apply the function correcly, _safe version for deleting, normal version for accessing, but I'm curious how the extra argument made it 'safe'?

Consider the following, where I'm deleting every node in a linked list using list_for_each_safe():

struct kool_list{
    int to;
    struct list_head list;
    int from;
    };

struct kool_list *tmp;
struct list_head *pos, *q;
struct kool_list mylist;

list_for_each_safe(pos, q, &mylist.list){
         tmp= list_entry(pos, struct kool_list, list);
         printf("freeing item to= %d from= %d\n", tmp->to, tmp->from);
         list_del(pos);
         free(tmp);
    }

How does giving q help in deleting?

Thanks for any help!

like image 412
I'm a frog dragon Avatar asked Feb 09 '12 08:02

I'm a frog dragon


2 Answers

That is necessary because list_del internally modifies the value of pos fields. In your example the loop body even frees the memory occupied by pos. Suppose that you would use unsafe version of the loop:

for (pos = (head)->next; pos != (head); pos = pos->next)

After executing the loop body pos pointer becomes invalid breaking the increment expression: pos = pos->next.

As opposite, the safe foreach pre-saves the value of pos->next in a temporary variable and then refers to the latter instead of dereferencing pos:

for (pos = (head)->next, n = pos->next; pos != (head); \
    pos = n, n = pos->next)
like image 70
Eldar Abusalimov Avatar answered Nov 06 '22 21:11

Eldar Abusalimov


pos = start;
del(pos);
pos = pos->next;

as opposed to

pos = start;
n = pos->next;
del(pos);
pos = n;

if del() is free() and memset(), pos->next is undefined

like image 23
Peter Miehle Avatar answered Nov 06 '22 20:11

Peter Miehle