Leak in this function: How do I get rid of it?

[ciphermagi]

Okay, I'm having trouble with this function. Basically it's supposed to read in a bunch of stats and names and assign them to a list of Character objects. Now the real problem I'm having is that it always reads in the last element twice. I've kind of made a little bit of an ugly workaround for that, but with the workaround I've picked up a memory leak, and I'm not sure how to solve it.

Opening a file and reading in characters:

// Passing the list and the iterator by reference so they can be changed
void openChars(list<Character*>& characters, list<Character*>::iterator& it){
	ifstream myOpenFile;
	Character * temp;
	string name;
	int stats[9];

    myOpenFile.open("save.txt");
    it = characters.begin();
    myOpenFile.seekg (0, ios::beg);
    while (!myOpenFile.eof()){
        myOpenFile >> name;
        cout << "Calling " << name << endl;
        for (int i = 0; i < 9; ++i){
            myOpenFile >> stats[i];
        }
        cout << "Loading " << name << endl;
        temp = new Character(name, stats);
        characters.push_back(temp);
    }
    it = characters.end(); // These four lines are the workaround
    --it; // I am going to the last element
    delete *it; // Deleting the memory
    it = characters.erase(it); // And erasing the element from the list
    for (it = characters.begin(); it != characters.end(); ++it){
    cout << (*it)->getName() << endl;
    }
    myOpenFile.close();
}

Handling the memory in the list:

// This loop goes through the list and deletes everything left over just before main returns 0
for (it = characters.begin(); it != characters.end(); ++it) {
    delete *it;
    it = characters.erase(it);
}

valgrind --leak-check=full ./a.out result:

==5808== 
==5808== HEAP SUMMARY:
==5808==     in use at exit: 96 bytes in 2 blocks
==5808==   total heap usage: 11 allocs, 9 frees, 9,111 bytes allocated
==5808== 
==5808== 96 (64 direct, 32 indirect) bytes in 1 blocks are definitely lost in loss record 2 of 2
==5808==    at 0x4C28B42: operator new(unsigned long) (vg_replace_malloc.c:261)
==5808==    by 0x402D99: openChars(std::list<Character*, std::allocator<Character*> >&, std::_List_iterator<Character*>&) (in ***/a.out)
==5808==    by 0x40353A: main (in ***/a.out)
==5808== 
==5808== LEAK SUMMARY:
==5808==    definitely lost: 64 bytes in 1 blocks
==5808==    indirectly lost: 32 bytes in 1 blocks
==5808==      possibly lost: 0 bytes in 0 blocks
==5808==    still reachable: 0 bytes in 0 blocks
==5808==         suppressed: 0 bytes in 0 blocks
==5808== 
==5808== For counts of detected and suppressed errors, rerun with: -v
==5808== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 4 from 4)

[JLBorges]

> How do I get rid of it?

The painless way would be to smart pointers instead of dumb pointers.

For example, a std::list< std::shared_ptr<Character> >

http://en.wikipedia.org/wiki/Smart_pointer

[ciphermagi]

I'm trying to make this code friendly to the C++98 so that my partner can still compile it, so smart pointers aren't quite as viable as I would like. I did, however, manage to find my problem, and I feel like an idiot for not thinking of it sooner.

// This loop goes through the list and deletes everything left over just before main returns 0
for (it = characters.begin(); it != characters.end(); ++it) {
    delete *it;
    it = characters.erase(it);
}

I was incrementing the iterator and erasing, so I was skipping elements. I just changed the above to:

// This loop goes through the list and deletes everything left over just before main returns 0
for (it = characters.begin(); it != characters.end();) { // Note the missing incremental
    delete *it;
    it = characters.erase(it);
}