c++ correct deletion of structs and pointers

Question

I have a question regarding how to correctly delete structs and it's respective pointers declared inside.

I have extracted an example from a project i have running and it doesn't seem to work correctly, the code doesn't crash but it seems i have some "memory leaks". I'm not sure that is the right wording. The issue is that the values is not really reset and are kept in the memory next time i initiate a class.

Sudocode below:

Header:

ProgramHeader.h

class ClassA : public publicClassA
{
    public:
        ClassA(void);

        virtual ~ClassA();

    private:

    struct ApStruct{
            struct
            {
                float *refA[2];
                float *refB[2];
                float *pVarA;
            } fR;

            struct
            {
                float *refA[2];
                float *refB[2];
                float *pVarA;
            } f1kHz;
        };
        ApStruct* GetApStruct;  
}

Program:

Program.cpp

#include "ProgramHeader.h"

ClassA::~ClassA()
{
    //EDIT i did a typo my looks like this:
    //delete ApStruct; //Wrong code
    delete GetApStruct; //Corrected - however still not working
}

main()
{
    GetApStruct  = new ApStruct();

    //Do Code
}

Hope it all makes a bit sense,

EDIT: I have updated one wrong line in the code - however the question still remains the same. I will have a look at below to understand before i implement a solution.

EDIT 24/10/2015 I have been trying out a few of the suggestions below and im not able to find a solution to my issue, i must admit i also have difficulties to narrow it down what could cause it.

My code is part of a DLL. The code wraps some source code im not in control of, and therefore i have limited options how i init using constructors and new on pointers.

The reason i still think i have memory leak issues is if i add a "magic float" in my code the output of my functions change, even the float is not used anywhere - it is just declared.

I get different results when:

1. Calling InitCode - once!
2. then i will call CallCode multiple time - doing my calculations
3. Destruct the instance of the class

When i repeat the above again i get different result from the first time i run the code but afterwards it stays the same.

If i include the magic line all seems to work???

Updated SudoCode:

Program.cpp

#include "ProgramHeader.h"

ClassA::~ClassA()
{
    //EDIT i did a typo my looks like this:
    //delete ApStruct; //Wrong code
    delete GetApStruct; //Corrected - however still not working
}

main()
{
    void initCode()
    {
        GetApStruct  = new ApStruct();

        float InitValue = 0.F

        //Magic line:
        float magicLine = 123456.f; //If this line is commented out i get different results in my code
        //End Magic Line

        fr.refA[0] = &InitValue;
        fr.refA[0] = &InitValue;
        fr.refA[0] = &InitValue;
        fr.pVarA   = &InitValue;
        ... 
    }

    void CallCode()
    {
        float CallValue = 123.F

        //Magic line:
        float magicLine = 123456.f; //If this line is commented out i get different results in my code
        //End Magic Line

        fr.refA[0] = &CallValue;
        fr.refA[0] = &CallValue;
        fr.refA[0] = &CallValue;
        fr.pVarA   = &CallValue;
        ...
    }
}

Thanks guys for you support,

Thomas

Answer 1

Alright, let me be direct:

If you are using new or delete, you are doing it wrong.

Unless you are an experienced user, or you wish to implement a low-level side project, do not ever use new and delete.

Instead, use the existing standard classes to handle memory ownership, and just avoid heap-allocation when it is unnecessary. As a bonus, not only will you avoid memory leaks, but you will also avoid dangling references (ie, using memory after deleting it).

class ClassA : public publicClassA {
public:

private:
    struct ApStruct{
        struct
        {
            float refA[2];
            float refB[2];
            float pVarA;
        } fR;

        struct
        {
            float refA[2];
            float refB[2];
            float pVarA;
        } f1kHz;
    };

    ApStruct GetApStruct;  
}

And yes, in your case it is as simple as removing the pointers. Otherwise, if you want dynamic arrays (ie, arrays whose length is unknown at compile-time) use std::vector.

Answer 2

From Wikipedia Resource Acquisition Is Initialization

