Pointer to array deletion

Question

Hi, I have an array that is passed via a function and assigned to a class member which is a pointer.

class ClassA{

unsigned char * pointerToArray;

ClassA(unsigned char array[]){
    pointerToArray = array; 
}

~ClassA(){
    //what to do here   
}

};

Now i have this second class which uses this

class ClassB {
std::list<ClassA> classAcollection;

void firstFunction(){
    for(int i = 0; i < 5; i++){
        unsigned char buffer[3000];
        somePopulationFunction(&buffer);
        classAcollection.push_back(ClassA(buffer);
    }
}

void secondFuncion(){
    std::list<ClassA>::iterator it;

    for(it = classAcollection.begin(); it != classAcollection.end(); it++)
    {
        classAcollection.erase(it);
    }      
}                              

};

My question is how do i make sure the pointerToArray is deleted each time classAcollection.erase(it) is called.

I cant add a delete call in the ClassA destructor as i get the error: pointer being freed was not allocated

Answer 1

Change

unsigned char buffer[3000];

to

unsigned char* buffer = new unsigned char[3000];

otherwise it's allocated on the stack and goes out of scope which means your pointer points to invalid memory which in turn can lead to very very weird things.

Add

delete [] pointerToArray;

in the destructor to make sure the memory is cleaned up. And read up on the difference between operator "delete" and "delete []"

Answer 2

   void firstFunction(){
        for(int i = 0; i < 5; i++){
            unsigned char * buffer = new unsigned char[3000];
            somePopulationFunction(&buffer);
            classAcollection.push_back(ClassA(buffer);
        }
    }

and

~ClassA(){
    delete [] pointerToArray;   
}

Answer 3

Your first sentence already shows all the the confusion:

Hi, I have an array that is passed via a function and assigned to a class member which is a pointer.

You cannot "pass an array". You can pass pointers or references to arrays or array elements to functions. The type of the parameter array in the constructor for ClassA is actually a pointer type. These are the C rules that already confuse enough people as it is. Be sure to check out good array/pointer tutorials.

This is what really happens in your code:

• You create an array with automatic storage duration inside the first function,
• You fill it with data.
• You pass a pointer to the first element of the array to the constructor ClassA::ClassA
• The iteration is done, the array ceases to exist and steps 1-3 are repeated 4 more times

The problem: All ClassA objects have a pointer member that is invalid because all the arrays they referred to are gone now.

If you want these arrays to outlive their scope, this calls for dynamic allocation. You can allocate the array dynamically and "pass over ownership" to a ClassA object. And by "ownership" we usually mean the responsibility to manage and delete the array. It's possible and the other answers show you how to approach this. But they are actually incomplete. You see, if you want a class object to manage a resource like memory you have to do more than just providing a destructor. You have to take care of copying as well. That means a copy constructor and an assignment operator. Since you're not only creating an object of class ClassA but using it as argument for a list's push_back function, the list will make a copy of it and store/manage this copy. So, in case you still want to make ClassA manage the buffer properly by storing a simple (dumb) pointer, you have to write copy ctor, assignment operator and destructor because they would otherwise be compiler-generated. And the compiler-generated ones simply copy member-wise. I guess you don't want two distinct ClassA objects to share the same buffer, right?

Instead of handling raw pointers you could just use a vector as a ClassA member:

class ClassA
{
    std::vector<unsigned char> buffer;
public:
    ClassA(unsigned char const* begin, unsigned char const* end)
    : buffer(begin,end)
    {}
};

:
:

void firstFunction()
{
    for(int i = 0; i < 5; i++) {
        unsigned char buffer[3000];
        somePopulationFunction(&buffer);
        classAcollection.push_back(ClassA(buffer+0,buffer+3000));
    }
}

This works because std::vector doesn't behave like a pointer. If you copy a vector, the copy will manage its own elements. There's no sharing. In that respect, a vector works more like a regular int or double. It is usually implemented im terms of pointers. But it feels like a "regular value type".

There's not a lot else we can do here. ClassA has to be copyable if you want to put ClassA objects into a list. So, it's hard to avoid any copying and at the same time avoid sharing of buffers between ClassA objects.

In C++0x (codename for the upcoming C++ standard) you will be able to do this:

class ClassA
{
    std::vector<unsigned char> buffer;
public:
    ClassA(unsigned char const* begin, unsigned char const* end)
    : buffer(begin,end)
    {}
    ClassA(ClassA const&) = default;
    ClassA(ClassA &&) = default;
    ClassA& operator=(ClassA const&) = default;
    ClassA& operator=(ClassA &&) = default;
};

It'll create a class that is copyable and "movable". A move-enabled class can usually be passed from and to functions very efficiently. In your example, the ClassA object is a temporary object. As such, it could be quickly moved into the list without any unnecessary copying of the buffer objects. But this is just something to tease you. C++0x is not yet official.