patterncppMinor

Memory leak in stack

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

stackleakmemory

Problem

I'm implementing a stack by C++ and here I think it has memory leakage problem: in Stack::peek() and Stack::pop(), where I created heap space and returned pointer to function caller.

Users who call these two functions may perform deletion on returned pointer, which will recycle heap space once done. I am wondering what is a better approach to my current leakage prone approach.

Header file:

```
// this is the header file of stack data structure.

#ifndef MY_STACK_H
#define MY_STACK_H

class Node
{
private:
int number;
Node * next;

public:
Node(){number = 0; next = NULL;}

Node(int initialNumber, Node * initialNext = NULL)
{
number = initialNumber;
next = initialNext;
}

// copy constructor
Node(Node & copyFromNode)
{
this->number = copyFromNode.getNumber();
this->next = copyFromNode.getNext();
}

// setters & getters
int getNumber() {return number;}
Node * getNext() {return next;}
void setNumber(int newNumber) {number = newNumber;}
void setNext(Node * newNext) {next = newNext;}
};

class LinkedList
{
private:
Node * head;

public:
LinkedList(){head = NULL;}

// very similar to Stack::push()
void addFirst(Node *newNode)
{
newNode->setNext(head);
head = newNode;
}

// somehow similar to Stack::pop()
void deleteFirst()
{
if (head == NULL)
return;
else
{
Node * temp = head;
head = head->getNext();
delete temp;
}
}

bool isEmpty() {return (head == NULL);}
Node * getHead() {return head;}

// there is no setter to head
// since head should be maintained by addFirst() & deleteFirst() only.
};

class Stack
{
private:
Node * top;
LinkedList ll;
public:
Stack(){top = NULL; }

// push new node to the stack
// new node will become the new top of stack
void push(Node * newNode);

Solution

Don't use NULL unless you have to

Some IDEs will refuse to compile your code unless you include ` so that NULL is available. If you compile using a C++ compiler using latest standards, you could use nullptr` instead, since it is a language construct in those versions and does not require including anything.

Minor improvements

//bool Stack::isEmpty()
//{
//    if(top == NULL)
//        return true;
//    else
//        return false;
//}

bool Stack::isEmpty()
{
    return top == nullptr;
}

and

//Node * Stack::pop()
//{
//
//    if (isEmpty())
//        return NULL;
//    else
//    {
//        Node * copy = new Node(*top);
//        ll.deleteFirst();
//        top = ll.getHead();
//        return copy;
//    }
//}

Node * Stack::pop()
{
    if (isEmpty())
    {
        return nullptr;
    }

    Node * copy = new Node(*top);
    ll.deleteFirst();
    top = ll.getHead();
    return copy;
}

Alternative implementation

It's not difficult to devise a generic stack:

my_stack.h

#ifndef MY_STACK_H
#define MY_STACK_H

#include 
#include 
#include 

template
class MyStack {

    std::vector storage;
    void check_not_empty()
    {
        if (storage.empty())
        {
            throw std::runtime_error{"Peeking from an empty stack."};
        }
    }

public:
    void push(const T& element)
    {
        storage.push_back(element);
    }

    void pop()
    {
        check_not_empty();
        storage.pop_back();
    }

    const T& peek() const
    {
        check_not_empty();
        return storage[storage.size() - 1];
    }

    size_t size()
    {
        return storage.size();
    }

    bool is_empty()
    {
        return storage.empty();
    }

    friend std::ostream& operator& stack)
    {
        os << "[";
        std::string separator = "";

        for (size_t i = 0; i != stack.storage.size(); ++i)
        {
            os << separator << stack.storage[i];
            separator = ", ";
        }

        return os << "]";
    }
};

#endif

main.cpp

#include "my_stack.h"
#include 

using namespace std;

int main()
{
    MyStack stack;

    cout << stack << endl;

    for (int i = 1; i <= 5; ++i)
    {
        stack.push(i);
        cout << stack << endl;
    }

    while (!stack.is_empty())
    {
        stack.pop();
        cout << stack << endl;
    }
}

Code Snippets

//bool Stack::isEmpty()
//{
//    if(top == NULL)
//        return true;
//    else
//        return false;
//}

bool Stack::isEmpty()
{
    return top == nullptr;
}

//Node * Stack::pop()
//{
//
//    if (isEmpty())
//        return NULL;
//    else
//    {
//        Node * copy = new Node(*top);
//        ll.deleteFirst();
//        top = ll.getHead();
//        return copy;
//    }
//}

Node * Stack::pop()
{
    if (isEmpty())
    {
        return nullptr;
    }

    Node * copy = new Node(*top);
    ll.deleteFirst();
    top = ll.getHead();
    return copy;
}

#ifndef MY_STACK_H
#define MY_STACK_H

#include <iostream>
#include <stdexcept>
#include <vector>

template<typename T>
class MyStack {

    std::vector<T> storage;
    void check_not_empty()
    {
        if (storage.empty())
        {
            throw std::runtime_error{"Peeking from an empty stack."};
        }
    }

public:
    void push(const T& element)
    {
        storage.push_back(element);
    }

    void pop()
    {
        check_not_empty();
        storage.pop_back();
    }

    const T& peek() const
    {
        check_not_empty();
        return storage[storage.size() - 1];
    }

    size_t size()
    {
        return storage.size();
    }

    bool is_empty()
    {
        return storage.empty();
    }

    friend std::ostream& operator<<(std::ostream& os, const MyStack<T>& stack)
    {
        os << "[";
        std::string separator = "";

        for (size_t i = 0; i != stack.storage.size(); ++i)
        {
            os << separator << stack.storage[i];
            separator = ", ";
        }

        return os << "]";
    }
};

#endif

#include "my_stack.h"
#include <iostream>

using namespace std;

int main()
{
    MyStack<int> stack;

    cout << stack << endl;

    for (int i = 1; i <= 5; ++i)
    {
        stack.push(i);
        cout << stack << endl;
    }

    while (!stack.is_empty())
    {
        stack.pop();
        cout << stack << endl;
    }
}

Context

StackExchange Code Review Q#143977, answer score: 7

Revisions (0)

No revisions yet.