CODEWITHSUNDEEP
c++visual-c++memory-managementmemory-leaks
chintan s
chintan s

Reputation: 6488

Resource leak from an array

I have following class:

class estimate
{
    public:
        estimate();
        ~estimate();
        double *tHanning;
}

estimate::estimate()
{
    tHanning = NULL;
    tHanning = new double [1000];

    for (int m=0; m<1000; m++)
    {
        tHanning[m]=(0.5-0.5*cos(2.0*PI*(m+1)/(1001))); 
    }
}

estimate::~estimate()
{
    delete [] tHanning;
    tHanning = NULL;
}

I am not sure why C++ Memory Validator shows resource leak in the constructor when I assign "new" to the variable.

Can somebody please help me?

Edit: How I sue the above class:

class HBMain
{
    public:
        HBMain();
        ~HBMain();
        bool Init();
        estimate *objEstimate;
}

HBMain :: HBMain()
{
    objEstimate = NULL;
}

HBMain :: ~HBMain()
{
    delete objEstimate;
    objEstimate = NULL;
}

bool HBMain :: Init()
{
    ....
    objEstimate = new estimate();
    ....
}

Upvotes: 2

Views: 168

Answers (2)

Mr.C64
Mr.C64

Reputation: 42924

Both your estimate class's constructor and destructor seem fine (you dynamically allocate memory with new[] in the constructor, and delete[] it in the destructor).

But I think you may have problems during copies.

In fact, your class has default copy constructor and operator=, but the default behavior is not good in this case; in fact, the default behavior is just member-wise copy, but copying the raw pointer tHanning data member is a "leaktrocity".

Either disable copy constructor and operator= (e.g. declaring them private in C++98/03, or using the new = delete C++11 syntax), or give a proper implementation for them.

Proper implementation should do deep copy of the owned array (with proper deletion of any previously allocated array).

I think the best thing you can do is to use std::vector<double> instead of the raw pointer.

If you are really such highly constrained on the (relatively small) overhead of std::vector, then consider using a smart pointer like std::unique_ptr:

#include <memory>   // for std::unique_ptr

class estimate
{
private:
    std::unique_ptr<double[]> tHanning; // <--- SMART pointer

public:
    estimate()
        : tHanning(new double[1000])
    {
        for (int m = 0; m < 1000; m++)
        {
            tHanning[m] = (0.5-0.5*cos(2.0*PI*(m+1)/(1001))); 
        }
    }

   // No need to define destructor - smart pointer automatically deletes its owned stuff
};

In this way, the class will be movable but not copyable, and you will incur almost zero overhead if compared to the raw pointer case.

As a bouns reading, consider the Rule of Three in C++.

Again, in your HBMain class, don't use a raw estimate *objEstimate pointer data member. Instead, use smart pointers like: 

class HBMain
{
  ....
private:
    std::unique_ptr<estimate> objEstimate;

   // Again, no need to define a custom destructor for deleting objEstimate.
   // std::unique_ptr will do proper deletion *automatically*.
};

Upvotes: 1

Rak
Rak

Reputation: 76

As an alternative solution, why not simply change your pointer by a vector of double ?

You will avoid headaches about memory leaks.

Edit: For the HBMain class, you can also changed your naked pointer by a smart pointer (shared_ptr) from C++11 or the Boost library and delete the destructor. And so you will not have to implement all the boilerplate code.

But, do you really need a dynamic allocation for the HBMain property ?

Upvotes: 1

Related Questions