Resource Acquisition Is Initialization (RAII) is a programming idiom used prominently in C++. In RAII, resource acquisition is done during object creation, by the constructor, while resource release is done during object destruction, by the destructor. If objects are destroyed properly, resource leaks do not occur.

So you can new the memory used for pointers in constructor and release them in destructor:

ClassA::ClassA(void) {
    GetApStruct = new ApStruct;
    GetApStruct->fR.refA[0] = new float{ 1.f };
    GetApStruct->fR.refA[1] = new float{ 2.f };  
}

ClassA::~ClassA(void) {
    delete []GetApStruct->fR.refA;  
    delete GetApStruct;
}

Answer 3

Well when you create a structure/class object, it holds the variables and pointers in that object memory area( say an object occupies some space in memory. Let's call it a box). Those pointer variables when initialized with new() or malloc(), are given space outside of that box in which the object's data resides. Those pointers now point to some memory area that is outside of that object's memory area. Now when the object is destructed, that space occupied by object (as we called it the box) is destroyed accompanying the pointer variables. The memory area pointed by the pointers is still in there in program/process memory area. Now we have no clue what's it address or where it lies. That's called memory leak. To avoid this situation, we need to de-allocate the memory referenced by pointers using delete keyword. We're free to go now. I tried to illustrate it with a simple graphic below. ObjectA box illustrates the area occupied by it in the memory. Note that this container/box holds the local varialbes including pointer. The pointer points to some memory location, say 0xFFF... and is illustrated by green line. When we destroy ObjectA, It simply destroys everything in it including 0xFFF address. But the memory located on 0xFFF is still allocated in the memory. A memory leak.

In your destructor, de-allocate memory explicitly using delete keyword. Whoa! We saved the memory.

Answer 4

I would recommend something like the following for allocation and cleanup...

#include <iostream>

using namespace std;

class ClassA 
{
public:
    ClassA(void);
    virtual ~ClassA();

private:
    struct ApStruct {
        struct
        {
            float *refA[2];
            float *refB[2];
            float *pVarA;
        } fR;

        struct
        {
            float *refA[2];
            float *refB[2];
            float *pVarA;
        } f1kHz;
    };
    ApStruct* GetApStruct;
};

ClassA::ClassA(void) {
    GetApStruct = new ApStruct{};
    GetApStruct->fR.refA[0] = new float{ 1.f };
    GetApStruct->fR.refA[1] = new float{ 2.f };
    GetApStruct->fR.refB[0] = new float{ 3.f };
    GetApStruct->fR.refB[1] = new float{ 4.f };
    GetApStruct->fR.pVarA = new float { 0.f };
    // do same for struct f1kHz
    // ...
    cout << "Construction" << endl;
}

ClassA::~ClassA()
{
    if (GetApStruct != nullptr) {
        if (GetApStruct->fR.refA[0] != nullptr) {
            delete GetApStruct->fR.refA[0];
            GetApStruct->fR.refA[0] = nullptr;
        }
        if (GetApStruct->fR.refA[1] != nullptr) {
            delete GetApStruct->fR.refA[1];
            GetApStruct->fR.refA[1] = nullptr;
        }
        if (GetApStruct->fR.refB[0] != nullptr) {
            delete GetApStruct->fR.refB[0];
            GetApStruct->fR.refB[0] = nullptr;
        }
        if (GetApStruct->fR.refB[1] != nullptr) {
            delete GetApStruct->fR.refB[1];
            GetApStruct->fR.refB[1] = nullptr;
        }
        if (GetApStruct->fR.pVarA != nullptr) {
            delete GetApStruct->fR.pVarA;
            GetApStruct->fR.pVarA = nullptr;
        }
        // do same for struct f1kHz
        // ...
        // finally
        delete GetApStruct;
        GetApStruct = nullptr;
    }
    cout << "Destruction" << endl;
}

int main() {
    {
        ClassA a;
    }

    system("pause");
    return 0;
}