C++ Linked List Simple Program Crashing

Question

I made a Linked-List with only an Insert Node function and a Print function, but It doesnt work.

#ifndef LIST_H_
#define LIST_H_
#include <iostream>
using namespace std;

struct Node{
    int data;
    Node* next;
};

class List{

private:
    Node* head;

public:
    List(){
        head = NULL;
    }

    void insertEnd(int d){
        Node* newNode = new Node;
        newNode->next = NULL;
        newNode->data = d;

        if (head == NULL){
            head = newNode;
            return;
        }

        Node* cu = head;

        while (cu != NULL)
            cu = cu->next;
        cu->next = newNode;
    }

    void printList(){
        Node* temp = new Node;
        temp = head;
        while (temp != NULL){
            cout << temp->data << ", ";
            temp = temp->next;
        }
    }

};

And my main function:

#include <iostream>
#include "List.h"
using namespace std;

int main(){

List list1;

list1.insertEnd(1);
list1.insertEnd(2);
list1.insertEnd(3);

//list1.printList();

return 0;

}

This program works if I only insert one node, but if I do anything else it crashes and doesn't give me any error indications or anything.

I've checked on several sites if my pointers were doing the right thing, and I think they are, but what is going wrong here...?

Edit: fixed the problem... in while loop should have been

while (cu->next != NULL)

It most certainly will give you an error. If you're running this via a bat, add a pause at the end so you can read the error. — Carcigenicate
– Carcigenicate, Commented Jun 18, 2017 at 22:59
for example Node* cu = new Node; cu = head; - think this exist sense ? — RbMm
– RbMm, Commented Jun 18, 2017 at 23:00
insertEnd, printList() - completely wrong. Node* temp = new Node; temp = head; this is c++ ? — RbMm
– RbMm, Commented Jun 18, 2017 at 23:01
You might want to use a debugger if can't find out by yourself what happens. FYI cu->next = newNode will always crash because your while loop doesn't check the proper condition. it should check while(cu->next != null). Your current implementation somewhat ensures your pointer is a nullptr before breaking out of the while loop... — AlexG
– AlexG, Commented Jun 18, 2017 at 23:02
cu->next = newNode; after while (cu != NULL) - so cu - always 0 here — RbMm
– RbMm, Commented Jun 18, 2017 at 23:03

Vlad from Moscow · Accepted Answer · 2017-06-18 23:44:41Z

2

The function insertEnd is wrong.

After this loop

    while (cu != NULL)
        cu = cu->next;

the pointer cv is equal to NULL. As result the following statement

    cu->next = newNode;

leads to undefined behavior.

The simplest way to append a node to the list is the following

void insertEnd( int d )
{
    Node **last = &head;

    while ( *last != nullptr ) last = &( *last )->next;

    *last = new Node { d, nullptr };
}

The function has only three lines.:)

Take into account that this statement

    Node* temp = new Node;

in the function printList does not make sense and is a reason for a memory leak.

edited Jun 18, 2017 at 23:44

answered Jun 18, 2017 at 23:17

Vlad from Moscow

313k27 gold badges204 silver badges358 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Jayson Boubin · Accepted Answer · 2017-06-18 23:09:08Z

void insertEnd(int d){ 
        Node* newNode = new Node; 
        newNode->next = NULL; 
        newNode->data = d; 

        if (head == NULL){ 
            head = newNode; 
            return; 
        } 

        Node* cu = head; 

        while (cu->next != NULL) 
            cu = cu->next; 
        cu->next = newNode; 
}

This function will do the trick. You had a few relatively simple problems. First, you were trying to make a copy of head to iterate over your list. Instead of assigning that to a dummy pointer, you were allocating new memory, assigning that new memory to your dummy pointer, and then assigning your head pointer to that dummy pointer. This will create a memory leak, as you will never be able to delete that memory if you lose track of it. I changed this:

Node* cu = new Node;
cu = head

to this:

Node* cu = head;

Second, Your segmentation fault comes when you check if cu is not null in your while loop. You are setting cu to cu->next in your loop, then checking if cu is null. if cu is null, then you assign cu->next to your new node. Your null pointer doesn't reference any memory, so trying to reference its members gives you a segment fault. You want to access the last possible valid pointer in your linked list. To do that, you check to see if cu->next is null. I changed this:

while (cu != NULL)
            cu = cu->next;

To This:

while (cu->next != NULL) 
            cu = cu->next;

Manjunath · Accepted Answer · 2017-06-18 23:05:21Z

1

Your while loop is incorrect. Change it to cu->next from cu

while (cu->next != NULL)

answered Jun 18, 2017 at 23:05

Manjunath

5612 gold badges7 silver badges17 bronze badges

Collectives™ on Stack Overflow

C++ Linked List Simple Program Crashing

3 Answers 3

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related