Handwritten linked list is segfaulting and I don't understand why

Hi I was working on a bit of fun, making an interface to run gnuplot from within c++, and for some reason the my linked list implementation fails.

The code below fails on the line plots->append(&plot). Stepping through the c开发者_JS百科ode I discovered that for some reason the destructor ~John() is called immediately after the constructor John(), and I cannot seem to figure out why.

The code included below is a stripped down version operating only on Plot*. Originally I made the linked list as a template class. And it worked fine as ll<int> and ll<char*> but for some reason it fails as ll<Plot*>.

Could youp please help me figure out why it fails? and perhaps help me understand how to make it work?

In advance: Thanks a heap!

//B2S

#include <string.h>

class Plot{
  char title[80];
public:
  Plot(){  }
};

class Link{
  Plot* element;
  Link* next;
  Link* prev;
  friend class ll;
};

class ll{
  Link* head;
  Link* tail;
public:
  ll(){
    head = tail = new Link();
    head->prev = tail->prev = head->next = tail->next = head;
  }
  ~ll(){
    while (head!=tail){
      tail = tail->prev;
      delete tail->next;
    }
    delete head;
  }
  void append(Plot* element){
    tail->element = element;
    tail->next = new Link();
    tail->next->prev = tail;
    tail->next = tail;
  }
};

class John{
  ll* plots;
public:
  John(){
   plots= new ll();
  }
  ~John(){
    delete plots;
  }
  John(Plot* plot){
    John();
    plots->append(plot);
  }
};   

int main(){
  Plot p;
  John k(&p);
}

The statement:

 John();

Just constructs a new nameless John which is created and immediately destroyed. There is no way (in the current version of C++) to call a constructor from another constructor or on an already constructed object.

This means that the plots member of your John called k is never being initalized, hence the crash when you call append from the John constructor.

[As Neil Butterworth comments, if you had actually constructed an ll instance there are other issues with it, this is just the most immediate problem.]

Because you can't call alternate constructors like that in C++. In...

John(Plot* plot){
  John();
  plots->append(plot);
}

Your first line constructs a new John object using the default constructor, then throws the result away. It then calls append on plots (which as it has not been set to anything, is junk). Refactor the real plots initialisation code into a private method, then call this init method in both constructors before doing anything else.

head = tail = new Link();

Here you assign head and tail to point to the same link object. Since you never reassign either of them, that means that any time you change any property of head (e.g. head.next = foo), the same change is applied to tail.

Meaning head and tail will always have the same element, the predecessor and the same successor. Obviously this does not lead to the intended results.

  Plot p;
  John k(&p);

Use this instead:

  Plot * p = (Plot*) new Plot;
  John k(p);

This code is broken, since once p goes out of scope, k's reference to it will die. This is not the bug that caused the segfault, but it will probably cause a segfault later on once you fix your other bugs.

You will need to delete p later.