Using free for DLL Structure

Hi,

I'm not unfamiliar with programming, but I'm more comfortable with OOP languages like Java or C#. The last time I programmed in C was probably 2 1/2 years ago. So, I have some questions with regards to using the free function when dynamically allocating memory.

My issue is that I am getting a segmentation fault, which I'm pretty sure is because I am either using a pointer or memory incorrectly. However, I've been trying to figure out what is wrong, but I haven't found anything.

I'm compiling the program running: gcc -Wall -g -o dll dll.c, but I'm not getting any warnings or compile errors, which is leading me to believe it is within my code.

Here are the (I believe pertinent) items, which is implementing a Double Linked List.

Code:

---

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

//Double linked list structure
typedef struct Node
{
    long int x;
    char *a;
    char *b;
    char *c;
    float num;
    struct Node *next;
    struct Node *prev;
} DLLNode;

//Implementation of adding to list
void InsertDataIntoList(DLLNode *node, long int _i, char *_a, char *_b, char *_c, float _num)
{
    //Since the start of the list is passed in, iterate through the list until the end of list
    while(node->next!=NULL)    { node = node->next; }
   
    //Allocate memory for the new node
    node->next = malloc(sizeof(DLLNode));
    (node->next)->prev = node;            //Set up pointers for previous
    node = node->next;
    node->x = _i;
    node->a = _a;
    node->b = _b;
    node->c = _c;            //Put in data for the node
    node->num = _num;
    node->next = NULL;                //The next node is null until values are put into it
}

//Freeing of the nodes
void DeleteAllNodes(DLLNode *node)
{
    //Delete nodes from left to right
    DLLNode *temp = node->next;
    free(node->c);
    free(node->b);
    free(node->a);
    if (node->prev != NULL)
    {
        free(node->prev);
    }
    free(node);
    DeleteAllNodes(temp);
}

//In my main program I create a starter node, and then pass it in to functions
int main(int argc, char *argv[])
{
    DLLNode *start = malloc(sizeof(DLLNode));
    long int _i = 1234567;
    char *_a = strdup("Hello");
    char *_b = strdup("Its");
    char *_c = strdup("Me");
    float _num = 3.40;
    InsertDataIntoList(start, _i, _a, _b, _c, _num);
    DeleteAllNodes(start);
    return 0;
}

---

Any help is appreciated.

Please post code that compiles, which means including all headers.
Even adding headers, you still have an error (DLLNode does not have a member called i).
And since you're compiling with -g, presumably you've run it through a debugger. What did it say?

When you malloc memory, that memory is not initialized in any way, so you need to set up your start node.

Your insert function assumes that the head always points to a valid node, so you'll need to have one in order to start.

Sorry, that was a mistype on my part, it should have been node->x and not node->i. This is part of a programming assignment for class, so I would like to try and not post as much code as possible. If it is absolutely necessary I will.

I am running that compile command because that is what my professor specified to do so. The only error that I see specifically is:
double free or corruption (fasttop): 0x00000000020a0010 *** but I am unsure what that means exactly.

Initialize the memory in which way? So after doing a malloc, I would do something like start->prev ==NULL and start->next==NULL? Or do I need to intialize all of the allocated memory (i.e. prev, next and the character pointers)?

Quote:

---

Originally Posted by jbc223456

Sorry, that was a mistype on my part, it should have been node->x and not node->i. This is part of a programming assignment for class, so I would like to try and not post as much code as possible. I could post all of my code, but would like to try and not do so. If it is absolutely necessary I will.

Initialize the memory in which way? So after doing a malloc, I would do something like start->prev ==NULL and start->next==NULL? Or do I need to intialize all of the allocated memory (i.e. prev, next and the character pointers)?

---

You need to follow the specs (even if you get to write the specs yourself). Just based on what I can see, insert expects a dummy node at the beginning that has no real data in it, while delete ... well, delete as you have it written will never work no matter what the structure of your list (since it's recursive with no stop condition), but therefore it will chew through all the nodes and so sort-of assumes that all the nodes are "real data". In any event, you need to decide whether

1. there is always an actual node somewhere that is the "start" of the list that doesn't contain any data, but allows you to link to it right away
2. an empty list means there are no nodes anywhere, and so initially start = NULL

Both of these are pretty common approaches, so either will work, but you will have to pick one.

Another common (and probably better) alternative is to have a separate List struct:

Code:

---

typedef struct List {
    DLLNode *head;
    DLLNode *tail;
    size_t size;
} List;

---

You'd initialize it and use it like this:

Code:

---

    List list = {NULL,NULL,0};
    ListInsert(&list, ...);

---

Actually, I'd write a ListInit function instead of the above, but either will work.

I've made some changes to the code. I am not getting errors anymore (i.e. I've fixed my segmentation fault error), but using valgrind to check for memory leaks still says I have a leak somewhere, but I can't seem to find where.

Code:

---

void DeleteAllNodes(DLLNode *node)
{
    //Delete nodes from left to right
    if (node != NULL)
    {
    DLLNode *temp = node->next;
    free(node->c);
    free(node->b);
    free(node->a);
    free(node->prev);
    free(node);
    DeleteAllNodes(temp);
    }
    else
    {} //Do nothing
}

---

And I've initialized my start:

Code:

---

start->prev = NULL;
start->next = NULL;
start->x = 0;
start->a = NULL;
start->b = NULL;
start->c = NULL;
start->num = 0.0;

---

Wouldn't be a leak, just a crash, but you shouldn't free node->prev, since you already freed that node in the previous iteration of your ... loop.

Right, that makes sense now that I've looked at it again.