Search code examples
c++stlstdidiomsdynamic-memory-allocation

STL erase-remove idiom vs custom delete operation and valgrind


This is an attempt to rewrite some old homework using STL algorithms instead of hand-written loops and whatnot.

I have a class called Database which holds a Vector<Media *>, where Media * can be (among other things) a CD, or a Book. Database is the only class that handles dynamic memory, and when the program starts it reads a file formatted as seen below (somewhat simplified), allocating space as it reads the entries, adding them to the vector (v_) above.

CD
Artist
Album
Idnumber
Book
Author
Title
Idnumber
Book
...
...

Deleting these object works as expected when using a hand-written loop: EDIT: Sorry I spoke too soon, it's not actually a 'hand-written' loop per-se.. I've been editing the project to remove hand-written loops and this actually uses the find_if algorithm and a manual deletion, but the question is till valid. /EDIT.

typedef vector<Media *>::iterator v_iter;

...
void Database::removeById(int id) {
    v_iter it = find_if(v_.begin(), v_.end(), Comparer(id));
    if (it != v_.end()) {
        delete *it;
        v_.erase(it);
    }
}

That should be pretty self-explanatory - the functor returns true if it find an id matching the parameter, and the object is destroyed. This works and valgrind reports no memory leaks, but since I'd like to use the STL, the obvious solution should be to use the erase-remove idiom, resulting in something like below

void Database::removeById(int id) {
    v_.erase(remove_if(v_.begin(), v_.end(), Comparer(id)), v_.end());
};

This, however, 'works' but causes a memory leak according to valgrind, so what gives? The first version works fine with no leaks at all - while this one always show 3 allocs 'not freed' for every Media object I delete.


Solution

  • This is why you should always, ALWAYS use smart pointers. The reason that you have a problem is because you used a dumb pointer, removed it from the vector, but that didn't trigger freeing the memory it pointed to. Instead, you should use a smart pointer that will always free the pointed-to memory, where removal from the vector is equal to freeing the pointed-to memory.