-2

I'm trying to read in a file into an array so that I could process the array into selection sort. But when I try to read in the file, I get a segmentation fault(core dumped) error. Here is my code:

    #include <iostream>
    #include <fstream>
    #include <string>

    using namespace std;

    int main()
    {
        string array[40707];
        int loop = 0;
        int loop2;
        string line;
        ifstream inFile("beowulf.txt");
        if (inFile.is_open())
        {
            while(!inFile.eof())
            {
                getline(inFile, line);
                array[loop] = line;
                loop++;
            }
            inFile.close();
        }
        else cout <<  "Unable to open file" << endl;
        for (loop2 =0; loop2 <= loop; loop2++)
            cout << array[loop2] << endl;
        return 0;
    }
mcuvs
  • 7
  • 1
  • 1
    Try using `std::vector` rather than a `string` array. Then, you can use `array.push_back(line)` instead of an index. – jxh Jun 30 '15 at 02:29
  • 1
    How many lines in your file? I don't see any range checking here. – Tim3880 Jun 30 '15 at 02:31
  • Try `while(!inFile.eof() && loop < 40707)`. But this is terrible solution anyway. I'd use vector or at least dynamically reallocatable array. – Saraph Jun 30 '15 at 02:34
  • Also, if `loop` is equal to 40707, then your printing for loop is reading beyond the array boundary. – jxh Jun 30 '15 at 02:50
  • Also, `loop2 <= loop` is wrong regardless. It guarantees enumerating one-too-many strings. *Run this under a debugger*. You also forgot to ask a question. – WhozCraig Jun 30 '15 at 03:06
  • Which translation of Beowulf? Partial to the Seamus Heaney version, myself. [!eof only works by the grace of Odin and polite input](http://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong). `while(getline(inFile, line))` will do what you want no mus, no fuss, and catches all the file IO error cases !eof misses – user4581301 Jun 30 '15 at 03:07
  • 1
    `while(!eof` pattern is wrong, [see here](http://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong) – M.M Jun 30 '15 at 04:37

2 Answers2

1

Change your string array to:

std::vector<std::string> array;

Then you can read the file and copy into the vector simply as:

std::copy(std::istream_iterator<std::string>(inFile),
          std::istream_iterator<std::string>(),
          std::back_inserter(array));

EDIT: To read the file line-by-line, either define your own insert_iterator or do it like this:

std::string line;
while (getline(inFile, line))
    array.push_back(line);

Your code will then change to something like this

#include <bits/stdc++.h>
using namespace std;

int main() {
    vector<string> array;
    string line;
    ifstream inFile("beowulf.txt");
    if (!inFile.is_open()) {
        cerr <<  "Unable to open file" << endl;
        return 1;
    }
    while (getline(inFile, line))
        array.push_back(line);
    inFile.close();

    for (int i = 0; i < array.size(); ++i)
        cout << array[i] << endl;
    return 0;
}
Shreevardhan
  • 12,233
  • 3
  • 36
  • 50
1

Two potential error cases I can see right away.

Overrunning the end of the end of array. It can happen while reading because there is no guard on loop. If the array read was exactly 40707 lines it will happen while printing out when loop2 == loop. Either of these is probably the cause of the segfault. The recommended solution is to use C++'s std::vector because it will dynamically size itself to the input and automate iterating through the items stored.

The second error is less severe, but allows an infinite loop on an IO error. The end of file may never be reached if an error prevents reading a line and places the stream in an error condition. Unlikely to happen with getline, but a common occurrence with formatted reads.

Using most of the OP's solution:

#include <iostream>
#include <fstream>
#include <string>
#include <vector>

using namespace std;

int main()
{
    vector<string> array; // vector eliminates buffer overflow problem of string array.
    //int loop = 0; vector also eliminates the need for this counter
    //int loop2; and this one
    string line;
    ifstream inFile("beowulf.txt");
    if (inFile.is_open())
    {
        while(getline(inFile, line)) //exits if anything goes wrong with the file IO
        {
            array.push_back(line); // stores line in vector
        }
        inFile.close();
    }
    else cout <<  "Unable to open file" << endl;

// C++11 way    
    for (string const & str: array)
    { // iterates through the string automatically
        cout << str << endl;
    }
/* Old C++ way
    for (vector<string>::iterator str = array.begin(); 
         token != array.end();
         ++token)
    { // iterates through the string manually
        cout << *str << endl;
    }
*/
    return 0;
}
user4581301
  • 33,082
  • 7
  • 33
  • 54
  • Unless you're also fond of making needless copies, the "C++11 way" would be better off with `auto const& str : array` – WhozCraig Jun 30 '15 at 14:56
  • @WhozCraig With you on const and reference, but does `auto` let the compiler do anything smart that may outweigh the clarity of `string`? – user4581301 Jun 30 '15 at 18:39
  • Absolutely nothing `auto&&` would, but that is another subject entirely. – WhozCraig Jun 30 '15 at 18:40
  • @WhozCraig Gotcha. Going to leave it as string in this case to make what's happening as obvious as possible. Thanks! Now I'm going to screw around and see what happens when you const a universal reference. – user4581301 Jun 30 '15 at 18:52