string::string(char* data_pointer)
{
_size = (long)strlen(data_pointer);
_pointer = new char[_size];
memcpy(_pointer, data_pointer, _size);
};
ncommon::string test = ncommon::string("test");
Issue with memory leak
Author
I am string to build a string class as a learning example(so please no post about just use the std::string). this is the construct for a char pointer.
now this work fine when i create the string with new but when i just do
I get the memory leak because the destructor is never called which deletes the memory created in the constructor. Is there a way to fix this? I am going to implement some pre allocated data of 20 or so but I know that is the string is bigger than 20 character I will get the error, it there any way to avoid this(i don't think so since the std::string has the same memory leak with a big enough string)?
Quote:Original post by 3dmodelerguy
I get the memory leak because the destructor is never called which deletes the memory created in the constructor. Is there a way to fix this?
The destructor is *always* called (unless there is an exception thrown from the constructor).
I have a feeling that your problem is caused by a missing (or incorrect) copy-constructor.
Rule of Three
You need to define a copy constructor and assignment operator.
The above line first creates a temporary ncommon::string() object, then object is used to initialize test via copy-constructor. After all that happens the destructor is called for the temporary object thereby deleting its internal heap allocated members.
The problem is that the default copy and assignment operators perform "shallow copies", you want to perform a "deep copy".
You need to define a copy constructor and assignment operator.
ncommon::string test = ncommon::string("test");The above line first creates a temporary ncommon::string() object, then object is used to initialize test via copy-constructor. After all that happens the destructor is called for the temporary object thereby deleting its internal heap allocated members.
The problem is that the default copy and assignment operators perform "shallow copies", you want to perform a "deep copy".
Quote:Original post by 3dmodelerguy
does that mean the std::string also has an incorrect copy construct?
No, it means that std::string has a manually defined correct copy constructor.
Quote:Original post by SneftelQuote:Original post by 3dmodelerguy
does that mean the std::string also has an incorrect copy construct?
No, it means that std::string has a manually defined correct copy constructor.
... and assignment operator, and destructor. Re: Rule of Three.
Author
Quote:Original post by SneftelQuote:Original post by 3dmodelerguy
does that mean the std::string also has an incorrect copy construct?
No, it means that std::string has a manually defined correct copy constructor.
but when I created a string will std:string that is say 60 character longs I get a memory leak
Quote:Original post by 3dmodelerguyQuote:Original post by SneftelQuote:Original post by 3dmodelerguy
does that mean the std::string also has an incorrect copy construct?
No, it means that std::string has a manually defined correct copy constructor.
but when I created a string will std:string that is say 60 character longs I get a memory leak
I don't understand your question. In short: std::string works fine, your class is broken.
Anywhoo here is what is wrong with your code:
First, you aren't accounting for the null terminator in your original constructor.
string::string(char* data_pointer){ _size = (long)strlen(data_pointer) + 1; // Add one for the null terminator _pointer = new char[_size](); // Make sure buffer is zereo'd memcpy(_pointer, data_pointer, _size);};Here is the user defined copy constructor that you're lacking:
string::string(const string& rhs){ _size = (long)strlen(rhs._pointer) + 1; // Add one for the null terminator _pointer = new char[_size](); // Make sure buffer is zereo'd memcpy(_pointer, rhs._pointer, _size);};Here is the user defined assignment operator that you're lacking:
string::operator=(const string& rhs){ _size = (long)strlen(rhs._pointerr) + 1; // Add one for the null terminator _pointer = new char[_size](); // Make sure buffer is zere'od memcpy(_pointer, rhs._pointer, _size); return *this;};And lastly you need a custom destructor:
string::~string(){ if(_pointer) { delete [] _pointer; _pointer = 0; }};Quote:Original post by 3dmodelerguy
but when I created a string will std:string that is say 60 character longs I get a memory leak
Then it is due to a bug in your code. Getting this sort of thing right is trivial. Either the string hasn't been destroyed yet when you call your leak checker, or you're misreading the results.
Quote:Original post by fpsgamer
Here is the user defined assignment operator that you're lacking:
*** Source Snippet Removed ***
You forgot to free the previous memory addressed in _pointer ;)
string& string::operator=(const string& rhs){ char* oldPointer = _pointer;//don't do the delete immediately, in case new throws an exception. This way the internal state is preserved. long size = (long)strlen(rhs._pointer) + 1; // Add one for the null terminator _pointer = new char[size](); // Make sure buffer is zere'od _size = size; delete [] oldPointer; memcpy(_pointer, rhs._pointer, size); return *this;};This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
