Using copy constructor and operator overload - 'pointer being freed was not allocated'

Question

I've created a class called MyInteger, and I'm trying to create a copy constructor that will correctly make a separate copy of the memory pInteger points to, and make pInteger in the new object point to it. I need to overload the = operator so that each of the two objects involved has its own separate copy of the memory that its own pInteger points to.

MyInteger specification:

class MyInteger {
public:
    MyInteger(int);
    MyInteger(MyInteger &);
    ~MyInteger();
    int getMyInt();
    void setMyInt(int);
    MyInteger operator=(MyInteger&);

private:
    int * pInteger;

};

MyInteger implementation:

MyInteger::MyInteger(int i) {
    pInteger = new int;
    *pInteger = i;
}


MyInteger::MyInteger(MyInteger &obj) {
    pInteger = new int;
    pInteger = obj.pInteger;
}

MyInteger MyInteger::operator=(MyInteger &i) {
    pInteger = new int;
    pInteger = i.pInteger;

    return i;
}


MyInteger::~MyInteger() {
    delete pInteger;
}

int MyInteger::getMyInt() {
    return *pInteger;
}

void MyInteger::setMyInt(int i) {
    pInteger = new int;
    *pInteger = i;
}

I'm pretty sure the problem is at least partially due to the fact that I'm reassigning pInteger in the constructor and operator overload: pInteger = new int; and then pInteger = obj.pInteger;. I think the second assignment is nullifying the first assignment using new, so I'm getting

pointer being freed was not allocated

because pInteger was not correctly being dynamically allocated. Am I going in the right direction here? I'm not sure how to fix this, any help would be great.

Your copy-constructo and assignment-operator are both wrong. Look more closely, you write `pInteger =` and then `pInteger =` again on the second line - that will overwrite what you did on the first line. You did it right in the `int` constructor, so look at that and then apply the same idea to your other functions. The `setMyInt` function and the assignment operator leaks memory, you should only call `new` in the constructors. Also , the assignment operator should return `*this` and have a reference as the return type. — M.M, Feb 16 '16 at 23:13

David Schwartz · Accepted Answer · 2016-02-16T23:16:13.623

2

MyInteger::MyInteger(MyInteger &obj) {
    pInteger = new int;
    pInteger = obj.pInteger;
}

MyInteger MyInteger::operator=(MyInteger &i) {
    pInteger = new int;
    pInteger = i.pInteger;

    return i;
}

You assign pInteger the correct value -- pointing to a newly-allocated integer. Then you immediately overwrite it with the wrong value -- a pointer to an integer owned by another object.

Also, your operator= returns a copy. You'd expect (j = i)++; to modify j, not a temporary.

edited Feb 16 '16 at 23:16

answered Feb 16 '16 at 23:14

David Schwartz

179,497
17
214
278

I realize the second line is wrong, but I'm not sure what to do to make `pInteger` in the new object point to a copy of the memory the original `pInteger` points to? – 123 Feb 16 '16 at 23:24
1

@123 `*pInteger = *i.pInteger;` – user4581301 Feb 16 '16 at 23:25
1

Or just replace both lines with `pInteger = new int (i.getMyInt());` – David Schwartz Feb 16 '16 at 23:29

Using copy constructor and operator overload - 'pointer being freed was not allocated'

1 Answers1