I have a problem with my free.
When I free only l in main.c, without limit function use, its ok.
BUT if I put limit function use and I free l_limit, there is the problem : free(): double free detected in tcache 2 and valgrind is not happy.
Can you help me to fix the free errors ? :)
Minimal reproducible example :
#include <stdlib.h>
#include <stdio.h>
typedef void* gpointer;
struct cell_s {
gpointer ptr_value;
struct cell_s *next;
};
typedef struct cell_s cell_t;
typedef cell_t* adr; // address
struct list_s {
cell_t *head;
int size;
};
typedef struct list_s list_t;
typedef void (*list_gfree)(gpointer data);
typedef void (*list_gprint)(gpointer data);
cell_t* create_cell(gpointer v) {
cell_t *c = malloc(sizeof(cell_t));
c->next = NULL;
c->ptr_value = v;
return c;
}
void destroy_int(gpointer data) {
free(data);
}
void print_int(gpointer data) {
int *ptr_value = (int *)data;
printf("%d - ", *ptr_value);
}
list_t* list_create() {
list_t *l = malloc(sizeof(list_t));
l->head = NULL;
l->size = 0;
return l;
}
void list_insert_in_head(list_t *l, gpointer element) {
adr address_c = create_cell(element);
address_c->next = l->head;
l->head = address_c;
l->size;
}
void list_insert_next(list_t *l, gpointer element, adr address) {
adr address_c = create_cell(element);
if (l->head == NULL) {
list_insert_in_head(l, element);
} else {
address_c->next = address->next;
address->next = address_c;
}
l->size;
}
void list_remove_in_head(list_t *l, list_gfree ft_destroy) {
if (l->head != NULL) {
adr tmp = l->head->next;
ft_destroy(l->head->ptr_value);
l->head->ptr_value = NULL;
ft_destroy(l->head);
l->head= tmp;
--l->size;
}
}
void list_remove_after(list_t *l, adr address, list_gfree ft_destroy) {
if (l->head->next == NULL) {
printf("Use list_remove_in_head function\n");
} else if (address != NULL) {
adr tmp = address->next->next;
ft_destroy(address->next->ptr_value);
address->next->ptr_value = NULL;
ft_destroy(address->next);
address->next = tmp;
--l->size;
}
}
void list_destroy(list_t *l, list_gfree ft_destroy) {
adr current = l->head;
while(current != NULL) {
adr tmp = current;
current = current->next;
ft_destroy(tmp->ptr_value);
tmp->ptr_value = NULL;
ft_destroy(tmp);
}
free(l);
}
void list_print(list_t *l, list_gprint ft_print) {
adr current = l->head;
while (current != NULL) {
ft_print(current->ptr_value);
current = current->next;
}
printf("\n");
}
list_t* limit(list_t *l, int n) {
list_t *l_limit = list_create();
adr current = l->head;
list_insert_in_head(l_limit, current->ptr_value);
current = current->next;
adr current_addr_l_limit = l_limit->head;
int count = 1;
if (n < l->size) {
while (count < n && current != NULL) {
count;
list_insert_next(l_limit, current->ptr_value, current_addr_l_limit);
current = current->next;
current_addr_l_limit = current_addr_l_limit->next;
}
} else {
while (current != NULL) {
list_insert_next(l_limit, current->ptr_value, current_addr_l_limit);
current = current->next;
current_addr_l_limit = current_addr_l_limit->next;
}
}
return l_limit;
}
int main(void) {
list_t *l = list_create();
int *ptr_int = (int *)malloc(sizeof(int));
*ptr_int = 4;
list_insert_in_head(l, ptr_int);
list_print(l, print_int);
printf("Size : %d\n", l->size);
int *ptr_int_2 = (int *)malloc(sizeof(int));
*ptr_int_2 = 7;
list_insert_in_head(l, ptr_int_2);
list_print(l, print_int);
printf("Size : %d\n", l->size);
int *ptr_int_3 = (int *)malloc(sizeof(int));
*ptr_int_3 = 100;
list_insert_next(l, ptr_int_3, l->head);
list_print(l, print_int);
printf("Size : %d\n", l->size);
list_t *l_limit = limit(l, 2);
printf("\nLIMIT 2 \n");
list_print(l_limit, print_int);
printf("\n");
list_remove_in_head(l, destroy_int);
list_print(l, print_int);
printf("Size : %d\n", l->size);
list_remove_after(l, l->head, destroy_int);
list_print(l, print_int);
printf("Size : %d\n", l->size);
list_remove_after(l, l->head, destroy_int);
list_print(l, print_int);
printf("Size : %d\n", l->size);
int *ptr_int_4 = (int *)malloc(sizeof(int));
*ptr_int_4 = 447;
list_insert_next(l, ptr_int_4, l->head);
list_print(l, print_int);
printf("Size : %d\n", l->size);
list_destroy(l_limit, destroy_int);
list_destroy(l, destroy_int);
}
Output :
4 -
Size : 1
7 - 4 -
Size : 2
7 - 100 - 4 -
Size : 3
LIMIT 2
7 - 100 -
100 - 4 -
Size : 2
100 -
Size : 1
Use list_remove_in_head function.
100 -
Size : 1
100 - 447 -
Size : 2
free(): double free detected in tcache 2
Execution : (-g -fsanitize=address)
=================================================================
==16065==ERROR: AddressSanitizer: attempting double-free on 0x602000000070 in thread T0:
#0 0x7f8b09173517 in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:127
#1 0x55ad7141f365 in destroy_int /home/zzz/zzz/main2.c:34
#2 0x55ad7141fa5b in list_destroy /home/zzz/zzz/main2.c:112
#3 0x55ad714203a9 in main /home/zzz/zzz/main2.c:211
#4 0x7f8b08ec4fcf in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
#5 0x7f8b08ec507c in __libc_start_main_impl ../csu/libc-start.c:409
#6 0x55ad7141f204 in _start (/home/zzz/zzz/main 0x1204)
0x602000000070 is located 0 bytes inside of 4-byte region [0x602000000070,0x602000000074)
freed by thread T0 here:
#0 0x7f8b09173517 in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:127
#1 0x55ad7141f365 in destroy_int /home/zzz/zzz/main2.c:34
#2 0x55ad7141f6ea in list_remove_in_head /home/antoine/progc/main2.c:77
#3 0x55ad714200f5 in main /home/zzz/zzz/main2.c:193
#4 0x7f8b08ec4fcf in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
previously allocated by thread T0 here:
#0 0x7f8b09173867 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
#1 0x55ad7141feed in main /home/zzz/zzz/main2.c:176
#2 0x7f8b08ec4fcf in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
SUMMARY: AddressSanitizer: double-free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:127 in __interceptor_free
==16065==ABORTING
Valgrind
==16161== Invalid free() / delete / delete[] / realloc()
==16161== at 0x484621F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==16161== by 0x10921F: destroy_int (in /home/zzz/zzz/main2)
==16161== by 0x1094C2: list_destroy (in /home/zzz/zzz/main2)
==16161== by 0x109918: main (in /home/zzz/zzz/main2)
==16161== Address 0x4a97570 is 0 bytes inside a block of size 4 free'd
==16161== at 0x484621F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==16161== by 0x10921F: destroy_int (in /home/zzz/zzz/main2)
==16161== by 0x10939C: list_remove_in_head (in /home/antoine/progc/main2)
==16161== by 0x1097CA: main (in /home/zzz/zzz/main2)
==16161== Block was alloc'd at
==16161== at 0x4843839: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==16161== by 0x1096B7: main (in /home/zzz/zzz/main2)
==16161==
==16161== Invalid free() / delete / delete[] / realloc()
==16161== at 0x484621F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==16161== by 0x10921F: destroy_int (in /home/zzz/zzz/main2)
==16161== by 0x1094C2: list_destroy (in /home/zzz/zzz/main2)
==16161== by 0x10992E: main (in /home/zzz/zzz/main2)
==16161== Address 0x4a97610 is 0 bytes inside a block of size 4 free'd
==16161== at 0x484621F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==16161== by 0x10921F: destroy_int (in /home/zzz/zzz/main2)
==16161== by 0x1094C2: list_destroy (in /home/zzz/zzz/main2)
==16161== by 0x109918: main (in /home/zzz/zzz/main2)
==16161== Block was alloc'd at
==16161== at 0x4843839: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==16161== by 0x109715: main (in /home/zzz/zzz/main2)
==16161==
==16161==
==16161== HEAP SUMMARY:
==16161== in use at exit: 0 bytes in 0 blocks
==16161== total heap usage: 13 allocs, 15 frees, 1,168 bytes allocated
==16161==
==16161== All heap blocks were freed -- no leaks are possible
==16161==
==16161== For lists of detected and suppressed errors, rerun with: -s
==16161== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)
CodePudding user response：
Your limit fuction is :
list_t* limit(list_t *l, int n) {
list_t *l_limit = list_create();
adr current = l->head;
list_insert_in_head(l_limit, current->ptr_value);//Creates a new cell but uses the same ptr_value!
current = current->next;
adr current_addr_l_limit = l_limit->head;
int count = 1;
if (n < l->size) {
while (count < n && current != NULL) {
count;
list_insert_next(l_limit, current->ptr_value, current_addr_l_limit); //reuses the same ptr_value!
current = current->next;
current_addr_l_limit = current_addr_l_limit->next;
}
} else {
while (current != NULL) {
list_insert_next(l_limit, current->ptr_value, current_addr_l_limit);
current = current->next;
current_addr_l_limit = current_addr_l_limit->next;
}
}
return l_limit;
}
When inserting elements in l_limits from the source list, you do create new cells, but you don't create new elements!
So the cells in the original list use the same ptr_value as the cells in the new list!
Thus when you destroy the second list, you attempt to free the same ptr_values:
void list_destroy(list_t *l, list_gfree ft_destroy) {
adr current = l->head;
while(current != NULL) {
adr tmp = current;
current = current->next;
ft_destroy(tmp->ptr_value);//When this is called for the second list, you access the same pointer as for the first list!
tmp->ptr_value = NULL;
ft_destroy(tmp);
}
free(l);
}
To solve this issue, you could make actual copies of the object stored at "ptr_values" in limit, but that requires you to know the type of the value stored in ptr_value:
list_t* limit(list_t *l, int n) {
list_t *l_limit = list_create();
adr current = l->head;
gpointer buff = malloc(sizeof(int));//replace int by the correct type
if (buff == NULL)
exit(-1);
*((int*)buff) = *((int*)current->ptr_value);
list_insert_in_head(l_limit, buff);
current = current->next;
adr current_addr_l_limit = l_limit->head;
int count = 1;
if (n < l->size) {
while (count < n && current != NULL) {
count;
gpointer buff = malloc(sizeof(int));//replace int by the correct type
if (buff == NULL)
exit(-1);
*buff = *current->ptr_value;
list_insert_next(l_limit, buff, current_addr_l_limit);
current = current->next;
current_addr_l_limit = current_addr_l_limit->next;
}
} else {
while (current != NULL) {
list_insert_next(l_limit, current->ptr_value, current_addr_l_limit);
current = current->next;
current_addr_l_limit = current_addr_l_limit->next;
}
}
return l_limit;
}
Alternatively you could make a special list destructor that doesn't free the content of the cells, but that feels a bit weird.
CodePudding user response：
You allocate memory for the data elements using malloc in main and store the pointer in your list.
Later you call free for the data inside list management functions like list_destroy and list_remove_in_head.
Function limit creates a new list and stores pointers to the data from the original list in the new list. Then you will have duplicate (or multiple) pointers to data elements in different lists (in this case l and l_limit), and when the list management functions free the memory of the data elements this will result in double (or multiple) free.
The concept to allocate the memory in main, or more general in the caller of your list management functions and to free it inside your functions does not work when your functions can create duplicate or multiple pointers to the same memory object.