Heap Corruption

[plurple]

This is the .cpp file for a class, have a Heap Corruption problem and are sure the problem lies here. I've initialized all pointers to NULL, but are still unable to find the problem. Think the problem probably lies in the operator+ methodAny help, please?

#include "GenString.h"
#include <ctype.h>
#include <string>
using namespace std;

//Initialize static data members
int GenString::count = 0;

GenString::GenString()
{
	string = new char[1];
	string[0] = 0;
	count++;
}

GenString::GenString(char *string_value)				
{
	this->setString(string_value);
}
GenString::GenString(const GenString &string_value)	
{
	this->setString(string_value.string);
}
GenString::~GenString()							
{
	delete[] string;
}
void GenString::setString(char* string_value)	
{
	int length = strlen(string_value) + 1;
	string = new char[length];
	memset(string, NULL, length);
	strcpy_s(string, length, string_value);
}
char* GenString::getString()							
{
	int length = strlen(string) + 1;
	char* ret = new char[length];
	memset(ret, NULL, length);
	strcpy_s(ret, length, string);
	return ret;
}
void GenString::toLower()								
{
	int length = this->getLength();
	int i = 0;
	for(i = 0; i < length;  i++)
	{
		string[i] = tolower(string[i]);
	}
}
void GenString::toUpper()	
{
	int length = this->getLength();
	for(int i = 0; i < length; i++)
	{
		string[i] = toupper(string[i]);
	}
}
char GenString::getChar( int index)
{
	
	int length = this->getLength();
	if(index < 0 || index >= length)
	{
		cout << "ERROR: Index out of bounds" << endl;
		return NULL;
	}
	return (string[index]);
}
int GenString::getLength()
{
	int length = 0;
	length = strlen(string);
	return length;
}
bool GenString::isEqual(GenString another_string)
{
	if(strcmp(this->string, another_string.string) == 0)
		return true;
	else 
		return false;
}
bool GenString::operator==( GenString right_side)
{
	//Compare right string to this string
	if(strcmp(this->string, right_side.string) == 0)
		return true;
	else 
		return false;
}
bool GenString::operator>(GenString right_side)
{
	if(strcmp(this->string, right_side.string) < 0)
		return true;
	else
		return false;
}
bool GenString::operator<(GenString right_side)
{
	if(strcmp(this->string, right_side.string) > 0)
		return true;
	else
		return false;
}
//*******************************************
//Returns true if the GenString on the right side of the >= operator is greater than or 
//equal to the GenString on the left side (this GenString). 
//*******************************************
bool GenString::operator>=(GenString right_side)
{
	if(strcmp(this->string, right_side.string) == 0 || strcmp(this->string, right_side.string) < 0)
		return true;
	else
		return false;
}
//*******************************************
//Returns true if the GenString on the right side of the <= operator is less than or 
//equal to the GenString on the left side (this GenString). This is based on the results of strcmp. 
//*******************************************
bool GenString::operator<=(GenString right_side)
{
	if(strcmp(this->string, right_side.string) == 0 || strcmp(this->string, right_side.string) > 0)
		return true;
	else
		return false;
}
//*******************************************
//Concatenates the GenString on the right side of the + operator to this GenString 
//(which is on left side of the + operator) and stores the concatenated GenString in this GenString.  
//*******************************************
void GenString::operator+=(GenString right_side)
{
	int length = right_side.getLength() + this->getLength() + 1;
	strcat_s(this->string, length, right_side.string);	
}
//*******************************************
//Creates a GenString which is the concatenation of the 
//GenString on the right side of the + operator to this 
//GenString (left operand), without changing this GenString, and returns the concatenated GenString  
//*******************************************
GenString GenString::operator+(GenString& right_side)
{
	//creating new object
	GenString result;
	int newSize = this->getLength() + right_side.getLength() + 1;
	char* temp = new char[newSize];
	memset(temp, NULL,newSize);
	strcpy_s(temp, this->getLength() + 1, this->string);
	strcat_s(temp, newSize, right_side.string);
	result.string = temp;
	return result;
}
//*************************************************
// Assigns the GenString on the right side of an assignment statement 
//to the GenString on the left side (to this GenString).
//*************************************************
void GenString::operator=(GenString right_side)
{
	int length = right_side.getLength() + 1;
	this->string = new char[length];
	memset(string, NULL, length);
	strcpy_s(this->string, length, right_side.string);
}
//************************************
//Returns the character at the specified index.  
//If the index is < 0 or > the string length, prints an error message 
//and exits the program.  (Return reference for left side)
//*********************************************
char& GenString::operator[ ](int index)
{
	if(index < 0 || index > this->getLength())
	{
		cout << "Error: Invalid index" << endl;
		exit(0);
	}
	return(string[index]);
}
//***********************************
// Prepends a space to the private data field string using the prefix increment
//***********************************
GenString GenString::operator++()		
{
	int length = strlen(string) + 1;	
	char* temp = new char[length];
	memset(temp, NULL, length);
	strcpy_s(temp, length, string);
	char space[2] = " ";
	++length;
	string = new char[length];
	memset(string,NULL, length);
	string[0] = space[0];
	for(int i = 1; i < length; i++)
		string[i] = temp [i-1];
	delete[] temp;
	return(*this);
}
//***********************************
// Removes a space (if possible) from the first character of the  private data field 
// using the prefix decrement
//***********************************
GenString GenString::operator--()		
{
	int length = strlen(string);
	if(string[0] == ' ')
	{
		for(int i = 0; i < length; i++)
		{
			string[i] = string[i+1];
		}
		return (*this);
	}
	else
		return(*this);
}
//***********************************
// Appends a space to the private data field string using the postfix increment (if possible)
//***********************************
GenString GenString::operator++(int dummy)			
{
	
	//Appending space if possible
	int length = this->getLength() + 1;
	char* string2 = " ";
	if(string[length-1] == ' ')
	{
		return(*this);
	}
	else
	{
		string = strcat(string, string2);
		return(*this);
	}
}
//***************************************
// Removes a space from the end of the private 
//data field string using the postfix decrement (if possible)
//***************************************
GenString GenString::operator--(int dummy)
{
	int length = this->getLength();
	if(string[length-1] == ' ')
	{
		string[length-1] = NULL;
		return (*this);
	}
	else
	{
		cout << "Unable to remove space from the end of the private data field with postfix "<< endl;
		return (*this);
	}
}
//********************************************************
// Overloaded >> operator. Gives cin the ability to      *
// store user input directly into GenString objects.    *
//********************************************************
istream& operator>>(istream& input, GenString& gstr_obj)
{	
	std::string str;
	getline(input, str);
	char* temp = new char[str.size()+1];
	memset(temp, NULL, str.size()+1);
	for(int i = 0; i<= str.size(); i++)
	{
		temp[i] = str[i];
	}
	gstr_obj.setString(temp);
	delete[] temp;
	return input;			//allows cascading of in operator
}
//********************************************************
// Overloaded << operator. Gives cout the ability to     *
// directly display GenString objects.                  *
//********************************************************
ostream& operator<<(ostream& screen_out, GenString gstr_obj)
{
	screen_out << gstr_obj.getString();
	return screen_out;			//allows cascading of << operator
}

