0

My friend is writing a text-based game and asked me to look at this code that was crashing. I debugged it and it was getting a seg fault when creating a dynamic array. I'm not sure exactly why, I recommended he just avoid pointers altogether and use a vector so hopefully that will solve his problem but I'm curious as to what exactly is going wrong here. Here's his code:

#include <iostream>
#include <fstream>
#include <string>
#include <cstdlib>
#include <ctime>

using namespace std;

class nation
{
    public:
    void init();
    string genName();
    string getName();

    private:
    string myName;
    int* myBorderPoints;
};

string nation::getName()
{
    return myName;
}

string nation::genName()
{
    int listLength = 0, listPos = 0, listRand = 0;
    string nameToGen = "";
    string* namePartList;
    ifstream fileName;
    fileName.open("NamePart1.txt");
    listLength = fileName.tellg();
    namePartList = new string[listLength]; // Seg fault here
    while (fileName.good())
    {
        while (!fileName.eof())
        {
            getline(fileName,namePartList[listPos]);
            listPos += 1;
        }
    }
    listRand = rand() % listLength;
    nameToGen += namePartList[listRand];
    fileName.close();
    listLength = 0;
    listPos = 0;
    listRand = 0;
    nameToGen = "";
    fileName.open("NamePart2.txt");
    listLength = fileName.tellg();
    namePartList = new string[listLength];
    while (fileName.good())
    {
        while (!fileName.eof())
        {
            getline(fileName,namePartList[listPos]);
            listPos += 1;
        }
    }
    listRand = rand() % listLength;
    nameToGen += namePartList[listRand];
    fileName.close();
    return nameToGen;
}

void nation::init()
{
    srand(time(NULL));
    myName = genName();
}

int main()
{
    nation testNation;
    testNation.init();
    cout << testNation.getName();
    return 0;
}
6
  • Have you tried boiling it down to a complete minimal example? Commented Apr 20, 2013 at 2:15
  • what is the value of listLength have you seen? Commented Apr 20, 2013 at 2:15
  • 1
    My guess is that listLength = fileName.tellg(); is returning -1 since >=0 will not seg fault. Commented Apr 20, 2013 at 2:15
  • @ShafikYaghmour Close--he's calling it after just opening the stream, without having read anything. Likely ==0. OP probably thinks that tellg is giving him file length. I'd say write it up as the answer. Commented Apr 20, 2013 at 2:19
  • fileName.open() may be failing, because the file may not exist, or you may not have the right permissions, or the file is locked by another process. If the open fails, I would expect the tellg() to return -1, and that makes your string allocation crash. Commented Apr 20, 2013 at 2:24

1 Answer 1

1

You are calling tellg:

listLength = fileName.tellg();

without having read anything, which depending on whether the file was opening successfully or not will return 0 or -1 and so you will have this called:

namePartList = new string[listLength]

with a probably a undesirable value. I am pretty sure it is returning -1 since allocating a zero sized should be ok.

This also applies later on the code as well, going with std::vector probably makes more sense.

Sign up to request clarification or add additional context in comments.

2 Comments

It is valid to create an array with zero elements. Not valid, of course, to create one with a negative number.
Same is going to happen with NamePart2.txt If you want to solve this type of issues you may want to introduce an if else. Like if(fileName) then continue... or just throw an exception

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.