CODEWITHSUNDEEP
c++dictionarypointersstdstdmap
NaSh
NaSh

Reputation: 665

Avoid memory leakage when removing object pointer from the map

I create an object_ptr and insert it into a std::map. Then in a different part of the code, I find the object using the key and erase the value from the map and delete the object.

There seems to be a Memory-leak though, how can I find and fix it?

void foo(){
    Request * req = new Request();
    MyMap.insert (std::pair<int, Request *> (address, req));
    bar(address);
}
void bar(int address){
  map<int, Request*>::iterator it_req = MyMap.find(address);
  MyMap.erase(it_req);
  delete it_req->second;
}

Upvotes: 2

Views: 711

Answers (1)

BoBTFish
BoBTFish

Reputation: 19757

The problem is here:

MyMap.erase(it_req); 
delete it_req->second;

Once you have erased the element, that iterator is no longer valid. You can't dereference it to get the pointer to delete. The simplest solution is just to reverse these two lines: delete then erase.

However, a better approach is to not have to delete at all. Do you really need to dynamically allocate the Request? Can you just store it directly in the map?

std::map<int, Request> myMap;

Then you don't need to worry at all. Just do myMap.erase(it_req);; no delete to worry about. This is actually the simplest solution, in terms of logic to think about and amount of code (but will be a slightly larger change to your existing code, probably).

If you really do need to dynamically allocate the Request, for whatever reason, instead of storing a raw pointer, you could store a smart pointer, which will automatically delete the pointed-to object when it is destroyed. The default choice for this would be std::unique_ptr.

std::map<int, std::unique_ptr<Request>> myMap;

Then you again only need to do myMap.erase(it_req);; no manual delete to worry about.

Upvotes: 6

Related Questions