Why does this code corrupt memory?

Question

This is a fairly newbie question which should be answerable reasonably quickly...

Basically, after the first call to Printf in echo, the contents of args is corrupted. It sounds to me like i'm passing the pointers around incorrectly. But can't figure out why?

#define MAX_PRINT_OUTPUT 4096

void Echo(char *args[MAX_COMMAND_ARGUMENTS], int argCount)
{
    for (int i = 1; i < argCount; ++i)
    {
        Printf("%s ", args[i]);
        Printf("\n");
    }
};

void Printf(const char *output, ...)
{
    va_list args;
    char formattedOutput[MAX_PRINT_OUTPUT];

    va_start(args, output);
    vsnprintf(formattedOutput, sizeof(formattedOutput), output, args);
    va_end(args);

    g_PlatformDevice->Print(formattedOutput);
};

void RiseWindows::Print(const char *output)
{
    //Corruption appears to occur as soon as this function is entered
    #define CONSOLE_OUTPUT_SIZE 32767

    char buffer[CONSOLE_OUTPUT_SIZE];
    char *pBuffer = buffer;
    const char *pOutput = output;
    int i = 0;

    while (pOutput[i] && ((pBuffer - buffer) < sizeof(buffer) - 1))
    {
        if (pOutput[i] == '\n' && pOutput[i+1] == '\r' )
        {
            pBuffer[0] = '\r';
            pBuffer[1] = '\n';
            pBuffer += 2;
            ++i;
        }
        else if (pOutput[i] == '\r')
        {
            pBuffer[0] = '\r';
            pBuffer[1] = '\n';
            pBuffer += 2;
        }
        else if (pOutput[i] == '\n')
        {
            pBuffer[0] = '\r';
            pBuffer[1] = '\n';
            pBuffer += 2;
        }
        else
        {
            *pBuffer = pOutput[i];
            ++pBuffer;
        }
        ++i;
    }
    *pBuffer = 0;

    SendMessage(this->ConsoleWindow.hwndBuffer, EM_LINESCROLL, 0, 0xffff);
    SendMessage(this->ConsoleWindow.hwndBuffer, EM_SCROLLCARET, 0, 0);
    SendMessage(this->ConsoleWindow.hwndBuffer, EM_REPLACESEL, 0, (LPARAM)buffer);

};

NOTE This is not production code, just proof of concept.
EDIT g_PlatformDevice is of type RiseWindows, if that wasn't clear...
EDIT This is on a windows xp platform running under vs2008

UPDATE For anyone interested, the problem appears to have been an overflowed call stack, further down the stack then this another large array was being defined. Refactoring this eliminated the memory corruption. So chalked up to stack battering!

Answer 1

You haven't mentioned what environment this code runs under. It could be you are blowing your stack. You are declaring a 32767 byte array on the stack in RiseWindows::Print. On some embedded system environments that I am familiar with that would be bad news. Can you increase your stack size and/or allocate that buffer on the heap just to test that theory? You may want to make that buffer a std::vector instead, or possibly a private member vector to avoid allocating and reallocating it every time you call Print.

Along those lines, how big is MAX_PRINT_OUTPUT?

Answer 2

Probably not the bug you're asking about, but in your loop you are double incrementing pBuffer in some cases, which could be pushing you over the end of buffer because you only check against length-1 (for null termination).