How to free a linked list in C

Question

I am making a linked list in C and need some help freeing it. The structures for the elements and the whole list look like this:

typedef struct node_ {
    int num;
    struct node_ *next;
    struct node_ *prev;
} element;


typedef struct list_ {
    element **first;
    element **last;
    int *e_count;
} list;

Here is my problem with freeing the list. This code works fine:

void wipe(list l) {
    element *p = *l.first;
    element *t;

    while (p) {
        t = p;
        p = p->next;
        free(t);
    }

    *l.first = NULL;
    *l.last = NULL;
    *l.e_count = 0;
}

But when I try to use double pointers, I start getting an InvalidRead error from valgrind:

void wipe(list l) {
    element **p = l.first;
    element **t = l.first;

    while (*p) { // InvalidRead here after "free(*t);"
        *t = *p;
        p = &(*p)->next;
        free(*t);
    }

    *l.first = NULL;
    *l.last = NULL;
    *l.e_count = 0;
}

This is expected because in this case t and p are essentially pointing to the same address which is then being used by free(). I am wondering how is it possible to fix this and whether it would be better than the first case with the single pointers.

Thank you for your help!

Code example:

#include <stdlib.h>
#include <stdio.h>


typedef struct node_ {
    int num;
    struct node_ *next;
    struct node_ *prev;
} element;


typedef struct list_ {
    element **first;
    element **last;
    int *e_count;
} list;


list new_list();
void delete_list(list l);

element *alloc_element(int n);

int append(list l, int n);
void wipe(list l);


int main() {
    list l = new_list();
    append(l, 5);
    append(l, 7);
    delete_list(l);
}


list new_list() {
    element **first = malloc(sizeof(element*));
    element **last = malloc(sizeof(element*));
    int *n = malloc(sizeof(int));
    *first = NULL;
    *last = NULL;

    list list = {first, last, n};
    return list;
}


void delete_list(list l) {
    wipe(l);
    free(l.first);
    free(l.last);
    free(l.e_count);
}


element *alloc_element(int n) {
    element *e = malloc(sizeof(element));
    if (!e) {
        return NULL;
    }
    e->next = NULL;
    e->prev = NULL;
    e->num = n;
    return e;
}


int append(list l, int n) {
    element **p = l.first;
    element *e = alloc_element(n);
    if (!e) {
        return 1;
    }

    while (*p) {
        if (!(*p)->next) {
            e->prev = *p;
        }
        p = &(*p)->next;
    }

    *p = e;
    *l.last = e;
    ++*l.e_count;
    return 0;
}


void wipe(list l) {
    element *p = *l.first;
    element *t;

    while (p) {
        t = p;
        p = p->next;
        free(t);
    }

    *l.first = NULL;
    *l.last = NULL;
    *l.e_count = 0;
}

Answer 1

Since you need the functions to modify the list you need to pass pointers to list to the functions. Also, there seems to be a lot of confusion in your code about the use of double pointers which can be avoided. Here is a tidied up version of the code which fixes all the mess:

#include <stdlib.h>
#include <stdio.h>


typedef struct node_ {
    int num;
    struct node_ *next;
    struct node_ *prev;
} element;


typedef struct list_ {
    element *first;
    element *last;
    int e_count;
} list;


list *new_list(void);
void delete_list(list *l);

element *alloc_element(int n);

int append(list *l, int n);
void wipe(list *l);


int main(void) {
    list *l = new_list();
    append(l, 5);
    append(l, 7);
    delete_list(l);
}


list *new_list(void) {
    list *l = malloc(sizeof(list));
    if (!l) {
        return NULL;
    }
    l->first = NULL;
    l->last = NULL;
    l->e_count = 0;
    return l;
}


void delete_list(list *l) {
    if (l != NULL) {
        wipe(l);
        free(l);
    }
}


element *alloc_element(int n) {
    element *e = malloc(sizeof(element));
    if (!e) {
        return NULL;
    }
    e->next = NULL;
    e->prev = NULL;
    e->num = n;
    return e;
}


int append(list *l, int n) {
    element *e = alloc_element(n);
    if (!e) {
        return 1;
    }
    if (l->last) {
        l->last->next = e;
    } else {
        l->first = e;
    }
    e->prev = l->last;
    l->last = e;
    ++l->e_count;
    return 0;
}


void wipe(list *l) {
    element *p = l->first;
    element *t;

    while (p) {
        t = p;
        p = p->next;
        free(t);
    }

    l->first = NULL;
    l->last = NULL;
    l->e_count = 0;
}