Append suffix to string with pointer arithmetic

[RAWBERRY]

for my CS215 course, I was given a class called String215 which is a basic string class to help in the understanding of dynamic memory allocation and pointer arithmetic with char arrays.

The class was given to me in a very basic skeleton form with prototypes but no implementations, along with a test function to test my implementations. I CAN NOT use any C String functions in this assignment.

The part of the program which is troubling is the append function, which just appends a parameter string215 object to the end of the current string215 object.

// Add a suffix to the end of this string.  Allocates and frees memory.
void string215::append(const string215 &suffix)
{
	char *output = new char[str_len(data)+suffix.length()+1];
	for(int x = 0; x < str_len(data); x++) {
		*output = *data;
		output++;
		data++;
	}

	for(int x = 0; x < suffix.length(); x++) {
		*output = suffix.getchar(x);
		output++;
	}
	*output = '\0';
	output -= (str_len(data)+suffix.length()+1);
	delete[] data;
	data = output;
}

This portion of the code is tested in the 13th test of the test function as shown here:

string215 str("testing");

...

// Test 13: test that append works in a simple case.
curr_test++;
string215 suffix("123");
str.append(suffix);
if (strcmp(str.c_str(), "testing123") != 0) {
	cerr << "Test " << curr_test << " failed." << endl;
	failed++;
}

EDIT: Here is the description of the append class:

Add the suffix to the end of this string. Allocates a new, larger, array; copies the old contents, followed by the suffix, to the new array; then frees the old array and updates the pointer to the new one.

My program aborts at the very end of the append function execution with the error message:

Debug Assertion Failed!

Program: [Source path]\dbgdel.cpp
Line: 52

Expression: _BLOCK_TYPE_IS_VALID(pHead->nBlockUse)

...

Abort || Retry || Ignore

I'm fairly certain it has something to do with my very poor memory management. I know it's not a lot to go on, but I've been struggling with this for hours on end and can't seem to figure it out.

Here's a pastebin of the .cpp and .h file for this program
string215.cpp: http://pastebin.com/Xh2SvDKJ
string215.h: http://pastebin.com/JfAJDEVN

Any help at all is greatly appreciated!

Thanks,
RAW-BERRY

[toum]

I think it's because you incremented the "data" pointer but did not restore it to its original value before trying to delete[] it.

[RAWBERRY]

I thought so too immediately after posting but the insertion of
data -= (str_len(data)+1);
did nothing.

[LB]

Why would it? data is still at the wrong place, why would the str_len function magically give you the right number to put data back where it used to be? ;)

[toum]

Of course it did nothing, you're calling str_len() on the new value of "data".
Besides, the way you're using str_len(data) is wrong : the returned value changes every time you change data.
You should measure the length only once and then use the measured value.

void string215::append(const string215 &suffix)
{
	int StringLength = str_len(data);
        char *output = new char[StringLength +suffix.length()+1];
	for(int x = 0; x < StringLength ; x++) {
		*output = *data;
		output++;
		data++;
	}

	for(int x = 0; x < suffix.length(); x++) {
		*output = suffix.getchar(x);
		output++;
	}
	*output = '\0';
	output -= (StringLength +suffix.length()+1);
        data -= (StringLength + 1);
	delete[] data;
	data = output;
}

It would be less error-prone to store the initial values of output and data into temporary variables.

[RAWBERRY]

Okay, so I've taken all of the input I have received so far and come up with

// Add a suffix to the end of this string.  Allocates and frees memory.
void string215::append(const string215 &suffix)
{
	int dataLength = str_len(data);
	char *output = new char[dataLength+suffix.length()+1];
	for(int x = 0; x < dataLength; x++) {
		*output = *data;
		output++;
		data++;
	}
	for(int y = 0; y < suffix.length(); y++) {
		*output = suffix.getchar(y);
		output++;
	}
	*output = '\0';
	output -= (dataLength+suffix.length());
	data -= dataLength;
	delete[] data;
	data = output;
}

As previously stated, because I was iterating through data, when I would take its (data) length, I would also change the value of str_len(data). Adding another variable for data's original length solved that.

Then I was also not compensating for the iterations through data, and was compensating overcompensating for the iterations through output by 1. The correct iteration compensation:

	output -= (dataLength+suffix.length());
	data -= dataLength;

All of this allowed me to move on to my next error, a heap corruption.

Debug Error!

Program: [Source path]

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

which happens immediately after the execution of my append function.

From what I understand this means that I didn't allocate memory correctly somewhere?

Do I need to reallocate memory for data after I deleted it?

[LB]

How about keeping copies of the original data and output pointers?

[toum]

What happens if you store the initial values of the pointers into temporary variables ?

void string215::append(const string215 &suffix)
{
	int dataLength = str_len(data);
	char *output = new char[dataLength+suffix.length()+1];

        char* InitialDataPtr = data;
        char* InitialOutputPtr = output;

	for(int x = 0; x < dataLength; x++) {
		*output = *data;
		output++;
		data++;
	}
	for(int y = 0; y < suffix.length(); y++) {
		*output = suffix.getchar(y);
		output++;
	}
	*output = '\0';

	delete[] InitialDataPtr;
	data = InitialOutputPtr;
}

[RAWBERRY]

Sorry but I don't really understand how I would use copies of the original pointers to help solve my issue? I have to assign output to the data, data member. I'm just not sure what exactly I would do with the copies.

[LB]

The copies would let you keep the original values of the pointers so that you don't have to do any crazy might-not-work math with the pointers after messing with them.

[RAWBERRY]

Ah, yeah that makes sense. Definitely simplifies things, thanks

[toum]

They're doing the exact same thing as your

output -= (dataLength+suffix.length());
	data -= dataLength;

but without the risk of making a mistake.
For example, it would have avoided your first problem.

[RAWBERRY]

I see, does deleting InitialDataPointer after data has been assigned to it also delete the data pointer?

[LB]

Whatever data points to is no longer your program's memory, so you shouldn't access it.

[toum]

It would delete the memory associated with "data", but it would not change the value of "data".