Content of array being rewritten after return

Question

First things first, I have the below class A which at the same time have a nested class B:

// A.h
class A {
public:
    class B;

    A();
    A(const A& a); // Copy constructor

    B& operator[](const unsigned int& a) const;
    A operator+(const A& a) const;

    /*...*/

    ~A();
private:
    /*...*/
    unsigned int size;
    B* b;
};

and I'm trying to use the overloaded + operator to "add" two A objects by adding the content of both b members of said objects, and assign the result to a third A object.

• b is a dinamically allocated array of B objects.

• B is a pretty basic class with just some unsigned int

here's the main function:

// main.cpp
int main(int argc, char** argv) {
    A a1, a2, result;

    a1.read(argv[1]); // Initialize b member with content read from a file
    a2.read(argv[2]); // Initialize b member with content read from a file

    result = a1 + a2; // Error [3]

    getchar();

    return 0;
}

The problem is that when trying to make the sum I get what I think are memory errors like: HEAP[main.exe]: Invalid address specified to RtlValidateHeap( 00A50000, 00A59938 )

here's the implementation of class A:

// A.cpp
A::A() : /*...*/, size(0), b(nullptr) {}

// Copy constructor
A::A(const A& a) : /*...*/, size(a.size), b(nullptr) {
    b = new B[size]; // [1]

    for (unsigned int i = 0; i < size; i++) {
        (*this)[i] = a[i];
    }
}

A::B& A::operator[](const unsigned int& i) const {
    return b[i];
}

A A::operator+(const A& a) const {
    if (size != a.size) {
        exit(1); // Size must be the same on both operands
    }

    A tmp(*this); // Call to copy constructor

    for (unsigned int i = 0; i < a.size; i++) {
        tmp[i] += a[i];
    }

    return tmp; // Call to copy constructor [2]
}

A::~A() {
    if (b != nullptr) {
        delete[] b;
    }
}

and of class B:

// B.h
class A::B {
public:
    B();
    B(unsigned char, unsigned char, unsigned char);

    B& operator+=(const B& b);
private:
    unsigned int a, b, c, d;
};

// B.cpp
A::B::B() : a(0), b(0), c(0), d(0) {}
A::B::B(unsigned char _a, unsigned char _b, unsigned char _c) {
    /*...*/
}

A::B& A::B::operator+=(const B& b) {
    /*...*/

    return *this;
}

I'm using Visual Studio by the way, and when debugging I've observed:

1. b member of result points to the same address pointed by b in ^[1] when the copy constructor is called by the return statement in ^[2], so far so good

2. Until the return in ^[2] the content of b is all right, something like: 0x00669968 00 00 ff 00 00 ff 00 00 ..ÿ..ÿ..

3. After ^[3] the content of b in ^[1] and therefore the content of b member of result object becomes something like: 0x00669968 dd dd dd dd dd dd dd dd ÝÝÝÝÝÝÝÝ, I'm guessing is garbage

Note: all include directives and irrelevant sections of code have been ommited

I've been like two days shaking my head trying to figure out what's wrong with no luck so any help is greatly appreciated, thanks in advance.

Answer 1

I check your code and the problem is that you need a custom copy assignment for class A. In your main you have A a1, a2, result;, the default constructor is called for the 3 objects. Then, in the line result = a1 + a2;, the default copy assignment is called.

When you have pointers in your class and allocate the memory using new then you have to worry about copy constructor and copy assignment. Check this post and the rule of three.

---

I propose you the next code:

class A {
    class B {
        unsigned a, b, c, d;

    public:
        B() : a(0), b(0), c(0), d(0) { }

        B(unsigned char a_, unsigned char b_, unsigned char c_) : a(a_), b(b_), c(c_), d(0) { }

        // Copy constructor.
        B& operator=(const B& b_) {
            a = b_.a;
            b = b_.b;
            c = b_.c;
            d = b_.d;
            return *this;
        }

        B& operator+=(const B& b_) {
            a += b_.a;
            b += b_.b;
            c += b_.c;
            d += b_.d;
            return *this;
        }
    };

    unsigned size;
    B* b;

public:
    A() : size(0) { }

    // Copy constructor.
    A(const A& a) : size(a.size) {
        b = new B[size];
        for (unsigned i = 0; i < size; ++i) {
            b[i] = a[i];
        }
    }

    // Copy assigment
    A& operator=(const A& a) {
        clear();
        b = new B[size];
        for (unsigned i = 0; i < size; ++i) {
            b[i] = a[i];
        }
        return *this;
    }

    B& operator[](unsigned pos) const {
        if (pos > size) {
            throw std::out_of_range("Out of range");
        }
        return b[pos];
    }

    A operator+(const A& a) const {
        A tmp = *this;
        if (size != a.size) {
            throw std::out_of_range("Diferent sizes");
        }
        for (unsigned i = 0; i < a.size; ++i) {
            tmp[i] += a[i];
        }
        return tmp;
    }

    void read(const char* file) {
        clear();
        size = size_;
        b = new B[size];
        /*
         * Read your file and update b.
         */
    }

    void clear() {
        if (size) {
            delete[] b;
            size = 0;
        }
    }

    ~A() {
        clear();
    }
};