Reading in a file into a Linked List - Segmentation Fault

Discussion in 'C++' started by Francis Bell, Jun 2, 2004.

  1. Francis Bell

    Francis Bell Guest

    Hello,

    I'm trying to read data from a file and then insert that into a linked
    list. The way I have it, the program compiles, however, I'm getting a
    segmentation fault error message when I run the program. I'm fairly new
    at the pointer business, and I'd appreciate any advice. Here's my code:

    int main()
    {
    ifstream fin;
    Fish fishy;
    FishLinkedList lake;

    fishy.readFishData(fin);
    while (!fin.fail()) {
    lake.insertAtHead(fishy);
    fishy.readFishData(fin);
    }

    fin.clear();
    fin.close();

    }

    void Fish::readFishData(istream &sin) {
    getline(sin, type, '/');
    sin >> weight;
    sin.ignore(80, '\n');
    }

    void FishLinkedList::insertAtHead(Fish fishy)
    {
    if (head != NULL) { // there is a list
    Fish *temp = head;
    Fish *insertMe = &fishy;
    temp->setNext(insertMe);
    } else {
    head = &fishy;
    }
    }

    Thanks!

    Frank
     
    Francis Bell, Jun 2, 2004
    #1
    1. Advertising

  2. "Francis Bell" <> wrote...
    > I'm trying to read data from a file and then insert that into a linked
    > list. The way I have it, the program compiles, however, I'm getting a
    > segmentation fault error message when I run the program. I'm fairly new
    > at the pointer business, and I'd appreciate any advice. Here's my code:
    >
    > int main()
    > {
    > ifstream fin;
    > Fish fishy;
    > FishLinkedList lake;
    >
    > fishy.readFishData(fin);
    > while (!fin.fail()) {
    > lake.insertAtHead(fishy);
    > fishy.readFishData(fin);
    > }
    >
    > fin.clear();
    > fin.close();
    >
    > }
    >
    > void Fish::readFishData(istream &sin) {
    > getline(sin, type, '/');
    > sin >> weight;
    > sin.ignore(80, '\n');
    > }
    >
    > void FishLinkedList::insertAtHead(Fish fishy)


    This function accepts its argument _by_value_. That is, when
    the function is called a _temporary_ object is created. It
    lives only until the function returns (and that's important).

    > {
    > if (head != NULL) { // there is a list
    > Fish *temp = head;
    > Fish *insertMe = &fishy;


    Here you take an address of the temporary value.

    > temp->setNext(insertMe);


    Here you store that address.

    > } else {
    > head = &fishy;


    Here you store the address of the temporary in the 'head'
    variable (that you hope will be valid next time around).
    It will NOT.

    > }
    > }


    You need either to set up your list (which, BTW, you never
    showed) to store the _values_ of those fishes, not _pointers_,
    or, if you insist on storing pointers, you gotta store the
    addresses of dynamically allocated objects.

    I recommend the first approach, it's easier. The second one
    will require some kind of clean-up (for freeing that memory)
    which in itself can take all fun out of studying pointers.

    Victor
     
    Victor Bazarov, Jun 2, 2004
    #2
    1. Advertising

  3. Francis Bell

    Francis Bell Guest

    Victor Bazarov wrote:
    > "Francis Bell" <> wrote...
    >
    >>I'm trying to read data from a file and then insert that into a linked
    >>list. The way I have it, the program compiles, however, I'm getting a
    >>segmentation fault error message when I run the program. I'm fairly new
    >>at the pointer business, and I'd appreciate any advice. Here's my code:
    >>
    >>int main()
    >>{
    >> ifstream fin;
    >> Fish fishy;
    >> FishLinkedList lake;
    >>
    >> fishy.readFishData(fin);
    >> while (!fin.fail()) {
    >> lake.insertAtHead(fishy);
    >> fishy.readFishData(fin);
    >> }
    >>
    >> fin.clear();
    >> fin.close();
    >>
    >>}
    >>
    >>void Fish::readFishData(istream &sin) {
    >> getline(sin, type, '/');
    >> sin >> weight;
    >> sin.ignore(80, '\n');
    >>}
    >>
    >>void FishLinkedList::insertAtHead(Fish fishy)

    >
    >
    > This function accepts its argument _by_value_. That is, when
    > the function is called a _temporary_ object is created. It
    > lives only until the function returns (and that's important).
    >
    >
    >>{
    >> if (head != NULL) { // there is a list
    >> Fish *temp = head;
    >> Fish *insertMe = &fishy;

    >
    >
    > Here you take an address of the temporary value.
    >
    >
    >> temp->setNext(insertMe);

    >
    >
    > Here you store that address.
    >
    >
    >> } else {
    >> head = &fishy;

    >
    >
    > Here you store the address of the temporary in the 'head'
    > variable (that you hope will be valid next time around).
    > It will NOT.
    >
    >
    >> }
    >>}

    >
    >
    > You need either to set up your list (which, BTW, you never
    > showed) to store the _values_ of those fishes, not _pointers_,
    > or, if you insist on storing pointers, you gotta store the
    > addresses of dynamically allocated objects.
    >
    > I recommend the first approach, it's easier. The second one
    > will require some kind of clean-up (for freeing that memory)
    > which in itself can take all fun out of studying pointers.
    >
    > Victor
    >
    >

    Thanks Victor! I kind of understand what you are saying...with one
    glaring exception. You said I need to set up my list to store the
    values of those fishes; I honestly that that was what I was doing with
    this insert function. I thought I was creating a list and inserting
    each fishy that gets read in into the front position of the list. I'm
    simply drawing a blank on what I need to do here.

    Frank
     
    Francis Bell, Jun 2, 2004
    #3
  4. "Francis Bell" <> wrote...
    > Victor Bazarov wrote:
    > > "Francis Bell" <> wrote...
    > >
    > >>I'm trying to read data from a file and then insert that into a linked
    > >>list. The way I have it, the program compiles, however, I'm getting a
    > >>segmentation fault error message when I run the program. I'm fairly new
    > >>at the pointer business, and I'd appreciate any advice. Here's my code:
    > >>
    > >>int main()
    > >>{
    > >> ifstream fin;
    > >> Fish fishy;
    > >> FishLinkedList lake;
    > >>
    > >> fishy.readFishData(fin);
    > >> while (!fin.fail()) {
    > >> lake.insertAtHead(fishy);
    > >> fishy.readFishData(fin);
    > >> }
    > >>
    > >> fin.clear();
    > >> fin.close();
    > >>
    > >>}
    > >>
    > >>void Fish::readFishData(istream &sin) {
    > >> getline(sin, type, '/');
    > >> sin >> weight;
    > >> sin.ignore(80, '\n');
    > >>}
    > >>
    > >>void FishLinkedList::insertAtHead(Fish fishy)

    > >
    > >
    > > This function accepts its argument _by_value_. That is, when
    > > the function is called a _temporary_ object is created. It
    > > lives only until the function returns (and that's important).
    > >
    > >
    > >>{
    > >> if (head != NULL) { // there is a list
    > >> Fish *temp = head;
    > >> Fish *insertMe = &fishy;

    > >
    > >
    > > Here you take an address of the temporary value.
    > >
    > >
    > >> temp->setNext(insertMe);

    > >
    > >
    > > Here you store that address.
    > >
    > >
    > >> } else {
    > >> head = &fishy;

    > >
    > >
    > > Here you store the address of the temporary in the 'head'
    > > variable (that you hope will be valid next time around).
    > > It will NOT.
    > >
    > >
    > >> }
    > >>}

    > >
    > >
    > > You need either to set up your list (which, BTW, you never
    > > showed) to store the _values_ of those fishes, not _pointers_,
    > > or, if you insist on storing pointers, you gotta store the
    > > addresses of dynamically allocated objects.
    > >
    > > I recommend the first approach, it's easier. The second one
    > > will require some kind of clean-up (for freeing that memory)
    > > which in itself can take all fun out of studying pointers.
    > >
    > > Victor
    > >
    > >

    > Thanks Victor! I kind of understand what you are saying...with one
    > glaring exception. You said I need to set up my list to store the
    > values of those fishes; I honestly that that was what I was doing with
    > this insert function.


    What does 'setNext' do? Doesn't it simply copy the address into some
    kind of 'next' member? So, it's not storing _values_, it's storing
    _addresses_ of values.

    > I thought I was creating a list and inserting
    > each fishy that gets read in into the front position of the list.


    You were? Well, you might have been, we couldn't see what you did
    there since you didn't post the code that actually does the storing.

    > I'm
    > simply drawing a blank on what I need to do here.


    Your 'FishLinkedList' is based on storing pointers to 'Fish' objects
    (at least that's what it seems to be doing judging from the source,
    since you didn't provide the definition and implementation of the
    FishLinkedList class). Supposedly, it looks something like this:

    class FishLinkedList {
    Fish *head; // you have a pointer here
    FishLinkedList *next;
    public:
    FishLinkedList() : head(NULL) {}
    void setNext(Fish* pfish);
    };

    What you need to do here is something like

    class FishLinkedList {
    Fish value; // actual value, not a pointer
    bool has_head;
    FishLinkedList *next;
    public:
    FishLinkedList() : has_head(false) {}
    void insert(Fish newvalue);
    };

    The problem, of course, is that it's basically impossible to value-
    based storage unless you allocate all the storage before the program
    begins:

    class FishLinkedList {
    Fish storage[100];
    int tail; // where to insert
    public:
    FishLinkedList() : tail(0) {}
    void insert(Fish newvalue) {
    if (tail == 100) throw "list is full";
    storage[tail++] = newvalue;
    }
    };

    simply because if you don't, you have to fall back on dynamic
    allocation of the storage, which presents the same set of problems
    that you already faced: pointers.

    Find a good book on data structures, a good book on C++ (which will
    probably contain some common data structures with examples), and
    give it a good read. It is really impossible to present you with
    all the material on pointers, dynamic allocation, linked lists,
    lifetime of objects, etc. in one newsgroup conversation.

    V
     
    Victor Bazarov, Jun 2, 2004
    #4
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Henk
    Replies:
    4
    Views:
    854
  2. fool
    Replies:
    14
    Views:
    518
    Barry Schwarz
    Jul 3, 2006
  3. scythemk
    Replies:
    4
    Views:
    936
    Daniel T.
    Feb 21, 2006
  4. utab
    Replies:
    7
    Views:
    3,380
    Dietmar Kuehl
    Mar 14, 2006
  5. joshd
    Replies:
    12
    Views:
    679
    John Carson
    Oct 2, 2006
Loading...

Share This Page