Search code examples
cmemorydoubly-linked-list

New Object in C gets initialized to existing object


I'm trying to implement a circular doubly linked list in C. Inserting one element was fine, but when inserting the second element, curr had the same address as prev.

struct ListNode {
    void *item;
    struct ListNode *next;
    struct ListNode *prev;
};

struct List {
    int num_members;
    ListNode head;

    /* function pointers */
    ...
};

int  ListAppend(List* list, void* item){
    ListNode* node = &(list->head);
    ListInsertBefore(list,item,node);
    
    return 0;
}

int  ListInsertBefore(List* list, void* item, ListNode* node){
    ListNode* prev = node->prev;
    ListNode curr;
    curr.item = item;
    
    node->prev = &curr;
    curr.next = node;
    prev->next = &curr;
    curr.prev = prev;

    list->num_members += 1;

    return 0;
}

Output of GDB:

62      curr.item = item;
7: node = (ListNode *) 0xbfffef08
8: *node = {item = 0x0, next = 0xbfffee70, prev = 0xbfffee70}
9: &curr = (ListNode *) 0xbfffee70
10: curr = {item = 0x0, next = 0xbfffef08, prev = 0xbfffef08}
11: prev = (ListNode *) 0xbfffee70
12: *prev = {item = 0x0, next = 0xbfffef08, prev = 0xbfffef08}

Solution

  • ListNode curr in ListInsertBefore is a local variable that will get destroyed at the end of the function scope. Taking, storing and dereferencing its address outside the function scope make the program have undefined behavior. You need to allocate memory dynamically with malloc, example:

    ListNode *ListInsertBefore(List *list, void* item, ListNode *node) {
        ListNode* curr = malloc(sizeof *curr); // allocate memory dynamically
        if(curr) {
            *curr = (ListNode) {item, node, node->prev};
    
            node->prev->next = curr;
            node->prev = curr;
    
            list->num_members++;
        }
        return curr;
    }
    

    Note: I made it return a pointer to the newly created ListNode instead of always returning 0. You can now use that return value to check if insertion actually succeeded.

    Also note: Memory allocated dynamically also needs to be released using free:

    List *ListCreate() {
        List *list = malloc(sizeof *list);
        if(list) *list = (List) {0, {NULL, &list->head, &list->head}};
        return list;
    }
    
    void ListFree(List *list) {
        for(ListNode *next, *curr = list->head.next; curr != &list->head; curr = next) {
            next = curr->next;
            free(curr);
        }
        free(list);
    }
    

    Demo