heap corruption error

[devnewbie]

Hi,

I am trying to do a very simple program that takes two dynamic char arrays and does a strcpy() implementation?

The program works fine until it has to delete the two char arrays. Then I get a heap corruption error. I am losing it trying to figure out why this is happening. :(
Can anyone tell me what I am doing wrong here?? Thanks a lot!!!!

Here is the code:

# include<iostream>
using namespace std;

int main()
{
int i;
char *src=new char[];
char *dest=new char[];

cout<<"Enter the string to copy";
gets(src);

for(i=0;src[i]!='\0';i++)
{
dest[i]=src[i]; //the strcpy stuff
}
dest[i]='\0'; //truncate dest to remove extra garbage chars.

cout<<"the destination string is"<<dest;
delete [] src; //here the program breaks and shows heap corruption error
delete [] dest;

return 0;
}

[Denis]

# include<iostream>

int main()
{
    const int MAX_SIZE_INPUT_STRING(100);
    char src[MAX_SIZE_INPUT_STRING] = {0};

    std::cout<<"Enter the string to copy:\n";
    std::cin >> src;

    char* dest(new char[strlen(src) + 1]);
    int i(0);
    while(src[i] != '\0')
    {
        dest[i++] = src[i];
    }
    dest[i]='\0'; //truncate dest to remove extra garbage chars.

    std::cout << "the destination string is:\n" << dest << '\n';

    delete [] dest;

    return 0;
}

[devnewbie]

Thanks Denis.

What I was trying to do was avoid having to specify any max array size..like here you have specified 100.
const int MAX_SIZE_INPUT_STRING(100);

Is that why I am running into heap errors?

Thanks!

[Denis]

The first:
When you call operator new[] to allocat heap memory for array you have to define required memory size(example is line 11 of my solution), I hope it's clear
The second:
The user data should be located in array, so user input should be inserted in some array that is already created, but we don't know size of required array size, so we can just create some large array on stack(example is line 6 of my solution)

[Lv0f92yv]

Not trying to jack this thread - I am interested in this. Does line 17 in the example change the ammount of space the dest array takes up on the stack? I presume no, but I didn't realize why that sort of truncation was done until this example...

[guestgulkan]

Don't forget that we are dealing with C style strings.
The actual copying part:

    while(src[i] != '\0')
    {
        dest[i++] = src[i];
    }

copies everything EXCEPT the terminating 0 from the source array to the destination array.
So the destination array need the terminating 0 to be added - otherwise we have a dangerous undefined length string.
That is the whole point of line 17:

    dest[i]='\0'; //truncate dest to remove extra garbage chars.

[Lv0f92yv]

I see. The dest array was declared for size 100, now if that copy procedure above copies say, 15 elements into the array (including the null terminator), (stupid question coming up): what is the size of the array? 15? or 100?

[guestgulkan]

Still 100.

15 is the string length.

which means that there are 100 - 15 = 85 unused positons after the terminating 0.

[Lv0f92yv]

Alright, that makes sense. Thanks.

[Denis]

:)

C, C and again C, how long ago it was :), it's good language, but C++ .... mmm, C++ is my favorite.

[Disch]

Wow... there's a HUGE error here and I'm really suprised nobody caught this:

    while(src[i] != '\0')
    {
        dest[i++] = src[i];  // BAD BAD BAD BAD
    }

You can't use 'i' twice between sequence points if it's being modified like that. The compiler might increment i before indexing src... so if i=0, you might be left with something like: dest[0] = src[1]. Bad bad bad bad bad. Never do this.

No need to compound statements like that. Just do this:

while(src[i] != '\0')
{
  dest[i] = src[i];
  ++i;
}

The extra line doesn't hurt. In fact it prevents a world of hurt.

I don't know why people want to compound statements all the time. It just leads to more obfuscated code with nasty bugs.

[Denis]

Thanks.

But can you point me where is explained?

[DemienBjarne]

Actually there's no error in there:

int i = int(0);
while(src[i] != '\0')    
        dest[i++] = src[i];  // ok ok ok  
dest[i] = 0; // '\0' for the purist....   

The right hand side of an assignment operation is resolved first, after the character is fetched from
src[i] is assigned to dest[i++] where both i's from scr and dest have the same value; Only after the assignment is that the postcrement takes place....
The thing would get buggy the other way around, i mean scr[i++]!

[Albatross]

There's indeed no error, however different compilers will treat this differently. ++ is a fairly broken operator in C++, as far as that's concerned. We've had two long threads already about ++ and -- (which were fairly pointless, I must say), and... you get the idea. When he said error, I think he meant error in a human sense, not compiler.

To ensure that the code will behave the same regardless of the compiler, we use +1 or ++ in its own statement. Hopefully, they fixed it in 0x... *checks*

-Albatross