Why while loop become infinite loop?

Question

I made a method print to print all elements of a linked list. I saw the implementation on YouTube, and I wrote the same code. it made an infinite loop while the code in the video on YouTube goes fine. Why did that happen? One of my colleges told me to search about "ptrnull", and I searched but can't understand, and I used "brad" but it didn't give the reason.

#include <iostream>
using namespace std;
#define nl endl
struct MyNode {
  int val;  // value
  MyNode *next;  // address

};

class MyLinkedList {
private :
    MyNode *head = NULL;
    int size = 0;

public:
    void insertFirst(int val)
    {
        MyNode *node = new MyNode;
        node->val = val;
        // if the linked list is empty
        if (head == NULL)
        {
            head = node;
        }
        else
        {
            // the linked list is not empty
            node->next = head;
            head = node;
        }

        size++;
    }

    // to print all elements
    void print()
    {
        
        MyNode *temp = head;
        while (temp != NULL)
        {
            cout << temp->val << nl;
            temp = temp->next;
        }
        
    }
    
    int getSize()
    {
        return size;
    }

};

int main() {
   MyLinkedList list1;
   list1.insertFirst(1);
   list1.insertFirst(2);
   list1.insertFirst(3);
   list1.insertFirst(4);

   cout << list1.getSize() << nl;

   list1.print();
   

    return 0;
}

The return statement is unnecessary - the loop exits if head hence temp is null. — 273K, May 12 '23 at 23:13
Temporarily adding a statement like `printf("temp is now = %p\n", temp);` to just before the bottom of your while-loop will probably show you what is going wrong. — Jeremy Friesner, May 12 '23 at 23:20
Side note: prefer [nullptr](https://en.cppreference.com/w/cpp/language/nullptr) over `NULL` in modern C++. — Jesper Juhl, May 12 '23 at 23:23
Side note: The best way I've ever seen to help debug linked lists is to draw pictures. Draw the list before, all of the steps necessary to transform it into the after state you require and then draw the after state. If while drawing you spot an inconsistency, cool. Fix it. If the pictures don't help, step through your code with a debugger and, doing exactly what the debugger does, try to draw the list. When you find yourself drawing something different, you've found a bug, and you know exactly where it is and what you should have done instead. — user4581301, May 12 '23 at 23:26
"`#define nl endl`" - why are you obfuscating your code just to save typing 2 characters? — Jesper Juhl, May 12 '23 at 23:28
[Why is "using namespace std;" considered bad practice?](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice) — Jesper Juhl, May 12 '23 at 23:29
`return 0;` in `main` would be more portable as `return EXIT_SUCCESS;`. — Jesper Juhl, May 12 '23 at 23:32
Consider what happens when `if (head == NULL)` in `insertFirst`. Is there something missing? — Bob__, May 12 '23 at 23:34
Really useful tool: https://godbolt.org/z/zM9Ex534h It allows you to see what your program looks like to many different compilers. Here I've chosen good ol' g++ and turned on a bunch of extra compile-time diagnostics with `-Wall -Wextra` to spot logic errors before the programs run and extra runtime testing with `-fsanitize=address,undefined` to trap logic errors as the program runs. In the stack trace `#0 0x402666 in MyLinkedList::print() /app/example.cpp:42` says the program tripped over bad memory on line 42. Sadly it can't tell you WHY it's bad, but you can't always get an easy win. — user4581301, May 12 '23 at 23:36
Side note: `endl` is more than just a new line. It also flushes the stream's buffer, committing what you've written to the stream. Possibly doing so prematurely for a significant performance cost. If all you want is a new line, ask for it by name with `'\n'`. — user4581301, May 12 '23 at 23:40
@JesperJuhl — returning 0 and returning `EXIT_SUCCESS` from `main` [both indicate success](https://en.cppreference.com/w/cpp/utility/program/EXIT_status). — Pete Becker, May 13 '23 at 00:16
@PeteBecker `0` return from `main` does *not* indicate success on VMS for example, but `EXIT_SUCCESS` does. — Jesper Juhl, May 13 '23 at 00:18
@JesperJuhl — okay, you’re referring to non-conforming implementations. And that the compiler’s fault, not the OS’s. — Pete Becker, May 13 '23 at 00:21

273K · Answer 1 · 2023-05-12T23:59:14.853

4

You keep uninitialized MyNode::next in the first node. It becomes a tail pointing to nowhere. This is undefined behavior.

You can make the life much easier and less errorprone if you make the constructor:

struct MyNode {
  int val;  // value
  MyNode *next;  // address
  MyNode(MyNode * next, int val) : val(val), next(next) {}
};

// ...

    void insertFirst(int val) {
      head = new MyNode(head, val);
      size++;
    }

Or use the aggregate initialization w/o a user defined constructor:

    void insertFirst(int val) {
      head = new MyNode{val, head};
      size++;
    }

edited May 12 '23 at 23:59

answered May 12 '23 at 23:29

273K

29,503
10
41
64

After applying this answer all that remains is a couple of memory leaks: https://godbolt.org/z/revj7ze73 But that would be a new question. – user4581301 May 12 '23 at 23:44
1

To resolve the memory leak in this answer, the class needs to unwind the list in the destructor: `~MyLinkedList(){ MyNode* _node = head; while(_node != nullptr){ MyNode* prev_node = _node; _node = _node->next; delete prev_node; } }` – Thomas Stillwell May 13 '23 at 21:29

Why while loop become infinite loop?

1 Answers1