2

I wrote the following dummy class to understand how the copy constructor,the copy assignment operator and the destructor works:

#include <string>
#include <iostream>

class Box {

  public:
    // default constructor
    Box(int i=10,const std::string &t=std::string()) : a(i),s(new std::string(t)) {}
    // copy constructor
    Box(const Box &other) { a=other.a; s=new std::string(*other.s); }
    // copy assignment operator
    Box &operator=(const Box &other) { a=other.a; s=new std::string(*other.s); }
    // destructor
    ~Box() { std::cout<<"running destructor num. "<<++counter<<std::endl; }
    int get_int() { return a; }
    std::string &get_string() { return *s; }
  private:
    int a;
    std::string *s;
    static int counter;

};

int Box::counter=0;

I'm using this class type in my code to test how it works but I was thinking about the implications in destroying objects which have a member of built-in pointer type:

#include "Box.h"

using namespace std;

int main()
{
  Box b1;
  Box b2(2,"hello");
  cout<<b1.get_int()<<" "<<b1.get_string()<<endl;
  cout<<b2.get_int()<<" "<<b2.get_string()<<endl;
  Box b3=b1;
  Box b4(b2);  
  cout<<b3.get_int()<<" "<<b3.get_string()<<endl;
  cout<<b4.get_int()<<" "<<b4.get_string()<<endl;
  b1=b4;
  cout<<endl;
  cout<<b1.get_int()<<" "<<b1.get_string()<<endl; 
  {
    Box b5;
  }  // exit local scope,b5 is destroyed but string on the heap
     // pointed to by b5.s is not freed (memory leak)
  cout<<"exiting program"<<endl;
}

This pointer is initialized in the constructor to point to a (always new) dynamically allocated memory on the free store. So,when the destructor is called, members of the object to be destroyed are destroyed in reverse order. Is it right in this case, that only the int and the pointer objects are destroyed, and I end up having a memory leak (the string on the heap is not freed)?

Moreover, defining this copy assignment operator, do I have a memory leak every time I assign an object (the pointer points to a new object on the heap and the former is lost isn't it?) ?

Luca
  • 1,658
  • 4
  • 20
  • 41
  • Have a look here: http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three – πάντα ῥεῖ Sep 09 '15 at 10:50
  • 2
    And yes, your code leaks like a sieve. – πάντα ῥεῖ Sep 09 '15 at 10:50
  • @πάνταῥεῖ yes, that was the whole point.. – Luca Sep 09 '15 at 11:28
  • BTW I'd suspect that's just a simplified example, but in real code you woulldn't use a pointer to `std::string`, but just a plain member variable. Also you shouldn't use raw pointers and manage dynamic memory allocation, but use the [smart pointers](http://en.cppreference.com/w/cpp/memory) provided by the c++ standard library. – πάντα ῥεῖ Sep 09 '15 at 11:47
  • @πάνταῥεῖ yes, it's just for learning purpose, I'm trying some weird things out just to get the hang of C++ syntax and corner cases – Luca Sep 09 '15 at 12:39
  • It might be more illustrative if you used a pointer to your own class, say `Thing` instread of `string` and then counted outstanding instances of `Thing` as well – Glenn Teitelbaum Sep 09 '15 at 15:15

3 Answers3

4

Each time you call new, you have to delete it (except are shared pointers).

So you have to delete the string in the destructor.

The assignment operator works on an existing instance, so you already created s and do not have to create a new string for s.

the destructor destructs its members. Since a pointer is like a int, only the variable holding the address is destructed, not the object it is pointing to.

So yeah, you will have a memory leak in each object and everytime you use the assignment operator the way you designed your class.

Gombat
  • 1,994
  • 16
  • 21
2

Keep in mind allocation happens on base construction, copy construction and surprisingly conditionally on assignment

Deallocation happens in the destructor and conditionally on assignment

The condition to watch for is:

x = x;

So your code can be changed to the following pattern (in cases where you are not able to use the preferred appropriate smart pointer)

  Box(int i=10,const std::string &t=std::string()) : a(i),s(new std::string(t)) {}
// copy constructor
  Box(const Box &other) { cp(other); }
// copy assignment operator
  Box &operator=(const Box &other) {
    if (&other != this)  // guard against self assignment
    {
       rm();
       cp(other);
    }
    return *this;
  }
// destructor
  ~Box() { rm();  }
private:
  void cp(const Box &other) {a=other.a; s=new std::string(*other.s);
  void rm() {
    std::cout<<"running destructor num. "<<++counter<<std::endl;
    delete s;    // prevents leaks
  }
Glenn Teitelbaum
  • 10,108
  • 3
  • 36
  • 80
0

One possible way to deal with unnamed dynamically allocated members is to save them in a container every time they are created (in an object, function, etc), and then to run a for loop in your destructor with a delete statement followed by the elements of the container.

You can do this with a vector:

vector <string*> container;

and you can use it as follows:

// define it in the private members of your class
vector <string*> container;

// use it when you create the string
container.push_back(new dynamicallyAllocatedObject);

// free memory in the destructor
for(auto it = container.begin(); it != container.end(); ++it) delete *it;
Ziezi
  • 6,375
  • 3
  • 39
  • 49