views:

139

answers:

3

What's the problem with this code? It crashes every time.

One time it's a failed assertion "_ASSERTE(_CrtIsValidHeapPointer(pUserData));", other times it is just a "heap corrpuption" error.

Changing the buffer size affects this issue in some strange ways - sometimes it crashes on the "realloc", and other times on the "free".

I have debugged this code many times, and there is nothing abnormal regarding the pointers.

char buf[2000];
char *data = (char*)malloc(sizeof(buf));
unsigned int size = sizeof(buf);

for (unsigned int i = 0; i < 5; ++i)
{
 char *ptr = data + size;
 size += sizeof(buf);
 char *tmp = (char*)realloc(data, size);
 if (!tmp)
 {
  std::cout << "Oh no..";
  break;
 }
 data = tmp;
 memcpy(ptr, buf, sizeof(buf));
}

free(data);

Thanks!

+1  A: 

You're trashing the heap. realloc can freely choose to return you memory from an entirely different location as it reallocates, and this is invalidating your ptr. Set ptr after reallocating.

Matti Virkkunen
Yep.. After writing the post exactly that caught my eye.. I guess all that's required to get an answer is to write a post.. especially if you're debugging your application for nearly 4 hours to find a problem. :D Thanks!
Windindeed
A: 

On the second iteration of the loop here are the values

  • data points to a buffer of size sizeof(buf)
  • size has a value of sizeof(buf)

Given these values the value of ptr is that it points past the end of the buffer allocated into data. This is memory not owned by the process and the following memcpy operation writes to this and corrupts memory.

JaredPar
A: 
char *ptr = data + size;
char *tmp = (char*)realloc(data, size);
memcpy(ptr, buf, sizeof(buf));

The call to realloc() here can potentially free the old buffer, before returning the new one.

John Millikin