C++ Linked list, program stuck when calling the next pointer in the list

Question

I have made a linked list with c++. I have no idea when trying to point to the next element of the list, the program stops.

My Node Class as follows:

class Node {
friend class List;
private :
Node* next;
public:
int value;
Node()
{
    value = 0;
    next = nullptr;
}

Node(int data)
{
    this->value = data;
    this->next = nullptr;
}
};

My List class has next and delete methods. Whenever there is calling for the next attributes in the Node class. The program stucks. For my List Class I made them as follows:

class List {
private:
Node* head;
public:
    List ()
    {
        head = 0; // create an empty list
    }
    ~List ()
    {
        delete head; // clean up the list and all nodes
    }

    Node* first () const
    {
        return head;
    }

    Node* next(const Node* n) const{
        return n->next;
    }

    void append(int i)
    {
        Node* newNode = new Node(i);

        if (head == nullptr){
            head = newNode;
        }
        else
        {
            Node *ptr = head;
            // the loop sets ptr to last node of the linked list
            while (ptr->next != nullptr){
                ptr = ptr->next;
            }
            // ptr now points to the last node
            // store temp address in the next of ptr
            ptr->next = newNode;

            }
    }


    void insert(Node* n, int i)
    {
        Node *ptr = head;
        Node *newNode = new Node(i);
        newNode->next = n;

        if(n==head)
        {
            head = newNode;
        }
        else
        {
            while(ptr->next != n)
            {
                ptr = ptr->next;
            }
            ptr->next = newNode;
        }
    }


    void erase( Node* n)
    {
        Node *ptr = head;
        Node *before ;

        if (n->next == nullptr)
            {
                free(n);
                return ;
            }

        if(head == n)
        {
            head = n->next;
            free(n);
            return ;
        }
        else
            {

            while(ptr!= n)
                {
                    before = ptr;
                    ptr = ptr->next ;
                }
                before->next = ptr;
                free(ptr);
                free(n);
                return ;
            }
    }

    void printLst()
    {
        while(head != nullptr)
        {
            std::cout<<head->value<<" ";
            head = head->next;
        }
    }
};

and to get a full vision of program. I made the main function very simple:

int main()
{
List list;

list.append(55);
list.append(50);
list.append(20);
list.append(30);


list.insert(list.first(), 22);
list.insert(list.first(), 87);
list.printLst();

list.erase(list.first());
list.printLst();
}

Any suggestions ?

the reason this code doesnt work is because there is no append method in the class, and no insert either — pm100, May 26 '22 at 01:04
I have already made that, it works fine, but I didn't want to write it in order to make my code shorter. — ama989, May 26 '22 at 01:05
and how do you know its not the cause of the bug. Also what does 'stops' and 'stuck' mean. Not givving us that code means we cant run your code ourselves — pm100, May 26 '22 at 01:06
Why are you using `free`? Are you allocating your objects with `malloc`? Note that `malloc` does _not_ initialize your objects. You could use placement-new for this, but that's an advanced feature for people who know what they're doing. You should be using `new` and `delete`. Your `erase` function should also be doing the `head` test first. — paddy, May 26 '22 at 01:07
If you can't replicate the problem without `append` and `insert`, then we can't debug the problem without them. — David Schwartz, May 26 '22 at 01:10
The number of folks who thought, "That code works! I don't need to include it!" is staggering. And they're wrong so often that it's not funny. — user4581301, May 26 '22 at 01:11
ok now you posted insert and append. We see that you are correctly allocating nodes via new, first thing, change your code to use 'delete' not 'free' to delete the nodes. Try agan to see if that fixes your problem — pm100, May 26 '22 at 01:14
Rethink `free(ptr);`. And I don't just mean use `delete ptr;` instead. Ask yourself, "Why am I doing this? What is at `ptr` that I want destroyed? Do I really want to destroy it?" — user4581301, May 26 '22 at 01:16
Rethink `if (n->next == nullptr)`. Ask yourself, "How do I know that there is an `n` on which I can check the `next` member?" — user4581301, May 26 '22 at 01:22
Side note: See WhozCraig's community addition [to this answer about link-removal](https://stackoverflow.com/a/22122095/4581301) for a much easier way to write the `erase` function. — user4581301, May 26 '22 at 01:25
*"I made the main function very simple:"* -- making it simple is good. Can you do better? Do you need to both `append` and `insert` to reproduce the problem? Do you need all six nodes to reproduce the error? Could you get this down to appending two or three nodes, erasing one of them, then printing? — JaMiT, May 26 '22 at 01:29

pm100 · Accepted Answer · 2022-05-26T01:22:10.283

in erase you never assign a value to 'before'

      Node* before;

then you do

     before->next = ptr;

Error C4703 potentially uninitialized local pointer variable 'before' used ConsoleApplication1 C:\work\ConsoleApplication1\ConsoleApplication1.cpp 124

also - more importantly your printLst function sets head to null

void printLst()
{
    while (head != nullptr)
    {
        std::cout << head->value << " ";
        head = head->next; <<========
    }
}

your print function should not change head

so list.first then returns null so this

    list.erase(list.first());

calls erase with null

Exception thrown: read access violation. n was nullptr.

C++ Linked list, program stuck when calling the next pointer in the list

1 Answers1