Search code examples
cstructmallocfree

Proper memory free for pointers inside struct C


I have a struct list which contains a dynamic array of type struct pair, and each element of this array points to two variable of type struct element. In the main of my program I allocate, via a malloc call, memory for struct pair pairs array and for each element pointer. Then I assign a value to elements, check the value and free the memory. Here is the code

#include "stdlib.h"
#include "stdio.h"

struct element{
  int i;
};

struct pair{
  struct element *element1, *element2;
};

struct list{
  struct pair *pairs;
};

int main(void) {

  // declare variable my_list
  struct list my_list;

  // set size of pairs array (one for the example)
  my_list.pairs = (struct pair*)malloc(sizeof(struct pair));

  // set element1 and element2 of pairs[0]
  struct element element1 = {.i = 1};
  struct element element2 = {.i = 2};
  my_list.pairs[0].element1 = (struct element*)malloc(sizeof(struct element));
  my_list.pairs[0].element2 = (struct element*)malloc(sizeof(struct element));
  my_list.pairs[0].element1 = &element1;
  my_list.pairs[0].element2 = &element2;

  // check element values
  printf("elements are: %d %d\n", my_list.pairs[0].element1->i, my_list.pairs[0].element2->i); 

  // free memory
  free(my_list.pairs);

  return 0;
}

I can compile and run the code without problems but if I run the code in valgrind I get the following memory leak:

*** valgrind --leak-check=full -s ./a.out 
==6676== Memcheck, a memory error detector
==6676== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==6676== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==6676== Command: ./a.out
==6676== 
elements are: 1 2
==6676== 
==6676== HEAP SUMMARY:
==6676==     in use at exit: 8 bytes in 2 blocks
==6676==   total heap usage: 4 allocs, 2 frees, 1,048 bytes allocated
==6676== 
==6676== 4 bytes in 1 blocks are definitely lost in loss record 1 of 2
==6676==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==6676==    by 0x1091EE: main (foo.c:27)
==6676== 
==6676== 4 bytes in 1 blocks are definitely lost in loss record 2 of 2
==6676==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==6676==    by 0x1091FF: main (foo.c:28)
==6676== 
==6676== LEAK SUMMARY:
==6676==    definitely lost: 8 bytes in 2 blocks
==6676==    indirectly lost: 0 bytes in 0 blocks
==6676==      possibly lost: 0 bytes in 0 blocks
==6676==    still reachable: 0 bytes in 0 blocks
==6676==         suppressed: 0 bytes in 0 blocks
==6676== 
==6676== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)

Lines 27 and 28 are responsible of the leak. If I try to add (before line 36), something like free(my_list.pairs[0].element1); I get the following error in valgrind

==6684== 1 errors in context 1 of 3:
==6684== Invalid free() / delete / delete[] / realloc()
==6684==    at 0x483CA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==6684==    by 0x10924F: main (foo.c:36)
==6684==  Address 0x1ffefff918 is on thread 1's stack
==6684==  in frame #1, created by main (foo.c:16)

So my questions are:

  • how can I avoid the memory leak and properly free the memory allocated by lines 27 and 28?
  • why trying to free the pointer my_list.pairs[0].element1 rise the error Invalid free() / delete / delete[] / realloc()?

Code has been compiler with gcc as follow: gcc -Wall -g foo.c and my version of gcc is

gcc (Ubuntu 9.4.0-1ubuntu1~20.04.2) 9.4.0
Copyright (C) 2019 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Solution

  • Here your program is leaking memory:

      my_list.pairs[0].element1 = &element1;
      my_list.pairs[0].element2 = &element2;
    

    reason is - my_list.pairs[0].element1 and my_list.pairs[0].element2 are holding the reference of memory allocated dynamically and after the above assignment, they lose those memory references. Hence, resulting in memory leak.

    Note that, in your code, element1 and element2 are not the dynamically allocated objects. They are the objects created on the stack:

    struct element element1 = {.i = 1};   
    struct element element2 = {.i = 2};
    

    It's not valid to free them:

    free(my_list.pairs[0].element1); 
    

    free can only be used for pointers allocated by malloc(), calloc(), aligned_alloc() or realloc(), otherwise it will result in undefined behavior.

    To fix this problem either don't allocate memory to my_list.pairs[0].element1 and my_list.pairs[0].element2 dynamically or allocate them dynamically and free them once you are done with them. Implementation of later suggestion:

      struct element * element1 = (struct element*)malloc(sizeof(struct element));
      struct element * element2 = (struct element*)malloc(sizeof(struct element));
      element1->i = 1;
      element2->i = 2;
      my_list.pairs[0].element1 = element1;
      my_list.pairs[0].element2 = element2;
    
      // check element values
      printf("elements are: %d %d\n", my_list.pairs[0].element1->i, my_list.pairs[0].element2->i); 
    
      // free memory
      free(my_list.pairs[0].element1);
      free(my_list.pairs[0].element2);
    

    Additional:

    Follow good programming practice, make sure to check the returned value of malloc().