-2

I'm in the middle of creating a simple program with C++ (11).

My issue is that function called "loadRoomsFile()" is reading text from a file to char pointer array and then return it.

I want to convert it to a string so I have used string constructor with parameter of *char array.

All tests are showing, buffer also shows correct data but when calling string(loadRoomsFile()) I get bad_alloc exception and program stops running. Can anyone help me?

char *loadRoomsFile() {

fstream roomsFile;
char *buffer;

// helper
int i = 0;

cout << "TEST 1" << endl;

roomsFile.open("rooms.txt");

cout << "TEST 2" << endl;

if(roomsFile.is_open()) {

    cout << "TEST 3" << endl;
    roomsFile.seekg(0, ios::end);
    cout << "TEST 4" << endl;
    buffer = new char[roomsFile.tellg()];
    cout << "TEST 5" << endl;
    roomsFile.seekg(0, ios::beg);
    cout << "TEST 6" << endl;
    while(!roomsFile.eof()) {

        cout << "TEST WHILE" << endl;
        buffer[i] = roomsFile.get();
        i++;
    }
    cout << "TEST 7" << endl;
}

roomsFile.close();
cout << "TEST 8" << endl;

buffer[i] = '\0';
cout << "TEST 9" << endl;
cout << buffer << endl;

return buffer;
}


/*
 * GENERAL FUNCTION
*/

int main() {

// Variables
int currentSection; // option choosed in Main Menu
int revert = true; // used for going back in time XD
//string roomsData; // used for storage of data from file
vector<Room> rooms;

cout << "----------------------------------------------------------------" << endl;
cout << "Conference Room Manager ver. 0.1" << endl;
cout << "Component 1/4, Bartosz Kubacki/Bartlomiej Urbanek" << endl;
cout << "----------------------------------------------------------------" << endl;

cout << endl;

// load all data from files
cout << "TEST MAIN" << endl;
string roomsData(loadRoomsFile());

return 0;
}

Thanks!

  • 2
    Why not just read into a std::string in the first place? Also, you probably want to open the file in binary mode. And read https://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong –  May 15 '18 at 21:11
  • 2
    `int revert = true;` - huh? Type safety please - are you dealing with an `int` or `bool`? If `int` then don't assign a `bool`ean. If `bool` then change the variable type. What you have there may compile (due to backwards compatibility and stuff) but it doesn't make much sense. – Jesper Juhl May 15 '18 at 21:19

1 Answers1

2

First: You should have provided a Minimal, Complete and Verifiable example rather than making us have to correct your code to even build it. That's not respectful of our efforts.

Second: You allocate as many characters as the size of the file, but then you write '\0' after the last 1. So that seems to be the specific issue triggering the segfault.

But really, the main problem is with your approach:

  • You should not allocate memory yourself unless you absolutely have to.
  • You should not read the contents of an entire file into a string (unless you know it's really short).
  • You should not read one character at a time from a file. Either read line-by-line with std::getline(); or parse data using the pipe operator (like cin >> my_float); or read the whole file at once; or mmap() the file (that's not a C++ feature, but Linux, Windows, MacOS and other support this).
einpoklum
  • 86,754
  • 39
  • 223
  • 453