CODEWITHSUNDEEP
c++constructorcopy-constructor
GameTCoder
GameTCoder

Reputation: 3

Reseting object m_object = Object(new, parameters);

I have a simple class:

class Histogram {
    int m_width;
    int m_height;
    int m_sampleSize;
    int m_bufferWidth;
    int m_bufferHeight;

    uint8* m_buffer;
    int m_size;
public:
    Histogram() : m_buffer(0) { }
    Histogram(int width, int height, int sampleSize) {
        m_buffer = new unsigned char [width*height*sampleSize];
    }
    ~Histogram() {
        my_log("destructor: buffer: %p", m_buffer);
        if ( m_buffer ) { delete [] m_buffer; m_buffer = NULL; }
    }
    unsigned char* buffer() {
        return m_buffer;
    }
};

It is a member in other class:

class Other {
    Histogram m_histogram;

    void reset() {
        my_log("reset() called: buffer: %p", m_histogram.buffer());
        m_histogram = Histogram(512, 512, 2);
    }
}

Now, I first create "uninitialized" object using Histogram() constructor – which sets m_buffer to NULL;

Then, I call the reset method, which does m_histogram = Histogram( 512, 512, 3 ) – the new object has m_buffer initialized via new.

So expected sequence of log messages is:

But instead, I get:

So some irrational action is being performed. Moreover, the 0x072a7de address is displayied when I also delete the second object (created with "larger" constructor, with three int parameters).

Upvotes: 0

Views: 73

Answers (2)

juanchopanza
juanchopanza

Reputation: 227370

First of all, since you are pointing to a dynamically allocated array, you need to use operator delete[]

delete[] m_buffer;

Second, and more importantly, since you have dynamically allocated memory, you should follow the rule of three and implement a copy constructor, and assignment operator, as well as fixing the destructor.

What happens now is that your (compiler synthesized) assignment operator is making a "shallow" copy, i.e. it is copying the pointer. Then you will hve multiple destructors trying to delete it. You are invoking undefined behaviour.

You could really save yourself a lot of trouble by using an std::vector<uint8> as a buffer.

Upvotes: 1

ForEveR
ForEveR

Reputation: 55887

You MUST realize copy-ctor and assignment operator for your class Histogram, since 

m_histogram = Histogram(512, 512, 2);

is assignment operator call. Implicit operator = bitwise copy members of your class.

And you must use delete[] in destructor, not delete, since you allocate an array.

Upvotes: 2

Related Questions