0

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 can't add a delete call in the ClassA destructor as I get the error: pointer being freed was not allocated

1
  • You are passing a stack allocated array to your class. That array goes out of scope when the for() block scope ends. You cannot reference it outside of the same iteration of the loop you assign it to the ClassA constructor. Commented Oct 13, 2010 at 11:37

3 Answers 3

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.

Sign up to request clarification or add additional context in comments.

4 Comments

Thanks for the detailed response. So how do i go about creating a function in ClassA to return the vector buffer as a unsigned char array.
@user346443: The C++ standard guarantees that the elements in a vector are stored conescutivly in memory. So, if you want a pointer to the vector object's buffer you can get it via &buffer[0] in case the vector is not empty. But keep in mind that this pointer is only valid as long as the vector exists and you don't invoke any vector member functions that could lead to a reallocation (reserve, resize, push_back, insert, ...)
Three more questions. 1. Will the vector buffer be automatically destroyed or do i have to delete it in the destructor. 2. is &buffer[0] the data in the array or just the first element. 3. So does the constructor you specified copy the contents of the buffer array to ClassA's buffer member. Im actually using this to create a video from an array of images and using the above implementation none of my images are visible.
@user346443: 1. It'll be automatically destroyed. 2. &buffer[0] is a pointer to the first element. But as I said, the other elements directly follow it. 3. Yes, the data will be copied into the vector.
1
   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;   
}

1 Comment

Also, remember that a class with a destructor also needs a copy constructor and copy assignment operator.
0

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 []"

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.