Member Avatar for skatamatic

I'm not too sure why I'm getting memory leaks...hopefully someone here is wiser than me, and can help me find it :P .

#include "Canvas.h"

CDrawer CCanvas::gCanvas(RGB(0,0,0), 1);
int const GREEN(20000),BLUE(50), RED(200), THICK(3);

CCanvas::CCanvas(void): _iSize(0), _cptPoints(0)
{
}

CCanvas::CCanvas(CCanvas const & tmpCanvas): _iSize(tmpCanvas._iSize)
{
	if (_iSize > 0)
	{
		_cptPoints = new CPt [_iSize];
		for (int i(0); i < _iSize; ++i)
		{
			_cptPoints[i]._iX = tmpCanvas._cptPoints[i]._iX;
			_cptPoints[i]._iY = tmpCanvas._cptPoints[i]._iY;
			_cptPoints[i]._COLOR = GREEN;
		}
	}
}

CCanvas CCanvas::operator =(const CCanvas & RHS)
{
	if (RHS._iSize > 0)
	{
		_iSize = RHS._iSize;
		_cptPoints = new CPt [RHS._iSize];
		for (int i(0); i < _iSize; i++)
		{
			_cptPoints[i]._iX = RHS._cptPoints[i]._iX;
			_cptPoints[i]._iY = RHS._cptPoints[i]._iY;
			_cptPoints[i]._COLOR = RED;
		}
	}
	return *this;
}

CCanvas & CCanvas::Show(void)
{
	if (_iSize > 1)
	{
		gCanvas.Clear();
		for (int i(0); i < _iSize - 1; ++i)
		{
			for (int ii(i + 1); ii < _iSize; ++ii)
				if (i != ii)
					ShowPts(gCanvas, _cptPoints[i], _cptPoints[ii]);
		}
		gCanvas.Render();
	}
	return *this;
}

CCanvas::~CCanvas(void)
{
	Clean();
}

CCanvas & CCanvas::AddLines(int iX, int iY, COLORREF tmpCol)
{
	CPt * Tmp = new CPt [_iSize + 1];
	for (int i(0); i < _iSize; ++i)
	{
		Tmp[i]._iX = _cptPoints[i]._iX;
		Tmp[i]._iY = _cptPoints[i]._iY;
		Tmp[i]._COLOR = _cptPoints[i]._COLOR;
	}
	Clean();
	_cptPoints = new CPt [++_iSize];
	_cptPoints = Tmp;
	Tmp = 0;
	_cptPoints[_iSize-1]._iX = iX;
	_cptPoints[_iSize-1]._iY = iY;
	_cptPoints[_iSize-1]._COLOR = tmpCol;
	Show();
	return *this;
}

void CCanvas::Clean(void)
{
	delete [] _cptPoints;
	_cptPoints = 0;
}

this is the class where it's occurring. If anyone needs the other classes/files just ask. Thanks!

  • 3 Contributors
  • 4 Replies
  • 332 Views
  • 7 Hours Discussion Span
  • Latest Post Latest Post by ivailosp

Dani AI

Generated

Quick summary: the leak you hit was the classic “allocate then overwrite the pointer” problem that flagged and suggested removing — confirmed that fixed the immediate leak. That solves the symptom, but a couple of other class-level issues are worth fixing so the bug doesn’t reappear in another form.

Pointers to harden next:

  • The copy constructor should always initialize the pointer member when the size is zero; otherwise a subsequent destructor or Clean() may delete an indeterminate pointer. Initialize the pointer to 0/nullptr in the initializer list for the copy ctor.
  • The assignment operator as written returns by value and allocates without freeing existing storage or guarding self-assignment. Change it to return a reference and make it exception-safe (copy-and-swap or free-then-allocate with a self-assignment check).
  • Prefer RAII over manual new[]/delete[]. Replacing the raw array with std::vector<CPt> or a smart pointer removes most of these pitfalls (automatic copy/assignment, no manual delete, exception safety).

Safer alternatives (illustrative):

// simplest: let the container manage storage
class CCanvas {
  std::vector<CPt> points;
public:
  void AddLines(int x,int y,COLORREF c) {
    points.push_back(CPt{x,y,c});
    Show();
  }
  // default copy/assign/destructor are correct
};

If you must keep raw arrays, implement operator= with copy-and-swap so assignments are safe:

CCanvas& operator=(CCanvas other) { // pass-by-value makes a copy
  swap(*this, other);
  return *this;
}

Quick checklist for verification: run a memory checker (Valgrind or AddressSanitizer), enable compiler warnings, test copy/assign with empty and non-empty objects, and add unit tests that exercise exception paths. These steps will catch both the immediate leak and subtler undefined-behavior bugs.

Recommended Answers

All 4 Replies

Member Avatar for stilllearning

These lines here

_cptPoints = new CPt [++_iSize];
  _cptPoints = Tmp;

You allocate space for _cptPoints, and then you assign it to Tmp, which means you've now lost the memory pointer assigned by new CPt [++_iSize]; .. therefore not sure how you can delete it. That may be causing one of your leaks.

commented: Looks like a pretty good candidate for a leak to me, if only all leaks were so easy to spot. +22
Member Avatar for skatamatic

But just before that is the Clean() function, which should delete the old CPt, should it not? :(

Member Avatar for skatamatic

Never mind...I'm an idiot...it's fixed now! Thanks alot!

Member Avatar for ivailosp

remove this line

_cptPoints = new CPt [++_iSize];
Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.