0

Visual Studio 2010

When I allocate memory for a char string, pass the string (pointer) to a function, then try to free the memory, I get a "Heap Corruption Detected" run-time error.

I suspect this is a result of the function marking the memory as "freed" once it returns, but I am still puzzled as how to get around this. I do not feel comfortable simply removing the call to free().

// this function takes a char string, converts it to a wide char string, 
// then passes the converted string to another function.
void Class::WriteToLog(const char * msg)
{
    // allocate memory
    wchar_t * wMsg = (wchar_t*)malloc((strlen(msg) * sizeof(wchar_t)) + 1); // add one for null terminating char
    // convert to wide char. This works fine, and returns 4 when msg is 4 chars long.
    numChar = MultiByteToWideChar(CP_ACP, MB_PRECOMPOSED, msg, strlen(msg), wMsg, strlen(msg));

    wMsg[numChar] = L'\0'; // terminate string
    // At this point wMsg contains the same thing msg contains. Works fine.
    WriteToLogW(wMsg); // Works fine. Writes the string to a text file.
    free(wMsg); // Heap Corruption Detected
}


void Class::WriteToLogW(const wchar_t * msg)
{
    ...
}
paperduck
  • 1,175
  • 1
  • 16
  • 26
  • 1
    For future reference, “Heap Corruption Detected" Almost always means you wrote past the end of a dynamically allocated array. Like, 95% of the time. – Mooing Duck Jan 30 '14 at 22:29

2 Answers2

2

The +1 you added at the end of your malloc will only allocate enough memory for the null terminator when working with chars, as chars are 1 byte (and hence the null terminator will take up one byte).

Since you're using wchar_t, the null terminator will take up sizeof(wchar_t) bytes, which will virtually always be greater than 1. Therefore, the malloc call allocates insufficient memory. Re-structure your malloc call to look like this:

wchar_t * wMsg = (wchar_t*) malloc((strlen(msg) + 1) * (sizeof(wchar_t));

This way the total number of bytes allocated leaves room for the null terminator.

ApproachingDarknessFish
  • 14,133
  • 7
  • 40
  • 79
  • Thanks, that's what it was. Adding the extra space in memory made the program run without error. Marked as correct since you were first by 1 minute. – paperduck Jan 31 '14 at 14:50
2

Even the terminating nul of a wide-character string is wide, so giving it only a single byte isn't enough. Change:

wchar_t * wMsg = (wchar_t*)malloc((strlen(msg) * sizeof(wchar_t)) + 1);

to

wchar_t * wMsg = (wchar_t*)malloc((strlen(msg) + 1) * sizeof(wchar_t));
Dave
  • 44,275
  • 12
  • 65
  • 105
  • 2
    Another note: strlen is O(n), so you should calculate it once and cache it in a local variable instead of calling it 3 times. – Dave Jan 30 '14 at 22:27
  • Thanks, that's what it was. Adding the extra space in memory made the program run without error. I also combined the calls to strlen() into one call. – paperduck Jan 31 '14 at 14:51