Free a doubly linked list in C

I have a doubly linked list in C and I'm confused as to how I should go about freeing it. I understand that I have to traverse the list freeing each node. Where the confusion lies is that each of my nodes has a pointer to some other data and I'm unsure how I should free that.

My doubly linked list looks like this:

typedef struct Node_ Node;
typedef struct List_ List;

struct Node_ {
    void *data;
    Node *next;
    Node *prev;
};

struct List_ {
    Node *firstNode;
    Node *lastNode;
};

To free the list I've created a function called List_free() which traverses the list freeing each node with Node_free(). These functions look like this:

void *List_free(List *list)
{
    Node *next = list-&g开发者_运维问答t;firstNode;

    while(next)
    {
        Node *node = next;
        next = node->next;
        Node_free(node);
    }

    free(list);
}

void Node_free(Node *node)
{
    free(node->data);
    free(node);
}

Where this is going to fall down is where node->data is pointer to another struct that itself contains pointers. In my case I use the same list code to store two different structs.

The way I see it I have the following options:

1. Create lists where nodes hold specific data. Not very reusable.
2. Find another way to keep track of the pointers within the node data.

Am I thinking along the right lines or have I missed something obvious? This is my first attempt at C so I wouldn't be surprised if this is all completely wrong.

One solution is to supply a function pointer responsible for freeing the node properly.

typedef void(*NodeDataFreeFn)(void*);

List_free is modified like this:

void *List_free(List *list, NodeDataFreeFn data_free)
{
    Node *next = list->firstNode;

    while(next)
    {
        Node *node = next;
        next = node->next;
        (*data_free)(node->data);
        free(node);
    }
    free(list);
}

Example data_free:

void data_free_fn(void* data_ptr) {
    // Add your custom stuff here.
    free(data_ptr);
}

Example call to List_free:

List_free(my_list, data_free_fn);

If you don't want to pass the data free function pointer by argument, you could store it into the List structure like this instead:

struct List_ {
   Node *firstNode;
   Node *lastNode;
   NodeDataFreeFn data_free;
};

Disclaimer: I didn't test this code, it may be buggy...

You're not wrong, that's one of the problems 'solved' by OOP and in particular C++.

You could add a pointer to function member in your struct that would be used to free its data. Basically adding a C++ destructor by hand. That would keep genericity without too much duplication.

If the application can store arbitrary data in your list, then is is also the responsibility of the application to arrange for proper cleanup.
This can be done in two ways:

1. Before calling List_free, the application must clear-out all elements of the list.

2. You add another parameter to List_free (and Node_free), which is a pointer to a deleter function:

   
   typedef void (deleter_t*)(void*);
   
   
   void Node_free(Node* node, deleter_t deleter)
   {
       if (deleter) (*deleter)(node->data);
       free(node);
   }
   

   For data that does not have to be freed explicitly, the application can pass a NULL deleter, for data that can be freed with a single free call, that function can be passed as deleter. For more complex data, the application has to pass a function that does the right thing.

Your problem is not in the doubly linked list as the question indicates, but rather in composite dynamic structures. That's how manual memory management works: you have to keep track of what you have allocated. It's best to try to stick to node->data being the same structure every time so that a predefined routine can free it. Alternatively, use a marker on the structure and switch on the marker at runtime to choose between the deallocation routines for different structures (or just use a function pointer to simulate a C++ destructor).

Hmmm: create a Data_free function and call it for your data pointers.

void Data_free(void *ptr)
{
    /* first identify what data type ptr really is */
    /* and then deallocate memory used by ptr */
}

void Node_free(Node *node)
{
    /*free(node->data);*/
    Data_free(node->data);
    free(node);
}

Edit: change struct Node_ definition, to make it more similar to a C++ class :-)

struct Node_ {
    void *data;
    void (*freedata)(void*);
    struct Node_ *next;
    struct Node_ *prev;
}

One approach you may want to try is to implement a memory pool; which can be especially trivial to implement in case you don't plan to free nodes individually.

Simply malloc (or mmap, perhaps to /dev/zero) a large chunk and then you can "allocate" memory for one node by a simple pointer addition. De-allocating the entire list is then a matter of freeing (or munmaping) the large chunk.

If done correctly, this may also give a slight performance boost.

As others have said, you've identified something that is a pain to do in C manually, yet must be done. You can use function pointers to make the freeing mechanism generic to what your data is, but you still have to implement the mechanism to go down the hierarchy of data in each node to free everything up.

This is a good exercise and worth doing just for the learning experience.

But there's also a very useful library that will deal with all of this for you: talloc. It keeps track of the hierarchy for you, so freeing the top level automatically frees up everything underneath. It is well documented. It was developed for samba and is still used there today, so is very well maintained.

The issue isn't that the list has prev pointers. The issue is that every different kind of Node has a different way of deleting its contents.

So there needs to be a generic Node_free routine (which you have) and it needs to dispatch on the type of the node. So either the node needs to contain a generic type code, or it needs to contain a pointer to a destructor routine tailored for that node type.

Either way, you're basically doing what C++ does for you.