Heap Corruption Detected: Manipulating strings with pointers

[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)
{
	int dataLength = str_len(data);
	char *output = new char[dataLength+suffix.length()+1];
	char *outTemp = output;
	char *dataTemp = data;

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

Execution of this function causes 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.

Here is the description of the append function:

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.

I've stepped through my append function using the debugger and still can't seem to figure out what's wrong with it.

The error changes to a Debug Assertion Failed as soon as I try and free up the memory I allocated for output.

Adding:
delete[] output;
after I assign outTemp to data generates:

Debug Assertion Failed!

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

Expression: _BLOCK_TYPE_IS_VALID(pHead->nBlockUse)

...

Abort || Retry || Ignore

You can look at the entirety of my code here:

(Updated append function is in this post)

string215.cpp: http://pastebin.com/Xh2SvDKJ

string215.h: http://pastebin.com/JfAJDEVN

Any help at all would be greatly appreciated!

Thanks, RAW-BERRY

[Branflakes91093]

Line 67 of string215.cpp:

        char *arr = new char[0];
        arr[0] = '\0';
        data = arr;

You allocate 0 objects and then try to change the first one (which doesn't exist).

[Disch]

I don't see heap corruption in the function you posted... however I do see a memory leak (you are new[]ing a buffer, but you are not deleting one. You are replacing a previously allocated 'data' buffer without deleting its previous buffer... therefore you are leaking the previous buffer).

EDIT: the append function on your pastebin does the delete[], so that isn't leaking any more. That's good.


After looking at other source you posted, I did find some heap corruption in the default constructor:

string215::string215()
{
        char *arr = new char[0];  // <- allocating an array of ZERO characters
                 //  that is... this array is not big enough to hold anything
        arr[0] = '\0';  // <- then you write to it!  but it's not big enough to hold 1 char.
              // heap corruption!
        data = arr;
}
// also, why did you need 'arr' here?  Why not just use 'data' directly? 

[cire]

You also have several member functions that access no class data, which is confusing. Why are they members of the class if they have no reason to access the class?

[Disch]

You also have several member functions that access no class data, which is confusing. Why are they members of the class if they have no reason to access the class?

That's not his fault. The header was provided by the class. He just has to write the functions.

But I agree it's nuts.


Also... are you aware you can use the + operator to add a variable amount to pointers? You do not have to keep incrementing.

For example...

char string215::getchar(int index) const
{
//...
                for(int x = 0; x < index; x++) {
                        stepper++;
                }
                return *stepper;
//...
}

This works... however it's much more complicated than it needs to be.

Rather than having a for loop, you could just add the index to stepper directly:

stepper += index;
return *stepper;

or better yet...

return *(stepper + index);

or better yet...

return stepper[index];

[RAWBERRY]

Wow that's it... spent so much time scratching my head over this and yet the problem was so simple. That's programming I guess haha. Thanks for the help!

You also have several member functions that access no class data, which is confusing. Why are they members of the class if they have no reason to access the class?

I was having some issues early on when doing the implementations. I think I did it to some and then changed the code yet never changed them back from being member functions. Thanks for pointing that out