Home > Software design >  Linked list in C causing a memory leak upon creation and insertion
Linked list in C causing a memory leak upon creation and insertion

Time:05-03

I have a linked list, it works fine but I was checking for memory leaks and realised that creating the list and creating a node for the list is causing memory leaks even though I go and free them later. It's saying there is lossed memory due the malloc I create in both create list and insert. I can't go and free the mallocs because I need them, is this just a necessary evil or is there a way to fix it? Is there anyway to stop these memory leaks?

#include <stdio.h>
#include <stdlib.h>
#include "linkedList.h"
#include "macros.h"

LinkedList* createLinkedList()
{
    LinkedList* list;

    list = malloc(sizeof(*list) * 1);
    list->head = NULL;
    list->tail = NULL;
    list->length = 0;

    return list;
}

void insertLast(LinkedList* list, int entry)
{
    Node* newNode = malloc(sizeof(*newNode) * 1);
    
    newNode->data = entry;
    newNode->next = NULL;

    if (list->tail == NULL)
    {
        list->head = newNode;
    }
    else
    {
        list->tail->next = newNode;
    }
    
    list -> tail = newNode;
    list->tail-> next = NULL;
    list->length  ;
}

EDIT: How I'm freeing the nodes

void freeLinkedList(LinkedList* list)
{
    int i = 0;
    Node* current, *temp;
    current = list->head;

    while(i < list->length -1)
    {
        temp = current->next;
        free(current);
        current = temp;
        i  ;
    }
}

CodePudding user response:

In freeLinkedList your loop is not making sufficient iterations. For instance, when list->length is 1, there should be one call to free, but this does not happen as the loop makes no iterations in that case.

Don't make it more difficult than necessary: skip the use of i and just use the more natural condition. Also free list itself:

void freeLinkedList(LinkedList* list)
{
    Node* temp;
    Node* current = list->head;

    while(current != NULL)
    {
        temp = current->next;
        free(current);
        current = temp;
    }
    free(list);
}

CodePudding user response:

If the list contains length nodes then the loop in freeLinkedList should look at least like

while(i < list->length)

Also you need to free the pointer to the allocated LinkedList itself after calling the function.

Pay attention to that this statement in the function insertLast

list->tail-> next = NULL;

is redundant and may be removed.

CodePudding user response:

You need to free list as well:

void freeLinkedList(LinkedList* list)
{
    int i = 0;
    Node* current, *temp;
    current = list->head;

    while(i < list->length -1)
    {
        temp = current->next;
        free(current);
        current = temp;
        i  ;
    }
    free(list);
}
  • Related