0

I was going through this post Why do we need to delete allocated memory in C++ assignment operator? and I had a question about the memory allocated by new operation inside the assignment operator. How will it be freed after we assign to a MyString object testObject? Will the destructor for the testObject be called when it goes out of scope or I will have to call delete explicitly to free that memory?

const MyString& operator=(const MyString& rhs)
{ 
    if (this != &rhs) {
        delete[] this->str; // Why is this required?
        this->str = new char[strlen(rhs.str) + 1]; // allocate new memory
        strcpy(this->str, rhs.str); // copy characters
        this->length = rhs.length; // copy length
    }
    return *this; // return self-reference so cascaded assignment works
}
Community
  • 1
  • 1
vkaul11
  • 4,098
  • 12
  • 47
  • 79
  • It should be freed in the destructor, if you have implemented it correctly. – juanchopanza Apr 24 '13 at 22:12
  • 7
    Not exactly a duplicate, but you need to know all about the [Rule of Three](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) if you're going to write a resource management class. – Mike Seymour Apr 24 '13 at 22:14
  • Yes, I know but will the destructor free the memory allocated in the assignment operator? I thought it will only free memory allocated in the copy constructor. – vkaul11 Apr 24 '13 at 22:15
  • 1
    @vkaul11: The destructor should contain `delete[] str;`, which will free the most recently allocated array, wherever it was allocated. That's also why you need to delete the old array in that operator, before you overwrite your pointer to it on the next line. – Mike Seymour Apr 24 '13 at 22:17
  • @vkaul11 It will free whatever `str` is assigned to at the time. Regardless of who, when, or where it was assigned. – David Apr 24 '13 at 22:17
  • 5
    The destructor will free whatever memory it's instructed to free. – Nik Bougalis Apr 24 '13 at 22:17
  • +1 to what Nik Bougalis said - the destructor _should_ free everything that has been allocated during the lifetime of the object. That doesn't mean that it does so automatically. You have to write the destructor-code yourself. – Excelcius Apr 24 '13 at 22:21
  • @Excelcius The destructor cannot possibly know everything that has been allocated during the lifetime of the object. And it shouldn't delete things that have already been deleted. – James Kanze Apr 24 '13 at 22:52
  • The implementation of `operator=` that you show is broken, and can leave the object in an inconsistent state, in which it cannot be destructed. – James Kanze Apr 24 '13 at 22:52
  • @JamesKanze: Ok, what I meant was: Everything which has been _dynamically_ allocated by the object, hasn't been freed already and is still owned by the object. But that's not the point, I'm just still under the impression that the OP thinks everything which has been allocated in the constructor automatically will be freed in the destructor (without instructing it to do so) and that's wrong. – Excelcius Apr 25 '13 at 04:52
  • @Excelcius I agree with your categorisation of what the OP thinks (although only he could confirm it). But I think your statement was misleading as well: the requirement is that everything you explicitly allocate be explicitly deleted; many classes will delete memory before the destructor, and this, of course, should _not_ be deleted in the destructor. – James Kanze Apr 25 '13 at 07:45

3 Answers3

3

What happens when you have this?

{
  MyString s = "Something";
}

This will first construct a MyString, which will presumably dynamically allocate an array of chars to store the string data. Then the s variable goes out of scope and the MyString object is destroyed. It's destructor should clean up any dynamically allocated memory by doing delete[] str.

Let's say you use it like this instead:

{
  MyString s = "Something";
  s = some_other_string;
}

Now the MyString object is constructed in the same way, allocating memory for the string data. The second line will then call the assignment operator. If it as implemented as you have described, the existing allocated char array will be deleted and a new one will be allocated containing the same string data as some_other_string. It is then this newly allocated array that will be destroyed by the destructor when s goes out of scope.

The destructor just delete[]s whatever the member str is pointing at. After the assignment operator has been called, it is delete[]ing the newly allocated array.

Joseph Mansfield
  • 108,238
  • 20
  • 242
  • 324
  • Thanks for your answer. It makes it clear. I was thinking as if the destructor is only called for the memory allocated by the constructor and not the assignment operator. – vkaul11 Apr 24 '13 at 22:20
  • While the explination is good as far as it goes, it neglects to point out that you _must_ allocate the new memory block before freeing the old, or at least, you must ensure that the member pointer doesn't point to a deleted block when you allocate the new. – James Kanze Apr 24 '13 at 23:04
0

You are overwriting the value str when you allocate the string array to hold the copy of the content of rhs. If you have not delete'd str before you overwrite it, you will never be able to delete what it used to point to from the heap. That memory block it used to point to will still be reserved in the heap, and will be unavailable for re-use. If your program does this enough times, you will run out of space in the heap and your program will die. That is called a memory leak.

antlersoft
  • 14,636
  • 4
  • 35
  • 55
0

First, you don't necessarily have to delete in the assignment operator. You only have to delete if you overwrite a pointer to previously dynamically allocated memory. A better implementation of MyString would be to keep track of the capactity as well, and only reallocate (and delete) if you need more capacity.

Also, in your implementation, you delete before allocating. This is will result in undefined behavior if the allocation fails; you must do everything which might fail before the delete. In which case, you don't need the test for self assignment; the necessity of a test for self assignment is generally a signal that your assignment operator is broken.

Putting these two things together, we get something like:

MyString const&
MyString::operator( MyString const& other )
{
    if ( capacity < other.length ) {
        char* tmp = new char[ other.length ];
        delete str;
        str = tmp;
        capacity = other.length;
    }
    memcpy( str, other.str, other.length );
    length = other.length;
    return *this;
}

The delete is conditional, the delete is after the allocation, and we always use the length member for the length, rather than mixing strlen and the length member.

James Kanze
  • 150,581
  • 18
  • 184
  • 329