Ubuntu - Proper memory free for pointers inside struct C

fdv
October 30, 2023
87 views
2 votes
3 Answers

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 %dn", 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.

Tags: arrays c#struct

Answers

Your memory leak is here:

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; // overwrite of my_list.pairs[0].element1 --> memory leak
my_list.pairs[0].element2 = &element2; // overwrite of my_list.pairs[0].element2 --> memory leak

Solution:

Option 1

Delete the two lines doing malloc.

struct element element1 = {.i = 1};
struct element element2 = {.i = 2};
my_list.pairs[0].element1 = &element1;
my_list.pairs[0].element2 = &element2;

Option 2

Delete the lines creating and using local variables.

my_list.pairs[0].element1 = malloc(sizeof(struct element));
my_list.pairs[0].element2 = malloc(sizeof(struct element));
my_list.pairs[0].element1->i = 1;
my_list.pairs[0].element2->i = 2;

....
....

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

note: In C you don’t need to cast the value returned by malloc

- HS
- October 30, 2023 at 1:11 pm
- 0 votes
0
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 loose 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 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 %dn", 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 returned value malloc().
Login or Signup to reply.

- VladfromMoscow
- October 30, 2023 at 1:13 pm
- 0 votes
0
These statements
```
  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;
```
produce memory leaks.

At first memory was allocated and its addresses were assigned to pointers my_list.pairs[0].element1 and my_list.pairs[0].element2 and then the pointers were reassigned with addresses of local variable element1 and element2
```
  // set element1 and element2 of pairs[0]
  struct element element1 = {.i = 1};
  struct element element2 = {.i = 2};
```
So the addreses of the allocated memory were lost.

Instead of using this assignment
```
  my_list.pairs[0].element1 = &element1;
  my_list.pairs[0].element2 = &element2;
```
you should write
```
  *my_list.pairs[0].element1 = element1;
  *my_list.pairs[0].element2 = element2;
```
to copy values of element1 and element2 in the allocated memory.

To free correctly the allocated memory you need to free it in the reverse order. That is
```
free( my_list.pairs[0].element1 );
free( my_list.pairs[0].element2 );
free( my_list.pairs );
```
As for your code then this statement
```
  // free memory
  free(my_list.pairs);
```
frees only the first allocated extent of memory while you allocated three extents of memory
```
  my_list.pairs = (struct pair*)malloc(sizeof(struct pair));
  //...
  my_list.pairs[0].element1 = (struct element*)malloc(sizeof(struct element));
  my_list.pairs[0].element2 = (struct element*)malloc(sizeof(struct element));
```
Login or Signup to reply.

Please signup or login to give your own answer.

Click here to cancel reply.

Ubuntu – Proper memory free for pointers inside struct C

Answers