Home / Questions / 309452
Accepted
F. Iván
Asked: 2020-11-19 10:14:59 +0800 CST 2020-11-19 10:14:59 +0800 CST

Why is my Stack.Pop() method faulty in memory?

  • 772

I have to implement the pop method of a stack in C and I have done the following:

Stack_pop(tStack *stack) {       
    if (stack != NULL)
    {        
        tStackNode *node = malloc(sizeof(tStackNode));
        node=stack->first->next;
        stack = node;
    }
    else
    {
        return -1;
    }    
}

In theory it should work: I first get the node corresponding to the next one and then assign it as the current node. But when I try it, it hangs, so it's a memory allocation error. The tStack struct is as follows:

typedef struct tNode {
    tValue e;
    struct tNode *next;
} tStackNode;
c

1 Answers

  1. Best Answer

    The problem lies here:

    tStackNode *node = malloc(sizeof(tStackNode));
node=stack->first->next;
stack = node;

    That code has 3 flaws:

    • That memory reservation with mallocis unnecessary, the only thing it causes is a memory leak .

    • The expression: stack->first->nextwill return the memory address of the next node but then store it in the pointer node, it doesn't make any sense to do that, since the pointer nodewill be freed in memory when the function is finished (so you lose the reference).

    • Finally, you save the content of node, in stack, that is, at no time do you update the pointer first(the member of the tStackNode) structure.

    Solution:

    Stack_pop(tStack *stack) 
{      
    tStackNode* aux; 
    if (stack != NULL)
    {        
        aux = stack->first;
        stack->first = stack->first->next;
        free(aux);
    }
    else
    {
        return -1;
    }    
}

    As you will see, we need to rely on a helper pointer to be able to free the last node and additionally to that, update the pointer/member first. Since you must take into account that it firstmust have the base address of the following node.

    • 1