int GenString::getCount()
{
	return(count);
}

[helios]

Why this weird combination of std::string, direct string handling, and C functions? You're probably screwing something up somewhere and corrupting memory. Just move all your code to std::string and be done with it.

[imi]

+1 what helios said. If you don't really have a GOOD reason to implement yet another string class, don't do it.

Your code has several severe problems, among them: several places where you leak memory (operator= for example), strange logic in some of your operators (++s always add spaces, s++ only if there is no space at the end), bad performance (lots of temporary and unnecessary string copies, use of strcat instead of strcpy)....

Your original problem may lie in the postfix operator++, where you add a space without having enough memory for it, thus writing a null-termination on unallocated heap space.

But really. Save you the pain and just use std::string. Even if you fix that one, there are most probably a lot of other subtile errors. Good string classes are scare and very hard to write and std::string is usually one of the best implementations.

Ciao, Imi.

[kbw]

void GenString::setString(char* string_value)	
{
	int length = strlen(string_value) + 1;
	string = new char[length];
	memset(string, NULL, length);
	strcpy_s(string, length, string_value);
}

This doesn't release the old string, it's a memory leak.

GenString::GenString(const GenString &string_value)	
{
	this->setString(string_value.string);
}

This doesn't initialise string, so setString will corrupt the heap (when fixed).

char* GenString::getString()							
{
	int length = strlen(string) + 1;
	char* ret = new char[length];
	memset(ret, NULL, length);
	strcpy_s(ret, length, string);
	return ret;
}

This assumes the caller will delete the returned char array, possible memory leak.

bool GenString::isEqual(GenString another_string)
All these functions take a GenString by value. Are you sure you want to do this?

void GenString::operator+=(GenString right_side)
{
	int length = right_side.getLength() + this->getLength() + 1;
	strcat_s(this->string, length, right_side.string);	
}

This doesn't grow the buffer, it writes off the end, corrupting the heap.

void GenString::operator=(GenString right_side)
{
	int length = right_side.getLength() + 1;
	this->string = new char[length];
	memset(string, NULL, length);
	strcpy_s(this->string, length, right_side.string);
}

This doesn't release the old string, it's a memory leak.

... and so on.

[plurple]

thanks a lot, everybody. I am yet a beginner in this and are still learning. will make the proper changes.