Reading in a file into a Linked List - Segmentation Fault

[Francis Bell]

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

 

[Victor Bazarov]

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

 

[Francis Bell]

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).




Here you take an address of the temporary value.




Here you store that address.




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

 

[Victor Bazarov]

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