Unusual Error with strings

[snow 56767]

This is a problem that baffles me.

I have MSVC 9 (2008) and I have made an application that loads a basic text file from the disk, parses it for the information it want's, then fills in certain pieces of it's-self with the gathered information. It runs just fine (both in Release and Debug configurations) when I debug it in MSVC by hitting F5. But the second I try to load it from disk like a normal application, it crashes at a very specific line everytime. The line is:

size_t length = buffer.length()

were buffer is a std::string object and length is a simple unsigned integer, when I debug the crashed application with JIT debugger, it says that the program "deadlocks," but I'm not using more than one thread. So, I am perplexed.

HERE IS THE COMPLETE FILE:

#include <windows.h>
#include <tchar.h>
#include <cstdio>
#include <cstdlib>
#include <iostream>
#include <fstream>
#include "resource.h"
using namespace std;

long hScr = 0;
long int key = 0;
double time = 0.0;
int complete = 0;
int map = 0;
bool closed = false;
void LoadProfile();

BOOL CALLBACK
Dialog(HWND hWnd, UINT Msg, WPARAM wParam, LPARAM lParam)
{
    switch(Msg)
    {
        case WM_INITDIALOG:
        {
            LoadProfile();

            char *scr = new char;
            char *t = new char;
            char *comp = new char;
            char *k = new char;
            itoa(key, k, 10);

            sprintf(scr, "Combined Scores: %d", hScr);
            sprintf(t, "Total Time Played: %d", time);
            sprintf(comp, "Maps Completed: %d", complete);

            SendMessageA(GetDlgItem(hWnd, IDC_HSCR), WM_SETTEXT, 0, (LPARAM) scr);
            SendMessageA(GetDlgItem(hWnd, IDC_TIME), WM_SETTEXT, 0, (LPARAM) t);
            SendMessageA(GetDlgItem(hWnd, IDC_COMP), WM_SETTEXT, 0, (LPARAM) comp);
            SendMessageA(GetDlgItem(hWnd, IDC_KEY), WM_SETTEXT, 0, (LPARAM) k);

            return true;
        }

        case WM_COMMAND:
        {
            switch(LOWORD(wParam))
            {
                case IDOK:
                    DestroyWindow(hWnd);
                    break;

                case IDCANCEL:
                    DestroyWindow(hWnd);
                    break;
            }
            break;
        }

        case WM_DESTROY:
        {
            PostQuitMessage(0);
            return true;
        }

        default:
            return false;
    }

    return true;
}

int WINAPI
WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine, int nShowCmd)
{
    HWND dlg = CreateDialogParam(hInstance, MAKEINTRESOURCE(IDD_WIND), 0, Dialog, 0);
    ShowWindow(dlg, SW_SHOW);

    SendMessage(dlg, WM_SETICON, ICON_SMALL, (LPARAM)LoadIcon(hInstance, MAKEINTRESOURCE(IDI_ICON1)));
    SendMessage(dlg, WM_SETICON, ICON_BIG, (LPARAM)LoadIcon(hInstance, MAKEINTRESOURCE(IDI_ICON1)));

    BOOL rslt = FALSE;
    MSG msg;
    memset(&msg, 0, sizeof(MSG));

    while((rslt = GetMessage(&msg, 0, 0, 0)) != 0)
    {
        if(rslt == -1)
        {
            return -1;
        }

        if(closed)
        {
            break;
        }

        if(!IsDialogMessage(dlg, &msg))
        {
            TranslateMessage(&msg);
            DispatchMessage(&msg);
        }
    }

    return msg.wParam;
}

void LoadProfile()
{
    fstream f;
    f.open("Data/Profile/default.p", ios_base::in);

    if(f.fail())
    {
        f.close();
        return;
    }

    //LOAD THE FILE
    string buffer = "";
    string temp = "";
    string temp3 = "";
    char *temp2 = new char;
    bool good = f.eof();
    while(!good)
    {
        f >> temp2;
        buffer.append(temp2);
        buffer.append(" ");
        good = f.eof();
    }
    f.close();

    size_t length = buffer.length(); // <----- This is the broken line

    //PARSE THE FILE
    for(unsigned int i = 0; i < length; ++i)
    {
        if(buffer.at(i) == '$')
        {
            //LOAD THE DATA INTO A BUFFER
            temp.clear();
            string type = "";
            for(unsigned int j =0; j < length; ++j, ++i)
            {
                //FIND OUT WHAT CONFIGURATION IS BEING LOADED
                if(buffer.at(i) == '=')
                {
                    type = temp;
                    ++i;
                    ++i;
                    break;
                }

                if(buffer.at(i) == ' ')
                {
                    continue;
                }

                if(j != 0)
                {
                    temp.append(1, buffer.at(i));
                }
            }

            //USE THE DETERMINED CONFIG TYPE AND LOAD THE DATA
            temp.clear();
            for(unsigned int j = 0; j < length; ++j, ++i)
            {
                if(!strcmp(type.data(), "p_hscr"))
                {
                    if(buffer.at(i) == ' ')
                    {
                        hScr = atol(temp.data());
                        break;
                    }
                    temp.append(1, buffer.at(i));
                }

                else if(!strcmp(type.data(), "p_time"))
                {
                    if(buffer.at(i) == ' ')
                    {
                        time = atof(temp.data());
                        break;
                    }
                    temp.append(1, buffer.at(i));
                }

                else if(!strcmp(type.data(), "p_comp"))
                {
                    if(buffer.at(i) == ' ')
                    {
                        complete = atoi(temp.data());
                        break;
                    }
                    temp.append(1, buffer.at(i));
                }

                else if(!strcmp(type.data(), "p_map"))
                {
                    if(buffer.at(i) == ' ')
                    {
                        map = atoi(temp.data());
                        break;
                    }
                    temp.append(1, buffer.at(i));
                }

                else if(!strcmp(type.data(), "p_skey"))
                {
                    if(buffer.at(i) == ' ')
                    {
                        key = strtol(temp.data(), NULL, 16);
                        break;
                    }
                    temp.append(1, buffer.at(i));
                }
            }
        }
    }
}

