memory errors with std::map
Hello everyone,
I have some troubles with memory handling of std::map.
Here is an example of code.
Code:
#include <map>
#include <iostream>
struct MonDouble{
MonDouble(double d): v(d) {}
~MonDouble(){}
double v;
};
int main(int /*argc*/, char **/*argv*/) {
std::map<std::string,MonDouble*> mymap;
std::map<std::string,MonDouble*>::iterator it;
//insert one value (Part 1)
it = mymap.find("VAL");
if(it != mymap.end()){
delete it->second;
}
mymap.insert(std::make_pair("VAL",new MonDouble(1.5)));
//modify the value (Part 2)
it = mymap.find("VAL");
if(it != mymap.end()){
delete it->second;
}
mymap.insert(std::make_pair("VAL",new MonDouble(1.5)));
//delete the value (Part 3)
it = mymap.find("VAL");
if(it != mymap.end()){
delete it->second;
}
}
using valgrind :
Code:
valgrind --leak-check=yes main
I have "Invalid free() / delete / delete[]" errors on deletes of Part 2 and 3.
If I remove Part 2 (the modification), I have no error.
I don't see the error I made. Do you have the same problem ?
In advance, thank you for any answer
Re: memory errors with std::map
For starters, why not use a std::map<std::string,MonDouble> instead?
As for your problem: you did not erase the element from the map, and then attempted to insert another element with the same key. The insertion should have failed, but as you made no attempt to check for this, you went ahead with the find and delete, resulting in double deletion.
Re: memory errors with std::map
Thank you laserlight, to be so fast :-)
First this is a simplified example, and actually a really need to store pointers and not objects.
Second, I'm not sure to understand. But are you saying that, by using delete it->second, I only delete the MonDouble instantiation and not the pair<string,MonDouble*> of the map ?
Re: memory errors with std::map
Quote:
Originally Posted by
rilpo
First this is a simplified example, and actually a really need to store pointers and not objects.
In that case, I recommend you consider using smart pointers.
Quote:
Second, I'm not sure to understand. But are you saying that, by using delete it->second, I only delete the MonDouble instantiation and not the pair<string,MonDouble*> of the map ?
Correct.
Re: memory errors with std::map
Ok thank you,
I use "mymap.erase(it)" just after the delete it->second and it works ..
But I'm not sure this is most efficient..
Smart pointers have been abandoned in my project (not my choice :-))
Thank you again laserlight.
Re: memory errors with std::map
Without using smart pointers it's hard to do better for the erase portion. However, for the modify step you don't need to remove and recreate the mapping because only the value is changing, not the key:
Code:
//modify the value (Part 2)
it = mymap.find("VAL");
if(it != mymap.end()){
delete it->second;
it->second = new MonDouble(1.5);
}
else
mymap.insert(std::make_pair("VAL",new MonDouble(1.5)));
Re: memory errors with std::map
Yes this solution is better,
Thank you lindley.
Re: memory errors with std::map
Quote:
Originally Posted by
rilpo
First this is a simplified example, and actually a really need to store pointers and not objects.
A pointer will have same size as a double with newer compilers. Why should there be a need to store a pointer rather than an object (a copy)?
If you want to store the same variable into multiple containers and therefore have to use pointers, in my opinion you make a design error where I know not one single case where this really must be done that way. When the pointer must be deleted you need to update all the containers where the pointer is stored anyway. So you could do an update in multiple containers as well if storing a copy of the same object into multiple containers (if that really was necessary at all where I have serious doubts about.
On the contrary, storing pointers to containers nearly always give problems like the one you did encounter here and storing them to multiple containers always will lead to highly error-prone software which only can be maintained badly. I can see from your response that you already know that and the sooner you would make a new design the sooner you would solve the problems that arise only because of this.
Re: memory errors with std::map
Quote:
Originally Posted by
laserlight
For starters, why not use a std::map<std::string,MonDouble> instead?
As for your problem: you did not erase the element from the map, and then attempted to insert another element with the same key. The insertion should have failed, but as you made no attempt to check for this, you went ahead with the find and delete, resulting in double deletion.
I perceive using a pointer or a reference is just the same but coders of segmented lib code love to read real values rather than handles. What library is that actually ?
Re: memory errors with std::map
Quote:
Originally Posted by Ledidas
I perceive using a pointer or a reference is just the same
Your perception is not reality. For starters, a reference type cannot be used. With a pointer, there is the question of ownership; with a value, the container owns the object.
Quote:
Originally Posted by Ledidas
coders of segmented lib code love to read real values rather than handles. What library is that actually ?
Huh?
Re: memory errors with std::map
Quote:
A pointer will have same size as a double with newer compilers. Why should there be a need to store a pointer rather than an object (a copy)?
Actually, itsmeandnobodyelse, in this software, pointers are here to handle polymorphism.
The container can contains MyDouble, or MyInt.. that inherit MyValue.
When possible we try not to use pointers. To many memory errors can occur with pointers.
Note : I really appreciate these forums; always someone to talk to :-) Thanks to you guys
Re: memory errors with std::map
Quote:
Originally Posted by
rilpo
Actually, itsmeandnobodyelse, in this software, pointers are here to handle polymorphism.
The container can contains MyDouble, or MyInt.. that inherit MyValue.
When possible we try not to use pointers. To many memory errors can occur with pointers.
If you want to store one of a limited set of built-in types, consider using boost::variant.
Re: memory errors with std::map
Quote:
Originally Posted by
rilpo
pointers are here to handle polymorphism. The container can contains MyDouble, or MyInt.. that inherit MyValue.
Yes, it might be senseful to store baseclass pointers and function pointers into containers. But you also could make a Variant type which stores the pointer and handles all creation/allocation/freeing issues and especially handles copy paradigm. That way your map has a normal object to store and you have the polymorphism functionality well encapsulated into a C++ class.
Re: memory errors with std::map
Quote:
Originally Posted by
laserlight
Your perception is not reality. For starters, a reference type cannot be used. With a pointer, there is the question of ownership; with a value, the container owns the object.
Hello, how do you define "starters" ? I'm embarassed, I'm a starter of C++, and also learned use of references and pointers in basics C++, could you give an example of the difference you stated ?
Re: memory errors with std::map
References cannot be used in containers because they are not assignable. Once a reference is pointing to something, it cannot be redirected to point at something else.
Re: memory errors with std::map
Quote:
If you want to store one of a limited set of built-in types, consider using boost::variant.
Thanks for the advice. I've looked at this solution (few months ago and for another project).
In my case, there were some reasons no to use it :
- of course it is required to link with boost and it seems to me (personal point of view) that boost does not maintain stable branches and so API is not stable (change of version) or bugs are not fixed in earlier versions. I very like boost but there is a cost of using it. And Boost::variant won't be in the C++0X standard (If I'm right).
- Variant is for built-in types only and that's not my case.
- and finaly maybe less obvious, If you want to let the "user" to be able to implement his own MyValue and to store these obects in this container Variant is not adapted. Moreover the template is limited with 10 arguments (but maybe we can set it higher).
Re: memory errors with std::map
Quote:
Originally Posted by
rilpo
Thanks for the advice. I've looked at this solution (few months ago and for another project).
In my case, there were some reasons no to use it :
- of course it is required to link with boost and it seems to me (personal point of view) that boost does not maintain stable branches and so API is not stable (change of version) or bugs are not fixed in earlier versions. I very like boost but there is a cost of using it. And Boost::variant won't be in the C++0X standard (If I'm right).
The API is relatively stable, and the variant library is header-only so no linking required.
Quote:
- Variant is for built-in types only and that's not my case.
I believe it's actually safe for any type. That's what sets it apart from unions.
Quote:
- and finaly maybe less obvious, If you want to let the "user" to be able to implement his own MyValue and to store these obects in this container Variant is not adapted. Moreover the template is limited with 10 arguments (but maybe we can set it higher).
That limit will be lifted just as soon as any of the compiler vendors get around to implementing C++0x variadic templates.....
