1

I'm trying to make single linked list's fucntions.

but it reports an error. like this..

enter image description here

enter image description here

i am trying to a lot of thing about that. like using rvlaue reference, double pointer but nothings work..

what is the problem?

and can i return p pointer in getNode(int k) fucntion?

#include<iostream>

using namespace std;



template<typename T>
class SingleLList {
private:
    template<typename T>
    struct Node {
        T data;
        Node<T>* next;
    };
    Node<T>* head; 
    int size; // for List size
public:
    SingleLList() :head(nullptr) {}; 
    ~SingleLList() {    
        Node<T>* delNode;
        while (head->next != nullptr) {
            delNode = head;
            head = head->next;
            delete delNode;
        }
    };
    // add Node at index th 
    void addNode(int index, T data) {
        if (index < 0)return;
        Node<T>* newNode = new Node<T>;
        newNode->data = data;
        newNode->next = nullptr;

        if (index == 0) { // add at 0 

            // empty
            if (head == nullptr) head = newNode;

            // not empty
            else {
                newNode->next = head->next;
                head = newNode;
            }
            size++;
        }
        else {  
            Node<T>* prev = head;
            for (int i = 1; i < index && prev != nullptr; i++) {
                prev = prev->next;
            }
            newNode->next = prev->next;
            prev->next = newNode;
            size++;
        }
    }
    // traversa
    void showList()const {
        Node<T>* p = head;
        cout << "Single Linked List : [ ";
        while (p != nullptr) {
            cout << p->data << " ";
            p = p->next;
        }
        cout << " ]" << "total elements are : "
            << size << endl;
    }

    // return k th Node by reference.
    Node<T>*& getNode(int k)const {
        if (head == nullptr || k > size) {
            Node<T>* temp = nullptr;
            return temp;
        }
        // Node<T>* p;    < -- is it okay?
        Node<T>* p = new Node<T>;
        p= head;
        for (int i = 1; i < k && p->next != nullptr; i++) {
            p = p->next;
        }
        cout << " address of p : " << &p << endl;
        cout << "value of p : " << p << endl;
        return p;
    }

    

    // delete  n  Node in list
    void deleteNode(Node<T>*& n) {

    
        cout << "address of n : " << &n << endl;
        cout << n->data << endl;
        
        if (n->next == nullptr) { // if last node    
            delete n->next;
            n = nullptr;  // 
    
            size--;
        }
        else {
            Node<T>* del_node = n->next;
            n->data = n->next->data;
            n->next = n->next->next;
            delete del_node;
            size--;
        }
    }
};

int main() {
    SingleLList<int> sll;

    sll.addNode(0, 4);
    sll.addNode(1, 5);
    sll.addNode(2, 6);
    sll.addNode(3, 8);
    sll.addNode(4, 9);
    sll.showList();

    sll.deleteNode(sll.getNode(5));
    
    sll.showList();


    return 0;
}

and in main i make Linked List like this.

enter image description here

JIK
  • 23
  • 4
  • 2
    The address of 0xCCCCCCCC is a really big hint. Have you looked up what that bit pattern usually means yet? – user4581301 Aug 30 '21 at 23:12
  • `//Node* p; <--- is it okay to use??` Yes. There is no point to dynamically allocating with `Node* p = new Node;` and then discarding the address of that brand new node on the next line with `p= head;`. Only `new` when you absolutely have to. What you really wanted was `Node* p = head;` General rule of thumb: If you find yourself making new objects in a get or display function you're probably headed down the wrong path. – user4581301 Aug 30 '21 at 23:20
  • I see several problems with your code, but the one that stands out to me is returning a reference to a local variable, which is in the Very Bad category. Why did you decide to return a `Node*&` instead of a `Node*` or a `Node&`? – JaMiT Aug 30 '21 at 23:21
  • 2
    Jack up your compiler warnings and don't ignore them. The compiler is trying to tell you very important things about your program and can save you a lot of time. – user4581301 Aug 30 '21 at 23:36
  • thank you for your advice. i'm not familar with eng and also not good at coding so i can not understand all of yours. Nevertheless, i really appreciate your comments added for me, taking your time. okay i will make question MRE next time. and do more search on 0xccccccc error. – JIK Aug 30 '21 at 23:44
  • 1
    `0xCCCCCCCC` means uninitialized stack memory on msvc in debug mode: [https://stackoverflow.com/questions/127386/in-visual-studio-c-what-are-the-memory-allocation-representations](https://stackoverflow.com/questions/127386/in-visual-studio-c-what-are-the-memory-allocation-representations) – drescherjm Aug 31 '21 at 00:01

1 Answers1

2
    Node<T>*& getNode(int k)const {
        if (head == nullptr || k > size) {
            Node<T>* temp = nullptr;
            return temp;

This same basic bug occurs several times in the shown code. All instances of this bug will need to be fixed.

temp is a local variable. Once this function returns, it goes out of scope and gets destroyed.

However: this function is declared as returning a reference to a pointer, and by returning temp this ends up returning a reference to an object that's already destroyed, when the function returns. All subsequent use of this reference automatically becomes undefined behavior, and the likely reason for your crash. For example:

sll.deleteNode(sll.getNode(5));

For example, getNode() returns a reference here. To add insult to injury this reference isn't even used immediately, but it gets passed to deleteNode(). By that time temp, or a reference to whatever was originally returned from getNode, is a distant memory and was already destroyed a long, long time ago, and attempting to reference it will not end well.

There are likely other issues, but this is fundamental, and fixing it will require fundamental changes to the shown logic, as such the first order of business will be to redesign the shown code, and it will likely involve other major changes to the rest of the code, as well.

Sam Varshavchik
  • 114,536
  • 5
  • 94
  • 148