[Peter87]

The problem is on line 127: f >> temp2;
temp2 is a char* so it will try to read a whole word but there is not enough room for more than one char.

[snow 56767]

No,
temp2 is a char pointer. So it is a C-Style string and I have used that many times before without any errors, it is the the line 134: size_t length = buffer.length();
Whenever I use JIT debugger, it always stops on that line and nowhere else. But the code runs just fine when running the the debugging instance from MSVC. That is why I am confused.

[Peter87]

But you have not allocated enough room for your c string
char *temp2 = new char;
The only c string that can be stored in one char is the empty string "". Why not use a std::string instead?

[Moschops]

So it is a C-Style string

No. No no no. That is wrong. It is not true. It is a lie to say that a char pointer is a C-style string. I'll say it again. That's not true. It's not true. If you make a char pointer, you have not necessarily made a c-style string.

A char pointer is a pointer to a char.
A c-style string is an array of char, terminated with a final char of value zero.

Do you see the difference?

When you allocate enough space for a single char, like this:

char* p = new char;

how many letters can you put in it? One.
Can you put a terminating final char value of zero on there, into the second char space? No. There is no second space. You could put a single character into the space that p points at, but because you cannot put a final zero on it, it is not a C-style string. It is just a char.

What happens if you try to write to the second space? You write off the end of your allocated single space and you start trashing data. This is a classic buffer overflow error. It's very well-known in C.

I have used that many times before without any errors,

You've made this error many times and never been lucky enough to trash data that causes a segFault or some other way to easily tell you've made the error.

[snow 56767]

Sorry, my bad. I have always seem/heard it called a C-Style string. sorry about that.

The only problem though, is that MSVC doesn't say that it is an error. And I have written functional
C++ code that uses char pointers that hold entire strings. Now I'm confused, does that mean that
I can't do this:
const char *str = "This is A String";

I thought you could, and my compiler allows me to. Also, is it possible that JIT debugger is breaking at the wrong line? In other words, the problem line is not line 134?

Also, how would I read from istream into a string object? I can't seem to get it to work.
And thank you guys for helping me on this baffling problem.

[snow 56767]

Ok,
I finally got it. You guys are right that I am allocating only one character, I didn't realize it because MSVC allocates extra data during the debug session. So it was a buffer overflow problem.
Not that the std::string was broken. Thanks guys for your help! It is very much appreciated!!

Here is the fixed code:

//------// LINES 1 - 133 //------//

//LOAD THE FILE
    string buffer = "";
    string temp = "";
    string temp3 = "";
    char *temp2 = new char [4096];
    bool good = f.eof();
    while(!good)
    {
        f >> temp2;
        buffer.append(temp2);
        buffer.append(" ");
        good = f.eof();
    }
    f.close();
    delete[] temp2;

    size_t length = buffer.length();

//------// LINES 151 - 238 //------//

[Moschops]

The only problem though, is that MSVC doesn't say that it is an error

Welcome to C coding. What happens in C when you write over memory you shouldn't? Sometimes a segFault. Sometimes you trash data. Sometimes that memory wasn't being used and you never even notice.

It's not an error of syntax, and the compiler can only catch syntax errors. One of the fundamental principles of C is "trust the coder". It's assumed that you know what you're doing and if you want to do something to some memory, you can. The OS might segFault, but the compiler trusts you.

const char *str = "This is A String";
This is a "string literal". The text "This is A String" is written, exactly like that, into the compiled object, and then upon running, is created in memory, exactly like that. It takes up exactly enough space to be a C-style string. In this case, it will be an array of 17 char - 16 for the letters and one for the terminating zero.

The statement
"This is A String"
effectively returns a const char* - a pointer to the array of char.


Also, how would I read from istream into a string object?

If you want it from a file, how about something like this:

string aString;
ifstream theInputStream("somefile);
theInputStream >> aString;