twistedhat twistedhat - 1 month ago 6
C Question

How to free memory to a pointer in c

I have this (bad) code

void function(deq** dq, int data)
{
// TODO: add a new element at the end of the queue
deq *temp = (dequeue*)malloc(sizeof(dequeue));
deq *copy = (*dq);
temp->data = data;


if (copy == NULL) {
temp->next = NULL;
temp->prev = NULL;
(*dq) = temp;
}
else{
while (copy->next != NULL) {
copy = copy->next;
}

temp->prev = copy;
temp->next = NULL;
copy->next = temp;
(*dq) = temp;
}

//free(temp);
}


what is my problem is that i can not free temp without crashing the programm is there a way to solve this ? and can someone tell me why whenn i use this free i can't run the program but with valgrind it works ... so funny.

==17186== HEAP SUMMARY:
==17186== in use at exit: 216 bytes in 9 blocks
==17186== total heap usage: 15 allocs, 6 frees, 360 bytes allocated
==17186==
==17186== 72 (24 direct, 48 indirect) bytes in 1 blocks are definitely lost in loss record 8 of 9
==17186== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==17186== by 0x400670: dequeue_push_front (dequeue.c:12)
==17186== by 0x400A1D: main (main.c:21)
==17186==
==17186== 144 (24 direct, 120 indirect) bytes in 1 blocks are definitely lost in loss record 9 of 9
==17186== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==17186== by 0x400670: dequeue_push_front (dequeue.c:12)
==17186== by 0x400BA3: main (main.c:48)
==17186==
==17186== LEAK SUMMARY:
==17186== definitely lost: 48 bytes in 2 blocks
==17186== indirectly lost: 168 bytes in 7 blocks
==17186== possibly lost: 0 bytes in 0 blocks
==17186== still reachable: 0 bytes in 0 blocks
==17186== suppressed: 0 bytes in 0 blocks
==17186==
==17186== For counts of detected and suppressed errors, rerun with: -v
==17186== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)

Answer

You don't want to free(temp) in this function.

This function is allocating temp on the heap using malloc, then the node is placed in a list. If you free() the memory you just allocated here, you're left with a node in the list that points to invalid memory. Referencing that node leads to undefined behavior, which in this case manifests in a core dump.

The proper time to call free() is when a node is removed from the list.

As for the memory leak, that's happening in the else block.

You place temp at the end of the list, but then you overwrite the head of the list with temp, so your only reference to the rest of the list is in the prev pointer of temp. However, since that is now your new head, you probably don't bother to look at prev when cleaning up. So you have a memory leak.

To fix the leak, get rid of the assignment to *dq:

...
else {
    while (copy->next != NULL) {
      copy = copy->next;
    }

    temp->prev = copy;
    temp->next = NULL;
    copy->next = temp;
}
...