free node in linked list (C )-CodePudding

I wrote a code that supposed to be a stack (LIFO), and it work just fine except from the "pop" function that supposed to delete the last node in the linked list but instead it put some random values in it, If someone know how can I fix my code it will be great.
this is the struct:

typedef struct stack
{
    bool ActiveStructure;
    unsigned int element;
    struct stack* next;
} stack;

my main function:

#define PUSH 1
#define PULL 2
#define CLEAN 3
#define EXIT 4

int main()
{
    stack linkedStack;
    int maxSize = 0, newValue = 0, choice = 0, empty = 0;

    initStack(&linkedStack);

    do {
        std::cout << "1. push value into the stuck\n2. pop out value from the stuck\n3. clean stuck\n4. exit\nEnter your choice: ";
        std::cin >> choice;
        getchar();
        switch (choice)
        {
            case PUSH:

                std::cout << "Enter the value you want to add: ";
                std::cin >> newValue;
                getchar();

                push(&linkedStack, newValue);
                break;

            case PULL:
                empty = pop(&linkedStack);
                if (empty == -1)
                {
                    std::cout << "The stack is empty" << std::endl;
                }
                break;

            case CLEAN:
                if (!isEmpty(&linkedStack))
                {
                    cleanStack(&linkedStack);
                }
                else
                {
                    std::cout << "the stack is empty" << std::endl;
                }
                break;

            case EXIT:
                std::cout << "Goodbye!" << std::endl;
                break;
        }
    } while (choice != EXIT);
}

and this is the pop function

int pop(stack* s)
{
    if (!isEmpty(s))
    {
        stack* p = s;
        while (p->next)
        {
            p = p->next;
        }
        delete(p); /* this line do what she supposed to do when execited. but when I return to the main function suddenly the linkedStack->next has value in her and shes not NULL*/
        return 1;
    }
    return -1;
}

CodePudding user response：

Jeremy Friesner already told you what is wrong. Here's your code:

int pop(stack* s)
{
    if (!isEmpty(s))
    {
        stack* p = s;
        while (p->next)
        {
            p = p->next;
        }
        delete(p);
        return 1;
    }
    return -1;
}

Imagine you have two items in your stack. For fun, let's call them A and B. A->next points to B. B->next points to null. So your code above is going to delete B.

But A->next still points to it. You've deleted it, so now A->next points to garbage.

You need to clean up that dangling pointer.

int pop(stack* s)
{
    if (!isEmpty(s))
    {
        stack* p = s;
        stack * last = nullptr;
        while (p->next)
        {
            last = p;
            p = p->next;
        }
        delete(p);
        if (last != nullptr) {
            last->next = nullptr;
        }
        else {
            // Not sure how you want to handle deleting the first element in the stack, as that's what you're passing in.
        }
        return 1;
    }
    return -1;
}

CodePudding user response：

With this piece of code

 while (p->next)
 {
   p = p->next;
 }

you reach the last node of the linked list, then you do

 delete(p)

This does delete p for sure, but you never set its previous node to null, as you don't set it to null it points to an invalid memory location

What you can do instead is:

while (p->next) 
{
  prev = p;
  p = p->next;
}
delete(p);
prev->next = nullptr;