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 PolygonNodeEdited: added the part of the code where link_ is allocated, setLink is a simple setter method.
Best Answer
I can't see the constructor for the
PolygonNode
class. Is thelink_
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, thelink_
pointers in thePolygonNode
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
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 whenply
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.