CODEWITHSUNDEEP
c++visual-c++
Cyrix
Cyrix

Reputation: 266

strcat error "Unhandled exception.."

My goal with my constructor is to:

open a file read into everything that exists between a particular string ("%%%%%") put together each read row to a variable (history) add the final variable to a double pointer of type char (_stories) close the file.

However, the program crashes when I'm using strcat. But I can't understand why, I have tried for many hours without result. :/

Here is the constructor code:

Texthandler::Texthandler(string fileName, int number) 
        : _fileName(fileName), _number(number)  
{
    char* history = new char[50];

    _stories = new char*[_number + 1]; // rows
    for (int j = 0; j < _number + 1; j++)
    {
        _stories[j] = new char [50]; 
    }
        _readBuf = new char[10000]; 

    ifstream file;
    int controlIndex = 0, whileIndex = 0, charCounter = 0;

    _storieIndex = 0;

    file.open("Historier.txt"); // filename 
    while (file.getline(_readBuf, 10000))
    {
        // The "%%%%%" shouldnt be added to my variables
        if (strcmp(_readBuf, "%%%%%") == 0)
        {
        controlIndex++;
        if (controlIndex < 2)
        {
            continue;
        }
    }

    if (controlIndex == 1)
    {
        // Concatenate every line (_readBuf) to a complete history
        strcat(history, _readBuf);
        whileIndex++;
    }

    if (controlIndex == 2)
    {
        strcpy(_stories[_storieIndex], history);

        _storieIndex++;
        controlIndex = 1;
        whileIndex = 0;
        // Reset history variable
        history = new char[50];

    }
}
file.close(); 
}

I have also tried with stringstream without results..

Edit: Forgot to post the error message: "Unhandled exception at 0x6b6dd2e9 (msvcr100d.dll) in Step3_1.exe: 0xC00000005: Access violation writing location 0c20202d20." Then a file named "strcat.asm" opens..

Best regards Robert

Upvotes: 0

Views: 2079

Answers (2)

simonc
simonc

Reputation: 42175

strcat operates on null terminated char arrays. In the line

strcat(history, _readBuf);

history is uninitialised so isn't guaranteed to have a null terminator. Your program may read beyond the memory allocated looking for a '\0' byte and will try to copy _readBuf at this point. Writing beyond the memory allocated for history invokes undefined behaviour and a crash is very possible.

Even if you added a null terminator, the history buffer is much shorter than _readBuf. This makes memory over-writes very likely - you need to make history at least as big as _readBuf.

Alternatively, since this is C++, why don't you use std::string instead of C-style char arrays?

Upvotes: 1

paxdiablo
paxdiablo

Reputation: 881513

You've had a buffer overflow somewhere on the stack, as evidenced by the fact one of your pointers is 0c20202d20 (a few spaces and a - sign).

It's probably because:

char* history = new char[50];

is not big enough for what you're trying to put in there (or it's otherwise not set up correctly as a C string, terminated with a \0 character).

I'm not entirely certain why you think multiple buffers of up to 10K each can be concatenated into a 50-byte string :-)

Upvotes: 2

Related Questions