Go Back   Hardware Canucks > SOFTWARE > O/S's, Drivers & General Software

    
Reply
 
LinkBack Thread Tools Display Modes
  #1 (permalink)  
Old November 22, 2010, 03:45 PM
xMechanixxx's Avatar
Rookie
 
Join Date: Oct 2009
Location: Calgary, Alberta
Posts: 16

My System Specs

Default Linked List Segmentation Fault, Please Help!

Hey everyone, I'm writing a program as an assignment for school and I though I had worked out all the bugs until I decided to call my copy constructor. The program is interactive (CLI) which basically has two moduals: a .h and .cpp file for the LList class and a .h and .cpp file for the structure of the program and also a 3rd cpp file just for main(). It is suppose to be a program for a hydropower engineering company (fake company) in which the LList nodes hold data for their annual water flow in a river(year and flow). Here is some insight on the class:

Code:
//list.h
struct ListItem {
  int year;
  double flow;
};

struct Node {
  ListItem item;
  Node *next;
};

class FlowList {
public:
  FlowList();
  // PROMISES: Creates empty list

  FlowList(const FlowList& source);
    // REQUIRES: source refers to an existing List object
    // PROMISES: create the copy of source

  ~FlowList();
    // PROMISES: Destroys an existing list.

  FlowList& operator =(const FlowList& rhs);
  // REQUIRES: rhs refers to an existing FlowList object
  // PROMISES: the left hand side object becomes the copy of rhs

//....Other member functions

private:
  // always points to the first node in the list.
  
  Node *headM;
  // Initially is set to NULL, but it may point to any node.

  Node *cursorM;
  //For node manipulation within interactive CLI

  void copy(const FlowList& source);

  void destroy();
I belive the memory leak or collision is taking place somewhere within the copy function but cant pin point where.

Code:
//list.cpp

FlowList::FlowList() : headM(0), cursorM(0) {}

FlowList::FlowList(const FlowList& source) 
{
  copy(source);
}

FlowList::~FlowList() 
{
  destroy();
}

FlowList& FlowList::operator =(const FlowList& rhs) 
{
  if (this != &rhs)
  {
    destroy();
    copy(rhs);
  }
  return (*this);
}

//...more function definitions

void FlowList::copy(const FlowList& source)
{
  if (source.headM == NULL) 
  {
    headM = NULL;
    cursorM = NULL; 
    return;
  }

  Node* new_node = new Node;
  assert(new_node);
  new_node->item.year = source.headM->item.year;
  new_node->item.flow = source.headM->item.flow;
  new_node->next = NULL;    
  headM = new_node;

  Node* thisptr = new_node;

  for(Node* srcptr = source.headM->next; srcptr != NULL; srcptr = srcptr->next)
  {
    new_node = new Node;
    assert(new_node);
    new_node->item.year = srcptr->item.year;
    new_node->item.flow = srcptr->item.flow;
    new_node->next = NULL;
    thisptr->next = new_node;
    thisptr = thisptr->next;
  }   
  
}

void FlowList::destroy()
{
  Node* ptr = headM;
  Node* post = headM->next;

  while (ptr != NULL)
  {
    delete ptr;
    ptr = post;
    if (post)
      post = ptr->next;
  }
  headM = NULL;
}
The program works fine if I create a FlowList object, fill it with data from a .dat file; i can then manipulate the data within the program (display, perform calculations, add to the list, remove from list and save data to file) but program crashes (segmentation fault) if I create another FlowList object (within main.cpp).
Sorry for the lengthy post but any help would be really appreciated.
__________________

Reply With Quote
  #2 (permalink)  
Old November 23, 2010, 06:23 PM
Rookie
 
Join Date: Jun 2007
Location: The City Above Toronto
Posts: 15
Default

You've messed in the destructor of the FlowList.

Quote:
void FlowList::destroy()
{
Node* ptr = headM;
Node* post = headM->next;

while (ptr != NULL)
{
delete ptr;
ptr = post;
if (post)
post = ptr->next;
}
headM = NULL;
}
You're assigning *post with unchecked headM and get a segmentation fault because headM could be NULL.
And it is NULL if you just declare a variable and don't use it.

This simple one-line main will get you a segmentation fault.
Code:
int main() { FlowList a; return 0; }
You'd better have it like this:

Code:
void FlowList::destroy()
{
  Node* ptr = headM;
  Node* post;

  while (ptr != NULL)
  {
    post = ptr->next;
    delete ptr;
    ptr = post;
  }
  headM = NULL;
}
Reply With Quote
  #3 (permalink)  
Old November 23, 2010, 08:43 PM
xMechanixxx's Avatar
Rookie
 
Join Date: Oct 2009
Location: Calgary, Alberta
Posts: 16

My System Specs

Default

Thanks for the reply but the gents over at stackoverflow got back a bit quicker. They pointed out the same thing as you so I went ahead and modified it to

Code:
void FlowList::destroy()
{
  cursorM = NULL;

  if (headM == NULL)
    return;

  Node* ptr = headM;
  Node* post = headM->next;

  while (ptr != NULL)
  {
    delete ptr;
    ptr = post;
    if (post)
      post = ptr->next;
  }
  headM = NULL;
}
Your way does look a bit cleaner than my brute force method tho. Thanks anyway
__________________

Reply With Quote
  #4 (permalink)  
Old November 23, 2010, 09:32 PM
ZZLEE's Avatar
Hall Of Fame
F@H
 
Join Date: May 2009
Location: KANATA
Posts: 2,132

My System Specs

Default

one off my uncles does this type off thing. If need some real data look of flow rates on any river in Ontario on Enviorment Canada I think.

Edit: Link http://www.google.ca/url?sa=t&source...v9GjLg&cad=rja
__________________
"EVGA hunted down the last dozen or so expats living in Karachi." SKY
Reply With Quote
Reply


Thread Tools
Display Modes

Similar Threads
Thread Thread Starter Forum Replies Last Post
EVGA 680i fault codes?? catguy Troubleshooting 7 September 22, 2009 06:40 PM
is 520W enough? (it's the fault of the Q6600) nico Power Supplies 5 July 26, 2007 12:13 PM