This is C++ code for linked list. after putting value of ch=1 it will add a node with some value but after putting one value program shuts unexpectdly

Question

#include <iostream>
using namespace std;

class Node{
    public:
        int data;
        Node *next;
};
int main()
{
    Node *head=NULL;
    Node *temp;
    Node *nodeToAdd;
    int ch,val,flag=1;
    while(flag){
        cout<<"\n1.Add   2.Traverse   3.EXIT\n";
        cin>>ch;
        switch(ch){
            case 1:
                nodeToAdd=new Node();
                cout<<"Enter value - ";
                cin>>val;
                cout<<endl;
                nodeToAdd->data=val;
                temp=head;
                while(temp->next!=NULL){
                    temp=temp->next;
                }
                temp->next=nodeToAdd;
                break;
            case 2:
                temp=head;
                while(temp!=NULL){
                    cout<<temp->data;
                    temp=temp->next;
                }
                break;
            case 3:
                flag=0;
                break;
        }
    }
    return 0;
}

Please tell me my mistake that why progamm is terminating unexpectdly after putting value of ch=1. After putting value of ch 1 it takes one input as value and after it terminates unexpectdly.

Please write a test that doesn't depend on interactive input. Interactive unit tests aren't really useful, and you might as well rule out your input loop as a possible cause. — Useless, Aug 04 '21 at 16:48
Consider moving those cases into their own functions. Put too much responsibility on one function and you'll have a much harder time proving that the behaviour of the function is correct. — user4581301, Aug 04 '21 at 16:59

Vlad from Moscow · Answer 1 · 2021-08-04T16:53:22.590

4

This code snippet

temp=head;
while(temp->next!=NULL){
    temp=temp->next;
}
temp->next=nodeToAdd;

already invokes undefined behavior because initially the pointer head can be a null pointer and dereferencing the null pointer in the expression temp->next!=NULL yields undefined behavior.

Also you forgot to set the data member next of the added node to nullptr.

Change the code snippet under the label case 1: the following way

case 1:
    cout << "Enter value - ";
    cin >> val;
    cout << endl;

    nodeToAdd = new Node { val, nullptr };

    if ( head == nullptr )
    {
        head = nodeToAdd;
    }
    else
    { 
        temp = head;
        while ( temp->next ) temp = temp->next;
        temp->next = nodeToAdd;
    }
    break;

edited Aug 04 '21 at 16:53

answered Aug 04 '21 at 16:49

Vlad from Moscow

301,070
26
186
335

For OP's benefit: this is one of the reasons why adding a sentinel node is sometimes useful. It removes the need to special-case empty lists. – Useless Aug 04 '21 at 16:51
And you can also use [the pointer-to-pointer trick](https://stackoverflow.com/a/59779376/4581301) to abstract away the naming difference between `head` and any other `next` pointer to eliminate the need for special tests. – user4581301 Aug 04 '21 at 16:58

score 2 · Accepted Answer · answered Aug 04 '21 at 16:50

Before doing:

            temp=head;
            while(temp->next!=NULL){

you need to check whether head is NULL. Like

        if (head == NULL)
        {
            // ... Do stuff to insert first element
        }
        else
        {
            // Add to end of list
            temp=head;
            while(temp->next!=NULL){
            ...

This is C++ code for linked list. after putting value of ch=1 it will add a node with some value but after putting one value program shuts unexpectdly

2 Answers2