Search code examples
c++memory-leaksthisvalgrindnew-operator

c++ - replace this by new object


Basically I want the this-pointer in foo() to point at a new object which points at *this. Since I can't change the this-pointer, I am creating a new object which just takes over the array from *this and then empty *this and let it point at the newobject.

struct SomeClass{
  int* value;
  SomeClass* child;

  SomeClass() : value{nullptr}, child{nullptr}{}

  void foo(){
    SomeClass* otherobject = new SomeClass;
    otherobject->value = value;
    value = nullptr;
    child = otherobject;
  }

  ~SomeClass(){
    if(child){
      child->~SomeClass();
    }
    delete[] value;
  }
};

int main() {
  SomeClass objekt;
  objekt.value = new int[10];
  for (int i = 0; i<10; i++)
    objekt.value[i] = i;
  
  objekt.foo();
  return 0;
}

The problem is that valgrind tells me the program had memory leaks. Does anyone know how to solve it?


Solution

  • In the ~SomeClass() destructor, you need to change:

    if(child){
        child->~SomeClass();
    }
    

    to:

    delete child;
    

    There is more to destructing an object then just calling its destructor. The memory used by the object needs to be freed as well. That happens outside of the destructor. The destructor itself doesn't know HOW the object was allocated (In automatic memory? In dynamic memory?), so calling the destructor directly, as you are, will not free the memory used by the object. But calling delete on the object will call its destructor AND free its memory (provided it was allocated with new to begin with, as it is in your example).


    Also, SomeClass is not following the Rule of 3/5/0. You need to add (or delete) a copy constructor and copy assignment operator, and a move constructor and move assignment operator, eg:

    struct SomeClass{
      int* value = nullptr;
      SomeClass* child = nullptr;
    
      SomeClass() = default;
    
      // delete'd since we don't know how many ints to copy...
      SomeClass(const SomeClass &src) = delete;
    
      SomeClass(SomeClass &&src) : value(src.value) {
        src.value = nullptr;
      }
    
      // delete'd since we don't know how many ints to copy...
      SomeClass& operator=(const SomeClass &) = delete;
    
      SomeClass& operator=(SomeClass &&rhs){
        std::swap(value, rhs.value);
        return *this;
      }
    
      ~SomeClass(){
        delete child;
        delete[] value;
      }
    
      void foo(){
        delete child;
        child = new SomeClass(std::move(*this));
      }
    };
    

    That being said, this can be made better/safer if you used std::vector and std::unique_ptr instead:

    struct SomeClass{
      std::vector<int> value;
      std::unique_ptr<SomeClass> child;
    
      SomeClass(int size = 0) : value(size) {
      }
    
      SomeClass(const SomeClass &src) : value(src.value) {
      }
    
      SomeClass(SomeClass &&src) : value(std::move(src.value)) {
      }
    
      SomeClass& operator=(SomeClass rhs){
        std::swap(value, rhs.value);
        return *this;
      }
    
      void foo(){
        child = std::make_unique<SomeClass>(std::move(*this));
      }
    };
    
    int main() {
      SomeClass objekt(10);
      for (int i = 0; i < 10; i++)
        objekt.value[i] = i;
      
      objekt.foo();
      return 0;
    }