CODEWITHSUNDEEP
c++linked-listvalgrind
user3265963
user3265963

Reputation: 273

Valgrind error linked list

I'm working on a homework and all test on the output are working fine. But I am always getting this Valgrind error:

==22990== Memcheck, a memory error detector
==22990== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==22990== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==22990== Command: ./a.out
==22990== Parent PID: 22989
==22990== 
==22990== Invalid read of size 8
==22990==    **at 0x401DFB: IntList::removeAll() (IntList.cpp:113**)
==22990==    by 0x40113F: main (main.cpp:120)
==22990==  Address 0x4c50248 is 8 bytes inside a block of size 16 free'd
==22990==    at 0x4A05FD6: operator delete(void*) (vg_replace_malloc.c:480)
==22990==    **by 0x401DF6: IntList::removeAll() (IntList.cpp:117)**
==22990==    by 0x40113F: main (main.cpp:120)
==22990== 
==22990== 
==22990== HEAP SUMMARY:
==22990==     in use at exit: 0 bytes in 0 blocks
==22990==   total heap usage: 2,009 allocs, 2,009 frees, 40,128 bytes allocated
==22990== 
==22990== All heap blocks were freed -- no leaks are possible
==22990== 
==22990== For counts of detected and suppressed errors, rerun with: -v
==22990== ERROR SUMMARY: 6 errors from 1 contexts (suppressed: 6 from 6)

So it's telling me the that my removeall() function is leaking memory right? The error message talks of line 113 and 117. 113 is the line where while loop starts and 117 is delete temp.

removeall() function:

void IntList::removeAll()
{
    NodePtr temp;
    temp = head;
    if(head == NULL){
        cout << "The list is empty" << endl;
        return;
    }

    while (temp-> link != NULL)
    {
            temp = head;
        head = temp->link;
        delete temp;
    }

}

Can somoene help me out with this? Am I understanding the valgrind error correctly? How can I fix the memory leak?

Sorry if this has been asked before. I tried the search function with no luck.

Upvotes: 0

Views: 685

Answers (2)

Mike Seymour
Mike Seymour

Reputation: 254631

It's easy to see that you're dereferencing temp in the loop condition after you deleted it at the end of the previous iteration. You're also stopping before you delete the last element. You want something more like

while (NodePtr temp = head) {
    head = temp->link;
    delete temp;
}

This also behaves correctly if the list is empty, so there's no need to check that beforehand unless you particularly want to print something in that case.

Upvotes: 2

Luchian Grigore
Luchian Grigore

Reputation: 258618

If temp->link is NULL, you never delete temp, thus always having at least one node left in your list.

I think you're missing a delete head; at the end of the function.

Upvotes: 0

Related Questions