The reason why all pointers not-freed?

Question

I am working on stack data structure implementation using linked lists , building destroyStack() function by which i can free all allocated memory once the i finish using stack class as i execute the main i expected to get all the pointers freed after destroyStack() has been called.

class Stack{
    private:
        int size;
        typedef struct Node{
            stackType data;
            struct Node *last;
        } Node;
        Node *top;

    public:
        Stack(){
            size = 0;
            top = NULL;
        }

        void push(stackType element){
            Node *temp = new Node();
            if (!temp)
                throw invalid_argument("heap overflow");

            if (element != '\0'){
                temp->data = element;
                temp->last = top;
                top = temp;
                size++;
            }
            else{
                throw invalid_argument("invalid element");
            }
        }

        stackType pop(){
            if (empty())
                throw invalid_argument("this is an empty stack");
            Node *temp = new Node();
            stackType element = top->data;
            temp = top;
            top = top->last;
            temp->last = NULL;
            delete(temp);
            size--;
            return element;

        }

        void destroyStack(){
            Node *temp = top;
            while (top != NULL){
                top = top->last;
                delete(temp);
                temp = top;
            }
        }

};


int main(){

    Stack stack;
    stack.push(100);
    stack.push(90);
    stack.push(80);

    stack.printStack();
    for (int i = 0; i < 3; i++)
        cout << stack.pop() << endl;

    stack.destroyStack();
    return 0;
}

as i use valgrind to check if there any kind of leaks i find this message.

==53372== HEAP SUMMARY:
==53372==     in use at exit: 48 bytes in 3 blocks
==53372==   total heap usage: 8 allocs, 5 frees, 73,824 bytes allocated
==53372== 
==53372== Searching for pointers to 3 not-freed blocks
==53372== Checked 116,952 bytes

so any ideas how can i edit my code to free all the blocks once i used destroy function ?

OT, but any reason why you're using `typedef struct Node {...} Node;` in C++? — mediocrevegetable1, Apr 30 '21 at 07:27
`Node *temp = new Node();` is probably not what you meant to do in `pop` — Alan Birtles, Apr 30 '21 at 07:29
`typedef struct Node` -- `typedef struct` is not necessary in C++. Second, write a proper destructor for `Stack` instead of relying on `main` to call `destroyStack`. Third, please post a [mcve] -- where is `printStack`? And last, did you try your stack with a single node? How about just two nodes? If you did, what were the results? If they also caused a memory leak, having 3 nodes isn't going to make the issue go away, and you might as well debug single / two nodes. — PaulMcKenzie, Apr 30 '21 at 07:31
`if (!temp) throw ...` after `new`ing `temp` is meaningless - `new` throws an exception if it can't allocate memory, see [here](https://pvs-studio.com/en/docs/warnings/v668/). — Evg, Apr 30 '21 at 07:33
Valgrind's summary is good for telling you have a problem, but if you read through the valgrind documentation, it'll tell you how to get much more information that will target the problem pretty much down to the line that allocated the unreleased memory. But since telling an asker to f off and RTFM, is in pretty poor taste, [here's an oldie-but-goodie that aggregates and explains a lot of the information you need](https://stackoverflow.com/questions/5134891/how-do-i-use-valgrind-to-find-memory-leaks) to get better use out of valgrind. — user4581301, Apr 30 '21 at 07:41
Mind you, the line about about "Use cplusplus.com" hasn't aged well. These days we generally recommend cppreference.com around here. cplusplus targets a lower bar to entry, sometimes trading accuracy of information for ease of reading for the beginner. cppreference is probably second only to the C++ Standard for accuracy, but at a cost in readability when complexity of the topic being discussed requires the mind of a lawyer with a computer science degree. Some things just can't be explained easily. — user4581301, Apr 30 '21 at 07:47
the above code is not runnable, but you can add some debugging messages in you code and see — Alessandro Teruzzi, Apr 30 '21 at 07:51
`Node *temp = new Node(); temp = some_other_pointer;` is a leak. Now check all your `new`s. — 463035818_is_not_an_ai, Apr 30 '21 at 08:01

score 0 · Accepted Answer · answered Apr 30 '21 at 08:32

As @AlanBirties already mentioned in the comments, the problem is Node *temp = new Node(); in your pop() function.

You have to do a delete for every new, so that's a block you are not deleting.
However, you don't even need that new node... because that pointer's purpose is to hold the last, already existing, top node, not a new one.

// Problematic code:
Node *temp = new Node();
temp = top; // previous new node is not freed and never used

// Correct code:
Node *temp = top; // no useless memory allocated

You already did this in destroyStack() ;)

The reason why all pointers not-freed?

1 Answers1