error: pointer being freed was not allocated

Question

I am trying to overload the assignment operator to do a deep copy of a polygon object, the program compiles but I am getting an error toward the end that I want to clear up. Below is the relevant code, if you think I need to add more please just post a comment. Assume the proper #include's and that the << operator is overloaded for proper output etc...

The error is: malloc: * error for object 0x1001c0: pointer being freed was not allocated * set a breakpoint in malloc_error_break to debug.

//Polygon.h
// contains two classes PolygonNode and Polygon
class PolygonNode //Used to link points in a polygon so that they can be iterated through in order
{
public:
...
methods etc
...
private:
Point pt_; // the points in the polygon are made using the Point class
PolygonNode* link_ ; // pointer to the next point in the polygon
};

class Polygon // Connects points and forms a polygon
{
public:
...
Polygon& operator= (Polygon ply);
void Polygon::addPoint(const Point &p);
// methods etc
...
private:
int numPoints_;
bool closed_polygon_;
PolygonNode* first_ ; // points to the first point of the polygon
PolygonNode* last_ ; // points to the last point of the polygon
};

//Polygon.cpp
...
PolygonNode::~PolygonNode()
{
    delete link_ ; // possible problem area

}

Polygon::~Polygon()
{
    delete first_ ; // possible problem area
    last_ = NULL ;
}

void Polygon::addPoint(const Point &p)
{
    PolygonNode* ptr ;
    ptr = new PolygonNode(p) ;
    if( last_ != NULL )
        last_->setLink(ptr) ;
    last_ = ptr ;
    if( first_ == NULL )
        first_ = last_ ;
    numPoints_++ ;
}
Polygon& Polygon::operator= (const Polygon ply)
{
    for (int i = 0; i < ply.numPoints()-1; i++)
    {
        addPoint(ply.getPoint(i));
    }
    if (ply.isClosed())
    {
        closePolygon();
    }
    else
    {
        addPoint(ply.getPoint(ply.numPoints()-1));
    }
    return this;
}
        void Polygon::addPoint(const Point &p)
        {
            PolygonNode ptr ;
            ptr = new PolygonNode(p) ;
            if( last_ != NULL )
                last_->setLink(ptr) ; // sets the last pointer to the new last point
            last_ = ptr ;
            if( first_ == NULL )
                first_ = last_ ;
            numPoints_++ ;
        }
...

//main.cpp
Polygon ply;
...
        Point pt0(0,0);
        Point pt1(1,1);

    ply.addPoint(pt0);

    cout << "ply = " << ply << endl;
    Polygon newply;

    newply = ply; // use of the assignment operator

    cout << "Polygon newply = ply;" << endl;
    cout << "newply = " << newply << endl;
    cout << "ply = " << ply << endl;

    newply.addPoint(pt1);
    cout << "newply.addPoint(Point(0,0)); " << endl;

    cout << "newply = " << newply << endl;
    cout << "ply = " << ply << endl;


...

I have read elsewhere that this is possibly due to a bug in OS 10.6 or Xcode 3.2 if there is a workaround could someone please give me detailed instructions for how to do the workaround, I do not have a lot of experience with Xcode.

Edited: added parts of code that use delete, notice that it is being used in the destructors for Polygon and PolygonNode

Edited: added the part of the code where link_ is allocated, setLink is a simple setter method.

Answer 1

I can't see the constructor for the PolygonNode class. Is the link_ pointer initialized to null on creation? If not, that may be the problem manifesting itself in the error you get. You have to make sure, the link_ pointers in the PolygonNode instances get initialized to null. Define appropriate constructors.

Do you have a copy constructor defined for your polygon class? I can't see one in the code posted, but maybe you just didn't paste it and you have one. If not, that is one of possible sources of serious problems.

The copy constructor, that gets synthesized automatically by the compiler will just copy the pointers in the Polygon class.

Your assignment operator takes the argument by value

Polygon& operator= (Polygon ply);

This makes use of the copy constructor. If it's the automatically synthesized one, ply inside the operator has pointers pointing to the same list, the argument passed by value to the operator owns. ply behaves like it owned the list too and the list gets destroyed when ply goes out of scope. The original argument is left with dangling pointers.

You should define correct copy constructor.

You should also consider taking the argument in the assignment operator by const reference. I don't see a reason to take it by value. Maybe you have one, but even if you do, you can change it temporarily, to test the operator, before you define correct copy constructor. In your operator you should check for self-assignment. All I can see now is adding new nodes to the old Polygon. I don't think it's right, but I guess it's just for testing now.

Answer 2

I think the problem is the link_ variable, it's not allocated in your example and never used...

Answer 3

You should never, ever use raw pointers unless in a dedicated class, normally. Change them to a smart pointer (auto or shared will do in this case) and stop having to free your own memory -> problem solved. Edit:

The smarter option is just to use a std::list or std::vector.