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;
}
  • Have you tried boiling it down to a complete minimal example? – Beta Apr 20 '13 at 02:15
  • what is the value of `listLength` have you seen? – taocp Apr 20 '13 at 02:15
  • 1
    My guess is that `listLength = fileName.tellg();` is returning `-1` since `>=0` will not seg fault. – Shafik Yaghmour Apr 20 '13 at 02: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. – Matt Phillips Apr 20 '13 at 02: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. – Muscles Apr 20 '13 at 02:24
  • `new string[listLength]` eeewwwwww.... – Ed S. Apr 20 '13 at 03:04

1 Answers1

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.

Community
  • 1
  • 1
Shafik Yaghmour
  • 154,301
  • 39
  • 440
  • 740
  • It is valid to create an array with zero elements. Not valid, of course, to create one with a negative number. – Muscles Apr 20 '13 at 02:30
  • 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 – Wilmer E. Henao Apr 20 '13 at 02:38