HEAP CORRUPTION (#209)

[yukyduky]

#include "stdafx.h"
#include <iostream>

using namespace System;
using namespace std;

class KeepRunning
{
public:
	~KeepRunning()
	{
		system("PAUSE");
	}
};

int* MultTab (int Tal1, int Tal2)
{
	int h, l;
	int *Sum1 = new int[Tal2 - Tal1];

	l = 1;
	h = 0;

	for (int i = Tal1; i <= Tal2; i++)
	{
		while (h != 11)
		{
			h = h++;

			if (i % h == 0)
			{
				l = l++;
	
				Sum1[l] = Tal1;

				l = l++;

				Sum1[l] = h;
			}
		}
		
		h = 0;
	}
	delete[] Sum1;
	return Sum1;
};

int main(array<System::String ^> ^args)
{
	KeepRunning kr;

	int Tal1, Tal2; 

	cout << "Input a low and a high value" << endl;
	cin >> Tal1;
	cin >> Tal2;

	int *Sum1 = new int[Tal2 - Tal1];

	Sum1 = MultTab(Tal1, Tal2);

	for (int i = 1; i <= (Tal2 - Tal1); i++)
	{
		cout << Sum1[i] << endl;
	}

	return 0;
}

Error that I'm getting:

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

I'm using VS++ 2010.
Any ideas on what I'm doing wrong and how to solve it?
Other improvements are welcome as well.

[Disch]

Quite a few problems.


int *Sum1 = new int[Tal2 - Tal1];

If you make an array of size X, valid indexes are [0..X). Which means X is not a valid index.

For example:

int myarray[10];  //  a 10 element array

myarray[0] = 0;  // OK, accesses first element
myarray[9] = 0;  // OK, accesses last element
myarray[10] = 0;  // BAD!! memory corruption!!! 

You are doing this in a few places:

for (int i = 1; i <= (Tal2 - Tal1); i++)

Here, your loop condition is i <= (Tal2 - Tal1), which is incorrect, because if i == Tal2 - Tal1, then you are out of bounds in your array.

The proper way to construct this loop would be like this:

for (int i = 0; i < (Tal2 - Tal1); i++)

Remember: start at zero and stop as soon as you get to the array size.


Similar problem in your MultiTab function:

	l = 1;  // don't start at 1!  Start at zero!

	//...

	Sum1[l] = Tal1; // <- that will cause this to go out of bounds! 

Another problem:

	delete[] Sum1;
	return Sum1;

If you delete[] Sum1, then the array no longer exists. So what exactly are you returning? You're returning a bad pointer.

Also:

int *Sum1 = new int[Tal2 - Tal1];

	Sum1 = MultTab(Tal1, Tal2);

Here you have Sum1 point to a new array, but then you discard that and have it point to whatever MultTab gave you, which effectively just causes a memory leak because you are no longer referencing the array you new'd.

Trying to pass ownership of dynamic memory between functions is a bad idea. The best way to do this would be to allocate the array in main and pass it by pointer to MultTab:

// note:  MultTab returns a void, and has Sum1 passed to it as a parameter
void MultTab (int* Sum1, int Tal1, int Tal2)
{
  // don't allocate Sum1 here, just use it as passed
}

int main()
{
  int* Sum1 = new int [whatever]; // allocate it in main

  MultTab(Sum1,a,b);  // pass it to MultTab
               // this will change 'Sum1' as you want.

  // print Sum1 here

  delete[] Sum1;  // then delete[] it when you're done
}

Lastly, you seem to be coding in normal C++, but are using the C++/CLI entry point. If you want to use normal C++, then use normal C++. The whole System:: thing and String^ thing is a C++/CLI thing that you shouldn't do when writing C++.

C++ and C++/CLI are two entirely differnet langauges. Pick one and stick with it. Don't try to mesh the two.

[yukyduky]

Thanks a whole lot! This cleared up so much of what I thought I knew and what I was trying to get to know! Much appreciated!

EDIT: Why I was trying to make the array [X] was because I didn't know the exact size of it.