valgrind finding leaks that are hard to find

Question

This is part of a program. When i compile it with g++, it compiles and works fine, but valgrind says that there are definitely lost: 256 bytes in 8 blocks in the leak summary. I narrowed dow the part where the memory leak happens. I tried different things to fix it but valgrind keep finding ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0). the main leak is happening when the buildTree function is called. how can i fix it?

#include <fstream>
#include <iostream>
using namespace std;

const int ARRAYSIZE = 100;

class NodeData {
        friend ostream & operator<<(ostream &, const NodeData &);
public:
        NodeData();          // default constructor, data is set to an empty string
        ~NodeData();
        NodeData(const string &);      // data is set equal to parameter
        NodeData(const NodeData &);  
        bool setData(istream&);

        bool operator==(const NodeData &) const;
        bool operator!=(const NodeData &) const;
        bool operator<(const NodeData &) const;
        bool operator>(const NodeData &) const;
private:
        string data;
};

NodeData::NodeData() { data = ""; }                         // default

NodeData::~NodeData() { }            // needed so strings are deleted properly

NodeData::NodeData(const NodeData& nd) { data = nd.data; }  // copy

NodeData::NodeData(const string& s) { data = s; }

bool NodeData::operator==(const NodeData& rhs) const {
        return data == rhs.data;
}

bool NodeData::operator!=(const NodeData& rhs) const {
        return data != rhs.data;
}

//------------------------ operator<,>,<=,>= ---------------------------------
bool NodeData::operator<(const NodeData& rhs) const {
        return data < rhs.data;
}

bool NodeData::operator>(const NodeData& rhs) const {
        return data > rhs.data;
}

bool NodeData::setData(istream& infile) {
        getline(infile, data);
        return !infile.eof();       // eof function is true when eof char is read
}

//-------------------------- operator<< --------------------------------------
ostream& operator<<(ostream& output, const NodeData& nd) {
        output << nd.data;
        return output;
}

class BinTree
{
  public:
        BinTree();                      // default constructor
        BinTree(const BinTree&);                // copy constructor
        ~BinTree();  
    void makeEmpty();
    bool insert(NodeData*);
  private:
        struct Node
        {
                NodeData* data;         // pointer to data object
                Node* left;             // left subtree pointer
                Node* right;            // right subtree pointer
        };
        Node* root;                     // root of the tree
        void makeEmptyHelper(Node*&);
        bool insertHelper(Node*&, NodeData*);
};


BinTree::BinTree()
{       
        root = NULL;
}

BinTree::BinTree(const BinTree& bt)
{       
        this->root = NULL;
        *this = bt;
}

BinTree::~BinTree()
{       
        makeEmpty();
}

void BinTree::makeEmpty()
{       
        makeEmptyHelper(root);
}

void BinTree::makeEmptyHelper(Node*& current)
{       
        if (current != NULL)
        {     
                if (current->data != NULL)
                {       
                        delete current->data;
                        current->data = NULL;
                }
                makeEmptyHelper(current->left);
                makeEmptyHelper(current->right);
                delete current;
                current = NULL;
        }
        delete current;
}

bool BinTree::insert(NodeData* insertion)
{
        return insertHelper(root, insertion);
}

bool BinTree::insertHelper(Node*& current, NodeData* insertion)
{
        if (current == NULL)
        {
                current = new Node;
                current->data = insertion;
                current->left = NULL;
                current->right = NULL;
                //delete insertion;
        }
        else if (*insertion < *current->data)
        {
                insertHelper(current->left, insertion);
        }
        else if (*insertion > *current->data)
        {
                insertHelper(current->right, insertion);
        }
        else
        {
                return false;
        }
        return true;
}

void buildTree(BinTree&, ifstream&);

int main()
{       
        ifstream infile("data2.txt");
        if (!infile)
        {       
                cout << "File could not be opened." << endl;
                return 1;
        }

        BinTree T;
        cout << "initial data:" << endl << " ";
        buildTree(T, infile);
        cout << endl;

        NodeData* p;    // pointer of retrieved object
        bool found;     // whether or not object was found in tree

        return 0;
}

void buildTree(BinTree& T, ifstream& infile)
{
        string s;

        for (;;)
        {
                infile >> s;
                cout << s << ' ';
                if (s == "$$") break;
                if (infile.eof()) break;

                NodeData* ptr = new NodeData(s);

                bool success = T.insert(ptr);
                if (!success)
                        delete ptr;
        }
}

thanks

    valgrind --leak-check=full --show-leak-kinds=all ./a.out
==2201== Memcheck, a memory error detector
==2201== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==2201== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==2201== Command: ./a.out
==2201== 
initial data:
 iii not tttt eee r not and jj r eee pp r sssss eee not tttt ooo ff m m y z $$ 
==2201== 
==2201== HEAP SUMMARY:
==2201==     in use at exit: 72,960 bytes in 9 blocks
==2201==   total heap usage: 40 allocs, 31 frees, 83,512 bytes allocated
==2201== 
==2201== 256 bytes in 8 blocks are definitely lost in loss record 1 of 2
==2201==    at 0x4C2A0FC: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2201==    by 0x401820: buildTree(BinTree&, std::basic_ifstream<char, std::char_traits<char> >&) (new.cpp:187)
==2201==    by 0x401718: main (new.cpp:168)
==2201== 
==2201== 72,704 bytes in 1 blocks are still reachable in loss record 2 of 2
==2201==    at 0x4C29BBE: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2201==    by 0x4EC0A1F: pool (eh_alloc.cc:123)
==2201==    by 0x4EC0A1F: __static_initialization_and_destruction_0 (eh_alloc.cc:250)
==2201==    by 0x4EC0A1F: _GLOBAL__sub_I_eh_alloc.cc (eh_alloc.cc:326)
==2201==    by 0x400F4F9: call_init.part.0 (in /usr/lib/ld-2.24.so)
==2201==    by 0x400F60A: _dl_init (in /usr/lib/ld-2.24.so)
==2201==    by 0x4000DA9: ??? (in /usr/lib/ld-2.24.so)
==2201== 
==2201== LEAK SUMMARY:
==2201==    definitely lost: 256 bytes in 8 blocks
==2201==    indirectly lost: 0 bytes in 0 blocks
==2201==      possibly lost: 0 bytes in 0 blocks
==2201==    still reachable: 72,704 bytes in 1 blocks
==2201==         suppressed: 0 bytes in 0 blocks
==2201== 
==2201== For counts of detected and suppressed errors, rerun with: -v
==2201== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

Answer 1

Your insertHelper code is broken because when it calls insertHelper recursively, it ignores the return value and returns true even if the object wasn't inserted, leading to a leak.

Instead of this:

    else if (*insertion < *current->data)
    {
            insertHelper(current->left, insertion);
    }
    else if (*insertion > *current->data)
    {
            insertHelper(current->right, insertion);
    }

You need this:

    else if (*insertion < *current->data)
    {
            return insertHelper(current->left, insertion);
    }
    else if (*insertion > *current->data)
    {
            return insertHelper(current->right, insertion);
    }

Why are you using naked pointers anyway?