heap corruption

[patrickweiss52]

Hey guys,
I have a CS1410 class where I have to write my own vector class and then run it through a driver that has been provided. I'm getting the following error and I don't understand why:

HEAP CORRUPTION DETECTED: after Normal block (#139) at 0x00A14600.
CRT detected that the application wrote to memory after end of heap buffer.

This is my .cpp for my class:

#include "MyVector.h"
using namespace std;



MyVector::MyVector()
{
	sizeArray = 0;
	capacityArray = 2;
	address = new int [capacityArray];
}

MyVector::MyVector(int n)
{
	sizeArray = 0;
	capacityArray = n;
	address = new int[capacityArray];

}

void MyVector::clear(void)
{
	delete[] address;
	sizeArray = 0;
	capacityArray = 2;
	address = new int[capacityArray];
}

void MyVector::push_back(int n)
{
	
	if (sizeArray > capacityArray)
	{
		//Grow
		capacityArray = capacityArray * 2;
		int *temp = new int[capacityArray];
		for (int i = 0; i < sizeArray; i++)
		{
			*temp = *address;
		}

		delete [] address;
		address = temp;
	}
	address[sizeArray] = n;
	sizeArray++;
}


MyVector::~MyVector()
{
	if (address != NULL)
	{
		delete[] address;
		address = NULL;
	}
}

int MyVector::at(int n)const
{
	if (n > capacityArray)
	{
		throw  n;
	}
	else
	{
		return address[n];
	}


}
int MyVector::size() const
{

	return sizeArray;
}

int MyVector::capacity() const
{
	return capacityArray;
}

and this is the driver that I'm not allowed to change:

int main()
{
	// Create a default vector (cap = 2)
	MyVector sam;

	// push some data into sam
	cout << "\nPushing three values into sam";
	sam.push_back(21);
	sam.push_back(31);
	sam.push_back(41);

	cout << "\nThe values in sam are: ";

	// test for out of bounds condition here
	for (int i = 0; i < sam.size() + 1; i++)
	{
		try
		{
			cout << sam.at(i) << " ";
		}
		catch (int badIndex)
		{
			cout << "\nOut of bounds at index " << badIndex << endl;
		}
	}
	cout << "\n--------------\n";

	// clear sam and display its size and capacity
	sam.clear();
	cout << "\nsam has been cleared.";
	cout << "\nSam's size is now " << sam.size();
	cout << "\nSam's capacity is now " << sam.capacity() << endl;
	cout << "---------------\n";

	// Push 12 values into the vector - it should grow
	cout << "\nPush 12 values into sam.";
	for (int i = 0; i < 12; i++)
		sam.push_back(i);

	cout << "\nSam's size is now " << sam.size();
	cout << "\nSam's capcacity is now " << sam.capacity() << endl;
	cout << "---------------\n";

	cout << "\nTest to see if contents are correct...";
	// display the values in the vector
	for (int i = 0; i < sam.size(); i++)
	{

		cout << sam.at(i) << " ";
	}
	cout << "\n--------------\n";

	cout << "\n\nTest Complete...";

	cout << endl;
	system("PAUSE");
	return 0;
}

I've isolated the error to clear() but I don't know how to fix the error. Help?

[PrivateRyan]

in your push_back function, you check to see if size is greater than capacity. Consider what might happen if size is equal to capacity. A dynamic array with 5 spots is indexed with 0, 1, 2, 3, and 4.

What would happen if you tried address[size] = value when size is equal to capacity?

(People are gonna rage when they see that system("PAUSE") )

[Ganado]

First problem I see is this

		for (int i = 0; i < sizeArray; i++)
		{
			*temp = *address;
		}

You probably mean to do temp[i] = address[i] or something?

But that probably isn't causing the segfaults, I'll edit this if I find something else.

Edit: There might be a problem push_back function.
If sizeArray is 2 (after doing 2 push_backs),
and capacityArray is 2,
the if statement if (sizeArray > capacityArray)
will not pass, but it will still try to write to address[2], which is out of bounds.

Edit: Yep that seems to be the problem (and PrivateRyan answered before me anyway)

[patrickweiss52]

Thank you Ganado,
I fixed it to be temp[i]=address[i]. That fixed an issue I had with the last part of the driver.
Although I still have the aforementioned error popping up.

@PrivateRyan: I don't quite understand what you mean. In push_back I check to see if size(the current element) is past the capacity (size of the array) with that if statement in line 32. If it is I then make a new array and transfer the elements. If address[size] is equal to capacity isn't it just going to place the value in the last element of the array?

Why will people rage at the system("PAUSE")? I have no say on it... it's what the professor provided.

[Ganado]

Don't worry about the system pause for now.

The problem is that, let's say, the size of your current array is 2. You then try to write to array[2], but array indices go from [0, size-1] inclusive, so array[2] is out of bounds.

When sizeArray is equal to capacityArray, you don't want to write to array[sizeArray] because that's out of bounds. Basically your if-statement just needs a little change, and then it should work correctly as far as I can see.

[patrickweiss52]

@Ganado: The offset of -1! I understand now what you all were referring to. Thank you!
This offset the results with the driver so I had to go in and fix at() as well.
Everything working, new cod as follows:

void MyVector::push_back(int n)
{
	
	if (sizeArray >= capacityArray)
	{
		//Grow
		capacityArray = capacityArray * 2;
		int *temp = new int[capacityArray];
		for (int i = 0; i < sizeArray; i++)
		{
			temp[i] = address[i];
		}

		delete [] address;
		
		address = temp;
	}
	address[sizeArray] = n;
	sizeArray++;

and

int MyVector::at(int n)const
{
	if (n > sizeArray-1)
	{
		throw  n;
	}
	else
	{
		return address[n];
	}

Thank you for the quick help PrivateRyan and Ganado!