Re: Is this the wrong way to use std::list?
TBass wrote:
So I have a class:
class Client
{
unsigned int ClientID;
....
};
class MyListenSocket
{
...
AddClient( Client *pClient);
RemoveClient( unsigned int ID );
std::list<Client> m_listClients;
};
To add clients to the list, AddClient is called:
MyListenSocket::AddClient( Client *pClient )
{
m_listClients.push_back( *pClient );
}
But a client can be erased from anywhere on the list. I wrote this
function:
MyListenSocket::RemoveClient( unsigned int ID )
{
std::list<Client>::iterator it;
for( it = m_listClients.begin();
it != m_listClients.end();
++it )
{
if ( it->ClientID() == ID )
{
m_listClients.erase( it );
break;
}
}
}
The problem is that this seems to corrupt the heap in my program. I
know that the iterator is corrupted when I use the erase command, but
why would that corrupt the heap?
Is this not the way to delete an item from the middle of a list?
Should I not be using ::list for this type of purpose?
As others have stated, there are a few problems with this code, or can be.
First, as stated, erase invalidates iterators. So you need to use the
algorithm:
for( it = m_listClients.begin(); it != m_listClients.end(); )
{
if ( it->ClientID() == ID )
{
it = m_listClients.erase( it );
break;
}
else
{
++it;
}
}
Second, the desturctor for the Client is not being called. Now, if clients
are creating using new, then you'll need to delete the pointer with delete.
This will also free the memory so you don't get a memory leak.
for( it = m_listClients.begin(); it != m_listClients.end(); )
{
if ( it->ClientID() == ID )
{
delete (*it);
it = m_listClients.erase( it );
break;
}
else
{
++it;
}
}
This depends though on the ownership of the Client pointer. Who owns the
pointer? If your m_list_Clients is the only place you are keeping this
pointer, then m_listClients owns the pointer and needs to delete it when it
gets erased.
--
Jim Langston
tazmaster@rocketmail.com