CODEWITHSUNDEEP
c++memorymfcbreakpointsheap-corruption
IO_INC
IO_INC

Reputation: 75

Heap corruption and random breakpoints

I'm trying to finish up on my simple archive system. I use an MFC application to handle building archive files. The only problem is one function that seems to cause errors but Visual Studio can't tell me where or why so I am lost on what to do. Even after searching, none of the solutions work for me.

bool UE_ArchiveLoad(UE_Archive * archive, char* path)
{

ifstream input = ifstream(path, std::ifstream::in | std::ifstream::ate | std::ifstream::binary);

char* Header = new char[3];
char version = 0;
if (input.fail())
{
    return false;
}
input.seekg(0, ios::beg);


input.read(Header, 3);
input.read(&version, 1);

string T = Header;
T = T.substr(0, 3);
Header = (char*)T.c_str();

if (strcmp( Header,UE_ARCHIVE_SIGNATURE)!=0)
{
    printf("INVALID SIGNATURE: ");
    cout << Header <<endl ;
    return false;
}

if (version != UE_ARCHIVE_VERSION)
{
    printf("INVALID VERSION");
    return false;
}

archive->Header = UE_ArchiveHeader();

archive->Header.signature = Header;
archive->Header.version = version;

while(input.is_open())
{

    auto F = make_unique<UE_ArchiveFile>();

    auto Name = std::make_unique<char>();

    input.read(Name.get(), 128);

    F->name = Name.get();

    input.read(reinterpret_cast<char*>(&F->size), sizeof(long));
    if (F->size < 0)
        break;

    char * buffer = new char[F->size];

    input.read(buffer, F->size);
    F->buffer = buffer;

    if (input.fail())
    {
        break;
    }
    else

        archive->Files.push_back(move(F));

}

input.close();
return true;
}

Sometimes this function runs without auto triggering a breakpoint, but if it works then the program will randomly break even while sitting idle a few seconds after this function is used. Other times it stops at the very last line. I cannot figure out what is going on with so much inconsistency, I know I did something in here to break the program.

Upvotes: 0

Views: 352

Answers (1)

Sami Kuhmonen
Sami Kuhmonen

Reputation: 31163

Most likely due to the way you use Header.

First you dynamically allocate three bytes. Then you read three bytes. That's fine. Then you assign it to a string. Not fine. There is no terminating nul, so it will go on reading memory until one is found and causes undefined behaviour.

The whole thing is a mess. Why do you go through the string? You clearly realized something's wrong and tried to fix it.

You will also leak memory since you don't release the allocation.

Why not just allocate a four byte array, read three bytes and set the fourth to nul? Then you don't need the whole string kludge and probably won't have random crashes.

Also since archive is used outside you have another issue: you are storing a reference to a local object that will be destroyed after the method is finished, causing even more problems.

And do release the memory on the case of returning early.

Upvotes: 1

Related Questions