1. Not finding help here? Sign up for a free 30min tutor trial with Chegg Tutors
    Dismiss Notice
Dismiss Notice
Join Physics Forums Today!
The friendliest, high quality science and math community on the planet! Everyone who loves science is here!

Please help me resolve this program keeps crashing

  1. Dec 18, 2012 #1
    I'm writing a program in C++ that reads integer values from a file. I am trying to run it but it either keeps crashing or running uncontrollably. Here is the function where the problem is:

    Code (Text):
    void createList(intNode*& intList)
    {
        intNode* lastInt; //points to last integer in file
        lastInt = NULL;
        int fileInt; //int read from input file
        ifstream intInputFile;

        intInputFile.open("intInput.txt");
        if (intInputFile.is_open())
        {
            cout << "intInput.txt open successful" << endl;
        }
        else
        {
            cout << "intInput.txt open unsuccessful" << endl;
        }
        intInputFile >> fileInt;
        cout << "check" <<endl;
        while(!intInputFile.eof())
        {
            intNode* anotherInt;
            anotherInt = new intNode;
        [COLOR="Red"] [B]  if(intList==NULL)
           {
               intList = anotherInt;
               lastInt = anotherInt;
               lastInt->nextNode = new intNode;
           }
           else
           {
              lastInt->nextNode = new intNode;
              lastInt = lastInt->nextNode;
              lastInt->nextNode = NULL;
           }
           lastInt->intValue = fileInt;
           intInputFile >> fileInt;
           cout << "good" <<endl;[/B][/COLOR]
        }
        cout <<"whats the sitch"<<endl;
        intInputFile.close();
        cout << "List created from input file" << endl;
    }
    The area in bold red is the problem, but I can't figure out what exactly the issue is. Any help please?

    Thanks.
     
  2. jcsd
  3. Dec 19, 2012 #2
    There are several problems but the one that is causing the crash is this:

    1. Enter the function with intList != NULL
    2. lastInt = NULL
    3. In the else part of your section in red, lastInt->nextNode = new intNode

    will cause a segmentation fault.


    You've got lots of problems with memory leaks -- you may want to rethink how you form your linked list.

    In each iteration of your while loop you create a new intNode and assign it to anotherInt. It doesn't actually get used anywhere (except possibly one time in the first part of the if block in red). Each time through the loop, anotherInt is allocated another intNode and a pointer to the previous allocation is lost forever.

    if intList==NULL, the first part of the red if block is executed once. Inside there you create a list of one node that will be assigned with the read value after the if block is executed. But you also create another intNode and chain it to the end(?) without advancing the lastInt pointer. This extra node will be lost forever in the next loop iteration when the other part of the if block is executed.

    Anyway have a look and see if those problems can be fixed :)
     
  4. Dec 19, 2012 #3
    Thank you, what do you mean by segmentation fault? Sorry, I'm not much of a programmer so I couldn't really understand everything you said, can you please clarify? I've tried drawing out the problem, and this it's basically same code, just a bit different:

    Code (Text):
    while(!intInputFile.eof())
        {
            intNode* anotherInt;
            anotherInt = new intNode;
           if(intList==NULL)
           {
               intList = anotherInt;
               lastInt = anotherInt;
               lastInt->nextNode = NULL;
               lastInt->nextNode = new intNode;
           }
           else
           {
              lastInt = lastInt->nextNode;
              lastInt->nextNode = NULL;
              lastInt->nextNode = new intNode;
           }
           lastInt->intValue = fileInt;
           intInputFile >> fileInt;
           cout << "good" <<endl;
        }
    I added the cout << "good" part to see if it will reach there, and then my program just kept saying "good" uncontrollably. It seemed like this code will work fine, because intList is initially set to NULL, do you think it's an issue with my file?
     
  5. Dec 19, 2012 #4
    This is because the cout is inside the while loop. Each time an integer is read, that line is executed.

    Well it's not initially set to NULL :) You're depending on the caller to set it to NULL for you when you should be doing it inside the function. Someone who is unfamiliar with the code might think your function is able to append integers to an existing list because the int*& in the parameter is suggesting that. In other words, it is a potential bug for someone else down the line.

    A better way is to return the list as return value and not accept a parameter. Eg, like this:

    intNode *createList(void)
    {
    ...
    }

    Then it's very clear to anyone else reading your code that a new list is being returned. Maintenance is a big issue in the real world and a lot of software engineering is about being defensive.

    A segmentation fault is accessing memory that does not belong to the process (program) or accessing memory in a way that is disallowed (eg trying to write to a read-only segment). In modern computers, this is normally detected by the hardware and will cause an interrupt that the operating system will usually respond to be creating a core dump and terminating the program.

    In the scenario I gave you, you had lastInt=NULL, which is usually 0. The following assignment lastInt->nextNode=new intNode would have tried to store the pointer to the new intNode at an address close to 0, which is well outside the address range allowed for variables (on purpose and this is set up by the compiler/linker/operating system to detect such errors) and would trigger a segmentation fault.

    In the 'if' above, you are always creating one extra intNode for the next integer, which means when the function returns you will always have one extra void intNode at the end of the list. You should only create intNodes as you need them, when you have an integer to store.

    See if it still crashes after these things are fixed.
     
    Last edited: Dec 19, 2012
  6. Dec 22, 2012 #5
    Thank you!
     
Know someone interested in this topic? Share this thread via Reddit, Google+, Twitter, or Facebook




Similar Discussions: Please help me resolve this program keeps crashing
Loading...