0

For some reason, if len<3, I will get a heap corruption error at the delete[] buffer line.

case WM_COMMAND:
{
    switch(LOWORD(wParam))
    {
    case ID_BUTTON_OK:
        {
            int len = SendMessage(mapName, WM_GETTEXTLENGTH, 0, 0);

            char* buffer = new char[len+1];
            ZeroMemory(buffer,sizeof(buffer));

            len = SendMessage(mapName, WM_GETTEXT, (WPARAM)len+1, (LPARAM)buffer);

            if(isMapNameLegal(buffer)) // Makes sure the map name is legal
            {
            }
            else
            {
                MessageBox(NULL,"The map name is illegal","Error",MB_ICONERROR);
            }

            delete[] buffer;
        }
        break;

There is isMapNameLegal. It is static by the way:

bool Creator::isMapNameLegal(char* name)
{
    if(strlen(name)<=3)
        return false;

    int x=0;
    while(name[x]!='\0')
    {
        if(name[x]<48 && name[x]!=32)       //32=='\r', 48==0,
            return false;
        if(name[x]>57 && name[x]<65)        //57==9, 65==A
            return false;
        if(name[x]>90 && name[x]<97)        //90==Z, 97==a
            return false;
        if(name[x]>122)                     //122==z
            return false;
        x++;
    }

    return true;
}

Do you see where the corruption problem comes from?

trincot
  • 317,000
  • 35
  • 244
  • 286
Mickael Bergeron Néron
  • 1,472
  • 1
  • 18
  • 31
  • You might consider the difference between `sizeof(buffer)` and `len+1`. On that note, why are you zeroing an entire buffer only to fill all but the last char (which could be set as a single value) with a followup call for WM_GETTEXT?? – WhozCraig Apr 01 '13 at 21:52
  • 1
    You really don't need `ZeroMemory` at all, just check check the return value of `SendMessage` when you call it with `WM_GETTEXT`. And *strongly* consider compiling your code with Unicode support. It's 2013; ANSI apps went obsolete over a decade ago. – Cody Gray - on strike Apr 01 '13 at 21:56
  • The `ZeroMemory` is pointless. You are about to overwrite the buffer. – David Heffernan Apr 01 '13 at 21:57
  • @WhozCraig At first, I had a heap corruption error whatever the length of len. As I ran the debugger and noticed buffer was filled with stange symbols, I thought I should "ZeroMemory it" so to speak. While this didn't work, I let it ZeroMemoried thinking that maybe this was better. I eventually corrected the first heap corruption error but didn't remove the zeromemory. – Mickael Bergeron Néron Apr 01 '13 at 21:59

1 Answers1

5

sizeof(buffer) always returns sizeof(char*), which happens to be 4 on your system, so ZeroMemory(buffer,sizeof(buffer)); effectively becomes ZeroMemory(buffer,4);, regardless of how you allocated buffer.

When you allocate less than 4 bytes, the corruption happens.

If the name of the method suggest what it does, you can replace what you have with 

char* buffer = new char[len+1]();

The trailing () value-initializes the elements of the array.

Luchian Grigore
  • 253,575
  • 64
  • 457
  • 625