Search code examples
cmemory-leakslinked-listvalgrindsingly-linked-list

Leak_DefinitelyLost warning when creating new node in linked list (C)


I have a program in which I use a linked list, and I have a function called NewNode that is called via main or other functions (even recursive functions).

The program is very slow so I checked it with Valgrind and it seems like everything is working fine apart from this function, as I get Leak_DefinitelyLost error on the malloc, leaking like 1000 bytes (so it must be solved). I suppose I need to free in some way the nodes but I do not really understand how.

Here's how the node is defined and how the function is defined:

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

typedef struct node{
    char* data;
    struct node* next;
}list;

struct node* NewNode(char* to_insert) {
    struct node *new = malloc(sizeof(*new));
    new->data = malloc(strlen(to_insert) + 1);
    strcpy(new->data, to_insert);
    new->next = NULL;
    return new;
}

EDIT: I have two functions to free, one to free all the list, one to delete only a specific node from the list, here they are:

void Free_All(struct node* head){
    if (head == NULL)
        return;
    Free_All(head->next);
    free(head->data);
    free(head);
}

struct node* Delete_node(struct node* head, char* to_delete){
    struct node *p, *dummy;
    p = head;

    if (p != NULL && strcmp(p->data,to_delete) == 0) {
        head = p->next;
        return head;
    }
    while (p)
    {
        dummy = p->next;
        if (dummy != NULL){
            if (strcmp(dummy->data, to_delete) == 0){
                dummy = p->next;
                p->next = dummy->next;
                //free(dummy);
                break;
            }
        }
        if (p->next == NULL){
            break;
        }
        p = p->next;
    }
    return head;
}

Note: if I try to uncomment the free I get a SIGSEGV error and the program breaks.


Solution

  • At least the shown function Delete_node produces a memory leak because the deleted node is not freed with the stored string.

    struct node* Delete_node(struct node* head, char* to_delete){
        struct node *p, *dummy;
        p = head;
    
        if (p != NULL && p->data == to_delete) {
            head = p->next;
            return head;
        }
        //...
    

    Pay attention to that it seems instead of this comparison

    p->data == to_delete
    

    you have to use standard function strcmp

    if ( p != NULL && strcmp( p->data, to_delete ) == 0 ) {
    

    Using your approach the function can look the following way

    struct node* Delete_node(struct node* head, const char* to_delete)
    {
        if ( head != NULL )
        {
            if ( strcmp( head->data, to_delete ) == 0 ) 
            {
                struct node *tmp = head;
                head = tmp->next;
                free( tmp->data );
                free( tmp );
            }
            else
            {
                struct node *p = head;
                while ( p->next != NULL && strcmp( p->next->data, to_delete ) != 0 )
                {
                    p = p->next;
                }
    
                if ( p->next != NULL )
                {
                    struct node *tmp = p->next;
                    p->next = tmp->next;
                    free( tmp->data );
                    free( tmp );
                }
            }
        }
    
        return head;
    }