Search code examples
c++memorytreefree

memory leak in a Tree structure with pointers and Node nested class


I have this assignment in which I have to define a Tree class which has a nested private Node class.
The problem is that I cannot use neither smart pointers nor std::vector nor copy operator and copy assignment. And all I am allowed to use are raw pointers.
So I wrote the class and tested it with valgrind to check if I have memory leak, and I did. I know that the memory leak may come from Node class because I don't free _children but when I do that, I get a segmentation fault.
I really don't know how to fix this problem.

Thank you in advanced.


Solution

  • T** getChildren() { return &this->children; }
    

    returns the address of the children member. Later on , you use that address via dereference through array indexing by doing this:

    _info->getChildren()[index] = childTree;
    

    This is invoking undefined behavior. To address this:

    Change you're member to be:

    Tree** _children;
    

    Change your ctor to be:

    Node(T data) 
        : _data( std::move(data) )
        , _children(new Tree<T,N>*[N]())
        , _isWord(false)
    {
    }
    

    and note both the array of pointer syntax as well as the value-initialization of the elements, which will zero-fill the array.

    Finally, change the getChildren() member to simply be:

    Tree** getChildren() { return this->_children; }
    

    That should alleviate your UB, and with it your fault. Not going to sugar coat this. The manual memory management in this is error prone and problematic. You'd be far better off using at least smart pointers, if not outright concrete objects where warranted. But it is what it is.

    Alternative

    Lose the dynamic children allocation entirely. It's size is already fixed by compile-time specification of N. So use that.

    The member becomes simply:

    Tree* _children[N];
    

    The ctor can still value-initialize by doing this:

    Node(T data) 
        : _data( std::move(data) )
        , _children()
        , _isWord(false)
    {
    }
    

    And completely remove the delete [] children; entirely from the destructor;