Resource leak due to delete operator

Question

Here is the code that I have written:

class sCircBuffer
{
    public:
        sCircBuffer(void);
        ~sCircBuffer(void);
        double *Data;
        int Size;
        bool Init(int SizeBuffer);
        bool Delete();
}

sCircBuffer :: sCircBuffer(void) //Constructor
{
    Data=NULL; //Initialize input circular buffer
}

sCircBuffer :: ~sCircBuffer(void) //Destructor
{
    delete [] Data; //Initialize input circular buffer
    Data=NULL;
}

bool sCircBuffer :: Init(int SizeBuffer)
{
    delete [] Data; //Initialize input circular buffer
    Data=new double [SizeBuffer]; //Initialize input circular buffer    
    Size=SizeBuffer;
    for (int i=0; i<Size; i++)
        Data[i]=0;
    return true;
}

bool sCircBuffer :: Delete()
{
    delete [] Data; //Initialize input circular buffer
    Data=NULL;
    return true;
}

I am creating an object of above class in another class:

class PerChannel
{
    public:
        PerChannel();
        ~PerChannel();

        sCircBuffer m_InputDataRaw;
}

PerChannel :: PerChannel()
{
    m_InputDataRaw.Init(MAX_NUM_TO_FETCH); // MAX_NUM_TO_FETCH = 1000
}

PerChannel :: ~PerChannel()
{
    m_InputDataRaw.Delete();
}

In Coverity and C++ Memory Validator, I am getting resource leak error in the constructor of PerChannel.

I am not sure what is wrong here?

Your help is really appreciated.

Chintan

Sadly, you're not showing us the relevant code, as I'm guessing copy constructors will be involved. Compile a minimal example that reproduces the problem: http://sscce.org/ — avakar, Jul 23 '13 at 14:15
Are you sure about the `Delete` method signature? The declaration says it takes an argument, but not the definition. — Some programmer dude, Jul 23 '13 at 14:15
In `init` you are assigning to `Size` which is nowhere defined. In the `PerChannel` Constructor and Destructor you are accessing a member that is nowhere defined. Are you sure about the code you have posted? — ogni42, Jul 23 '13 at 14:25
I think you need to read about [the rule of three](http://en.wikipedia.org/wiki/Rule_of_three_%28C%2B%2B_programming%29). — Some programmer dude, Jul 24 '13 at 06:46

tomi.lee.jones · Answer 1 · 2013-07-23T14:47:53.413

1

In the init method of sCircbuffer you try delete [] a pointer that points to NULL.

edited Jul 23 '13 at 14:47

answered Jul 23 '13 at 14:38

tomi.lee.jones

1,563
1
15
22

1

`delete [] Data` is perfectly valid C++, even if Data is NULL. – Adrian McCarthy Jul 23 '13 at 18:36

score 0 · Answer 2 · answered Jul 23 '13 at 14:39

0

First off, it is unclear what m_InputDataTelem is, as you have specified m_InputDataRaw as being a circular buffer in the class, not m_InputDataTelem.

As of now, you are making a call to init, without first constructing the sCircBuffer, hence trying to delete a buffer that has not been initialized yet (access violation).

answered Jul 23 '13 at 14:39

Igneous01

719
1
10
24

I have edited the post. Please have a look at it again. Thanks – chintan s Jul 23 '13 at 14:43
The issue hasn't changed: in init() you are deleting the buffer, but because you have not constructed the circularbuffer (and your constructor does not allocate on heap) you are getting an access violation. You should be checking in init to see if (Data != NULL), then delete. Also, you should first construct the circular buffer in your perChannel class, then call init() if need be. – Igneous01 Jul 23 '13 at 14:50

score 0 · Answer 3 · answered Jul 23 '13 at 14:59

If I were you, I would definitelly changhe the class interface, for example by defining a constructor like:

sCircBuffer(int size = SOME_USEFUL_VALUE);

and allocating my initial buffer in it. Let me say that the Delete() method is an horrible idea: you should simply handle deletion inside your Init() method (which I would call Resize(), for example). Letting the user of your class to delete the internal buffer... Well, it simply defeats what Object Oriented Design is all about.

Anyway, if you don't want to get into trouble keeping your code as it is, you shuold at least add in your constructor these lines:

Size= 0;
Data= new double [Size];

But I still believe that initializing that to a meaningful size would be a better solution.

Resource leak due to delete operator

3 Answers3