stack corrupted after memcpy

Question

I want to copy an object and send it over the network with winsock, but there is one problem. I destroy the stack if I copy an object to an array of chars on the heap. Here is my code:

testclass backditup;       //This is an object with information
testclass restorenaarhier; //I will copy it to this object

backditup.setvalues(100,100);
restorenaarhier.setvalues(0,0);

char * dataholder = new char[sizeof(backditup)];  //This is the data holder
ZeroMemory(&dataholder,sizeof(backditup));

memcpy(&dataholder,&backditup,sizeof(backditup)); //Save it to the char[]

//Sending it over the network
//resieving the object

//Store the data on the same object 
memcpy(&restorenaarhier,&dataholder,sizeof(restorenaarhier));

//deleting the data holder
ZeroMemory(&dataholder,sizeof(dataholder));
delete dataholder;

//output the code
restorenaarhier.echo();

The code will work properly, but when I compile this in debug mode I get at the end:
http://imageshack.us/photo/my-images/839/errormnr.png/

Run-Time Check Failure #2 Stack around the variable 'dataholder' was corrupted.

Can someone help me with this?

Change `char* dataholder = new char[sizeof(backditup)];` to `char* dataholder = new char[sizeof(backditup)]();` and there's no need for `ZeroMemory` whatsoever. This isn't C, it's C++, so you should be using C++ idioms. — ildjarn, Feb 02 '12 at 21:58

score 2 · Answer 1 · answered Feb 02 '12 at 21:53

2

I'm unsure if this would be part of the problem, but the delete should be:

delete[] dataholder;

More importantly, the ZeroMemory call should not be passing the address of dataholder (&dataholder) but rather its value (what it points to):

ZeroMemory(dataholder ...

answered Feb 02 '12 at 21:53

Mark Wilkins

40,729
5
57
110

Good catch! I don't think that `delete` is the problem causing this particular error, though -- that will corrupt things in the heap, not the stack. – Brooks Moses Feb 02 '12 at 21:57
Is the difference between `delete` and `delete[]` really relevant for "plain old data"? Surely it plays a role if destructors are involved, but otherwise...? – krlmlr Feb 02 '12 at 22:38
1

@user946850: I believe the standard says you must use `delete[]` in this situation regardless. [This post discusses it some](http://stackoverflow.com/questions/1553382/is-delete-equal-to-delete). – Mark Wilkins Feb 02 '12 at 22:55
1

@user946850: Otherwise you still need it. The `delete` implementation needs to know how big the block is that it's freeing. If you're freeing a single value, that's implied by the pointer type -- but, if you're freeing an array value, it needs to have kept the array length around somewhere. Now, in practice _most_ compilers do the same thing in both cases (which I assume means keeping the allocated block length around regardless), but you can't rely on that always being the case. – Brooks Moses Feb 03 '12 at 00:03

Brooks Moses · Accepted Answer · 2012-02-02T22:03:28.703

Your dataholder variable is a pointer to an array the size of backditup, not the array itself. Thus, when you do the Zeromemory and memcpy calls, you should not take its address; instead, write:

ZeroMemory(dataholder,sizeof(backditup));
memcpy(dataholder,&backditup,sizeof(backditup));

without the &. Likewise, when you copy the data back, you want:

memcpy(&restorenaarhier,dataholder,sizeof(restorenaarhier));

And, finally, you need to make the same fix in the second Zeromemory call -- although, since you are deleting the array immediately after that call, there really is no point in having that call at all.

The size in the second Zeromemory call is erroneous for the same reason; sizeof(dataholder) is the size of the pointer, not the array it's pointing to. If you don't simply delete this call entirely, you should either use sizeof(backditup) here for consistency with the declaration, or better yet, declare a variable to hold the length of the dataholder array and use that consistently. (Or you can use the size of the data type, sizeof(testclass) -- that's probably the best option.)

Finally, as Mark Wilkins noted in his answer, you need to delete arrays with delete[], not delete, to avoid corrupting the heap.

There's no point in the first `ZeroMemory` call either, given that a simple `()` will do the exact same thing. — ildjarn, Feb 02 '12 at 22:03

score 0 · Answer 3 · answered Feb 02 '12 at 21:57

You are overwriting the stack in your memcpy calls. The reason is that you are taking the address of the variable which hold the address of your buffer. All you want is the address of your buffer.

Use "dataholder" not "&dataholder" in the Zeromemory and memcpy calls.

stack corrupted after memcpy

3 Answers3