Singly Linked List- Pointer issues and segfaults

[jrbobdobbs83]

Firstly, this is my first posting here. I've read through the guidelines, and have hopefully managed to follow them. If not, please advise! :)

Problem: I have written a class for a singly linked list, which uses a separate Node class. When my test program accesses a function that iterates through the linked list (Search function, Print list function); if the function goes to the end of the list, the program segfaults. I'm assuming that there is an issue with the last Node not correctly pointing to NULL, and I suspect that I may be dereferencing the null ptr somehow, but I cannot locate the error. I've included what I *think* are the relevant bits; if more is needed, please let me know!

Node header:

class Node
     {
     public:
          Node(const int& newdata = 0, Node* newlink = NULL); //constructor
          
          void setdata(const int newdata){data = newdata;} //Sets data 
          void setlink(Node* newlink) {link = newlink;}  //Set link
          
          int getdata() {return data;}  
          Node* getlink() {return link;}

          
          private:
               int data;
               Node* link;
     };
     void insert(Node*& headptr, const int entry);

Node implementation:

Node::Node(const int& newdata, Node* newlink)
     {
          data = newdata;
          link = NULL;
     }
     
     
     void insert(Node*& headptr, const int entry)
     {
          headptr = new Node(entry);
     }

Linked list constructor:

List::List()
     {
          used = 0;
          Node* headptr;
          headptr = NULL;
         // cout << "headptr link:  " << headptr->getlink() << endl;
     }

Insert function:

void List::inserthead(const int& entry)
     {
          if (headptr == NULL)
          {
               insert(headptr, entry);
          }
          else
          {
               Node* tempptr = new Node();
               tempptr->setlink(headptr);
               tempptr->setdata(entry);
               headptr = tempptr;
          }
          used++;
          cout << "headptr link:  " << headptr->getlink() << endl;
          return;
     }

Print function:

void List::printList()
     {
          assert (headptr != NULL);
          Node* tempptr;
          tempptr = headptr;
          int counter = 1;
          while (tempptr != NULL)
          { 
               cout << "Node " << counter << ":  " << tempptr->getdata() << endl;
               tempptr = tempptr->getlink();
               counter++;
          }
          return;
     }

Expected output (Nodes created holding 11, 22, 33, 44, 55 in that order)

Node 1: 55
Node 2: 44
Node 3: 33
Node 4: 22
Node 5: 11

Actual output:
Node 1: 55
Node 2: 44
Node 3: 33
Node 4: 22
Node 5: 11
Node 6: 1267779745
*segmentation fault here*

Sorry for the long post, and any help is appreciated!

[JLBorges]

#include <iostream>

struct list
{
    list() noexcept : head(nullptr) {}

    ~list() { while( !empty() ) remove_head() ; }

    // non-copyable
    list( const list& ) = delete ;
    list& operator= ( const list& ) = delete ;

    bool empty() const { return head == nullptr ; }

    void print() const
    {
        std::cout << "[ " ;
        for( node* n = head ; n != nullptr ; n = n->next ) std::cout << n->data << ' ' ;
        std::cout << "]\n" ;
    }

    // push_front
    void insert_head( int v ) { head = new node( v, head ) ; }

    void remove_head() noexcept // pop_front
    {
        if( !empty() )
        {
            node* temp = head ;
            head = head->next ;
            delete temp ;
        }
    }

    private:

        struct node
        {
            explicit node( int v = 0, node* n = nullptr ) noexcept : data(v), next(n) {}
            int data ;
            node* next ;
        };

        node* head ;
};

int main()
{
    list lst ;
    for( int v : { 11, 22, 33, 44, 55, 66, 77, 88, 99 } )
    {
        lst.insert_head(v) ;
        lst.print() ;
    }
}

http://coliru.stacked-crooked.com/a/0b5eb226a2d83bc3

[jrbobdobbs83]

Hmm. I found several ideas in this that weren't implemented in my code. I added the empty() function, restructured my inserthead function, added a separate function specifically for generating the first node, and fixed my destructor. I'm guessing, and I hope that you (or somebody) can confirm that this code:

void List::inserthead(int newdata) {headptr = new Node(newdata, headptr);}


     void List::insertnew(const int& entry)
     {
          if (headptr == NULL)
          {
               inserthead(entry);
          }
          else
          {
               Node* tempptr = new Node();
               tempptr->setlink(headptr);
               tempptr->setdata(entry);
               headptr = tempptr;
          }
          used++;
          cout << "headptr link:  " << headptr->getlink() << endl;
          return;
     }

is what actually fixed my issue?

Also, thank you!