What is the cause of the memory leak in c?-CodePudding

I was creating a linked list in c and I came across this memory leak. Valgrind is telling that the source of the problem is on line 15 but I still can't see the problem. I feel very stupid not knowing where the problem occurred but it would be nice if you could help me! thanks in advance!

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

typedef struct node
{
    int number;
    struct node *next;
}
node;


int main(void)
{
    node *list = malloc(sizeof(node));
    if (list == NULL)
    {
        return 1;
    }
    list = NULL;
    // create link list
    for (int i = 0; i < 3; i  )
    {
        node *n = malloc(sizeof(node));
        if(n == NULL)
        {
            free(list);
            return 1;
        }
        n->number = i 1;
        n->next = NULL;

        n->next = list;
        list = n;
    }
    //print link list
    for (node *tmp = list; tmp != NULL; tmp = tmp->next)
    {
        printf("%d - ", tmp->number);
    }
    printf("\n");
    //free link list
    while (list != NULL)
    {
        node *tmp = list->next;
        free(list);
        list = tmp;
    }


}

this is my valgrind output:

==3895== Memcheck, a memory error detector
==3895== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==3895== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==3895== Command: ./temp
==3895== 
3 - 2 - 1 - 
==3895== 
==3895== HEAP SUMMARY:
==3895==     in use at exit: 16 bytes in 1 blocks
==3895==   total heap usage: 4 allocs, 3 frees, 64 bytes allocated
==3895== 
==3895== 16 bytes in 1 blocks are definitely lost in loss record 1 of 1
==3895==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==3895==    by 0x401168: main (temp.c:15)
==3895== 
==3895== LEAK SUMMARY:
==3895==    definitely lost: 16 bytes in 1 blocks
==3895==    indirectly lost: 0 bytes in 0 blocks
==3895==      possibly lost: 0 bytes in 0 blocks
==3895==    still reachable: 0 bytes in 0 blocks
==3895==         suppressed: 0 bytes in 0 blocks
==3895== 
==3895== For lists of detected and suppressed errors, rerun with: -s
==3895== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

CodePudding user response：

Pointer is just a variable that stores a memory address. You got the memory address with malloc malloc(sizeof(node)) but you put null in it list = NULL and you lost the address. Isn't this what you want?

    node** list;
    size_t i;

    list = malloc(sizeof(node*));
    *list = NULL;

    for (i = 0; i < 3; i  )
    {
        node* n = malloc(sizeof(node));

        n->number = i   1;
        n->next = *list;

        *list = n;
    }

    /* print node... */
    /* release node... free(n) */
    free(list);
    list = NULL;

CodePudding user response：

It is evident that this allocation of memory

node *list = malloc(sizeof(node));
if (list == NULL)
{
    return 1;
}
list = NULL;

is redundant and produces a memory leak because the pointer list is reassigned after the memory allocation. As a result the address of the allocated memory is lost.

Just write

node *list = NULL;

Also in this code snippet

    n->number = i 1;
    n->next = NULL;

    n->next = list;
    list = n;

this statement

    n->next = NULL;

is redundant.

In the for loop

for (int i = 0; i < 3; i  )
{
    node *n = malloc(sizeof(node));
    if(n == NULL)
    {
        free(list);
        return 1;
    }
    //...

you need to free all the allocated memory.

For example

for (int i = 0; i < 3; i  )
{
    node *n = malloc(sizeof(node));
    if(n == NULL)
    {
        while ( list )
        {
            node *tmp = list;
            list = list->next;
            free( tmp );
        } 
        return 1;
    }
    //...

To avoid code duplication you could write a separate function that frees the allocated memory. For example

void free_list( node **list )
{
    while ( *list )
    {
        node *tmp = *list;
        *list = ( *list )->next; 
        free( tmp );
    }
}

and call it like for example

for (int i = 0; i < 3; i  )
{
    node *n = malloc(sizeof(node));
    if(n == NULL)
    {
        free_list( &list );
        return 1;
    }
    //...

Or at the end of the program like

free_list( &list );