Warum beschädigt dieser Code Speicher?

Question

Dies ist eine ziemlich Neulingsfrage, die vernünftigerweise schnell verantwortlich sein sollte ...

Grundsätzlich nach dem ersten Anruf an Printf in Echo, Die Inhalte von Args ist korrupt. Es klingt für mich so, als würde ich die Zeiger falsch weitergeben. Aber können nicht herausfinden, warum?

#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);

};

HINWEIS Dies ist kein Produktionscode, nur Proof of Concept.
BEARBEITEN G_PLATFORMDEVICE ist vom Typ Rehwindows, wenn das nicht klar war ...
BEARBEITEN Dies befindet sich auf einer Windows XP -Plattform unter VS2008

AKTUALISIERENFür alle Interessierten scheint das Problem ein überflüssiger Anrufstapel gewesen zu sein, weiter unten im Stapel wurde dieses weitere große Array definiert. Nach dem Nacharbeiten beseitigte die Speicherversorgung. Also kreidet, um zu stapeln!

Answer 1

Sie haben nicht erwähnt, in welcher Umgebung dieser Code ausgeführt wird. Es könnte sein, dass Sie Ihren Stapel blasen. Sie deklarieren ein 32767 -Byte -Array auf dem Stapel in Rippenwindows :: Print. In einigen eingebetteten Systemumgebungen, mit denen ich vertraut bin, wären schlechte Nachrichten. Können Sie Ihre Stapelgröße und/oder diesen Puffer auf dem Heap erhöhen, um diese Theorie zu testen? Möglicherweise möchten Sie diesen Puffer stattdessen zu einem std :: Vektor oder möglicherweise zu einem privaten Mitgliedsvektor machen, um das Zuteilung und Neuverglasungen jedes Mal, wenn Sie den Druck anzurufen, zu vermeiden.

Wie groß ist 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).

Answer 3

Random guess: I get the feeling that the problem is caused by this line in Printf:

char formattedOutput[MAX_PRINT_OUTPUT];

The reason I think this is because you've got some obviously declared pointers and some obviously undeclared pointers. An array of chars is a pointer - no way around that but it's not obvious. In the function definition of Echo args is defined as a TWO DIMENSIONAL ARRAY because you have it as

*args[MAX_COMMAND_ARGS]

Do you want that? My guess is that something is unintentionally being passed as a reference instead of a value because what is a pointer vs. array is vaguely defined and you're passing a pointer to a pointer which is the start of an array instead of a pointer that is the start of an array. Since you said that it gets corrupted when you enter RiseWindows::Print my guess is that you're passing the wrong thing.

In addition, a const pointer to a char only preserves the value of the pointer as far as I know, not the value of the contents at the pointer.

Answer 4

Did you try the divide and conquer strategy?

• Start commenting out lines until it work.
• Once it work correctly uncomment lines until you hit where the error is.

Watch the memory pointed by args[] in separate window while you do step by step also can be helpful.

Answer 5

Might I suggest stepping through with a debugger to see where the code corrupts?

Answer 6

while (pOutput[i] && ((pBuffer - buffer) < sizeof(buffer) - 1))

change to:

while (pOutput[i] && ((pBuffer - buffer) < sizeof(buffer) - 2))

you are writing 2 characters at a time so you need to make sure you have room for two characters.

Answer 7

not sure if it's how it's supposed to work or not but Echo doesnt print out the first element of args

// Changed i=1 to i=0;
for (int i = 0; i < argCount; ++i)
{
    Printf("%s ", args[i]);
    Printf("\n");
}

Answer 8

You want to move your #define out of the function call to the top of the file:

Preprocessor directives can appear anywhere in a source file, but they apply only to the remainder of the source file.

This probably isn't causing the corruption in this case, but it's non-standard, and could easily cause problems down the line.

Answer 9

The working theory is that we're blowing the stack with:

char buffer[CONSOLE_OUTPUT_SIZE]; char *pBuffer = buffer;

Instead try:

char *pBuffer = new char[CONSOLE_OUTPUT_SIZE];

And then remember to call delete [] pBuffer at the end.

Answer 10

I haven't really investigated this, but you've got your types mixed up... This is extremely pedantic, but it does make a difference in C.

Here, you have a single array of char.

char formattedOutput[MAX_PRINT_OUTPUT];

And here, you have a function that expects a const char pointer.

void RiseWindows::Print(const char *output)

Try:

void RiseWindows::Print(const char output[])

Additionally, I notice you are modifying the memory in those buffers - are you sure you can do that? At the very least, I'm certain that you can't just arbituarily use more without allocating more memory. (Hint hint!)

I would allocate my own array, and copy the string into it. I would then use string functions to replace the newlines as applicable.

Finally, I STRONGLY suggest that you use std::string here. (Though you won't be able to put those into the varargs stuff - you'll have to use c-strings for those, but copy those back into std::string's when you can).