Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C++ > memory leak

Reply
Thread Tools

memory leak

 
 
mast2as@yahoo.com
Guest
Posts: n/a
 
      07-05-2006
Hi guys

I wrote a short program to test a Tree class... I plan to use it a lot
in my application so thought I would do a memory leak check before I
start using it. The code seems good to me in a glance (new & delete
seem to be called for all dynamically crated objects) but when I run
valgrind, valgrind reports a memory leak.
The problem seems to come when i start using push().

Could anyone help me with this please ? I just don't see what i am
doing wrong.

thank you, mark

#include <iostream>
#include <deque>

using namespace std;

template<typename T>
class TreeNode
{
public:
TreeNode( ) : m_parent( NULL ) {
cout << "new node" << endl;
//cout << "new data" << endl;
//m_data = new T;
}

~TreeNode() {
cout << "delete node\n";
//cout << "delete data\n";
//delete m_data;
}

void setData( T data ) { m_data = data; }
void setParent( TreeNode<T> *node ) { m_parent = node; }
TreeNode<T> *getParent() const { return m_parent; }
T getData() const { return m_data; }
private:
TreeNode<T> *m_parent;
T m_data;
};

template<typename T>
class Tree
{
public:

Tree() {
m_currentNode = new TreeNode<T>;
m_currentNode->setParent( NULL );
}

~Tree() {
cout << "Tree destructor " << m_nodes.size() << endl;
while ( ! m_nodes.empty() ) {
TreeNode<T> *node = m_nodes.back();
m_nodes.pop_back();
delete node;
}
delete m_currentNode;
}

void push( T data )
{
TreeNode<T> *node = new TreeNode<T>;
cout << "size of Node " << sizeof( *node ) << endl;
node->setParent( m_currentNode );
node->setData( data );
m_nodes.push_back( node );

m_currentNode = node;
}

void pop() {
if ( m_currentNode->getParent() ) {
m_currentNode = m_currentNode->getParent();
} else {
cout << "Tree::data out of range (abort)" << endl;
}
}

T getCurrentData() const { return m_currentNode->getData(); }
int getTreeSize() const { return m_nodes.size(); }

private:
TreeNode<T> *m_currentNode;
deque<TreeNode<T> *> m_nodes;
};

int main()
{
Tree<int> tree;
tree.push( 1 );
return 0;
}

/// running it
new node
new node
size of Node 8
Tree destructor 1
delete node
delete node

/// VALGRIND REPORT
==12771==
new node
new node
size of Node 8
Tree destructor 1
delete node
delete node
==12771== Invalid free() / delete / delete[]
==12771== at 0x1B904616: operator delete(void*)
(vg_replace_malloc.c:156)
==12771== by 0x8048B2B: Tree<int>::~Tree() (in p_proto1/test)
==12771== by 0x80489BD: main (in p_proto1/test)
==12771== Address 0x1BB457C0 is 0 bytes inside a block of size 8
free'd
==12771== at 0x1B904616: operator delete(void*)
(vg_replace_malloc.c:156)
==12771== by 0x8048B05: Tree<int>::~Tree() (in p_proto1/test)
==12771== by 0x80489BD: main (in p_proto1/test)
==12771==
==12771== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 17 from
1)
==12771== malloc/free: in use at exit: 1288 bytes in 2 blocks.
==12771== malloc/free: 4 allocs, 3 frees, 1808 bytes allocated.
==12771== For counts of detected errors, rerun with: -v
==12771== searching for pointers to 2 not-freed blocks.
==12771== checked 2370936 bytes.
==12771==
==12771==
==12771== 8 bytes in 1 blocks are definitely lost in loss record 1 of 2
==12771== at 0x1B90406F: operator new(unsigned)
(vg_replace_malloc.c:133)
==12771== by 0x8048DD0: Tree<int>::Tree() (in p_proto1/test)
==12771== by 0x804899F: main (in p_proto1/test)
==12771==
==12771== LEAK SUMMARY:
==12771== definitely lost: 8 bytes in 1 blocks.
==12771== possibly lost: 0 bytes in 0 blocks.
==12771== still reachable: 1280 bytes in 1 blocks.
==12771== suppressed: 0 bytes in 0 blocks.
==12771== Reachable blocks (those to which a pointer was found) are not
shown.
==12771== To see them, rerun with: --show-reachable=yes

 
Reply With Quote
 
 
 
 
TB
Guest
Posts: n/a
 
      07-05-2006
http://www.velocityreviews.com/forums/(E-Mail Removed) skrev:
> Hi guys
>
> I wrote a short program to test a Tree class... I plan to use it a lot
> in my application so thought I would do a memory leak check before I
> start using it. The code seems good to me in a glance (new & delete
> seem to be called for all dynamically crated objects) but when I run
> valgrind, valgrind reports a memory leak.
> The problem seems to come when i start using push().
>
> Could anyone help me with this please ? I just don't see what i am
> doing wrong.
>
> thank you, mark
>
> #include <iostream>
> #include <deque>
>
> using namespace std;
>
> template<typename T>
> class TreeNode
> {
> public:
> TreeNode( ) : m_parent( NULL ) {
> cout << "new node" << endl;
> //cout << "new data" << endl;
> //m_data = new T;
> }
>
> ~TreeNode() {
> cout << "delete node\n";
> //cout << "delete data\n";
> //delete m_data;
> }
>
> void setData( T data ) { m_data = data; }
> void setParent( TreeNode<T> *node ) { m_parent = node; }
> TreeNode<T> *getParent() const { return m_parent; }
> T getData() const { return m_data; }
> private:
> TreeNode<T> *m_parent;
> T m_data;
> };
>
> template<typename T>
> class Tree
> {
> public:
>
> Tree() {
> m_currentNode = new TreeNode<T>;
> m_currentNode->setParent( NULL );
> }
>
> ~Tree() {
> cout << "Tree destructor " << m_nodes.size() << endl;
> while ( ! m_nodes.empty() ) {
> TreeNode<T> *node = m_nodes.back();
> m_nodes.pop_back();
> delete node;
> }


You delete every node in m_nodes,

> delete m_currentNode;


then you delete m_currentNode, which if you look below
actually already is deleted.

> }
>
> void push( T data )
> {
> TreeNode<T> *node = new TreeNode<T>;
> cout << "size of Node " << sizeof( *node ) << endl;
> node->setParent( m_currentNode );
> node->setData( data );
> m_nodes.push_back( node );
>
> m_currentNode = node;


^^^^^^^^^^^^^^^^^^^^^ This statement is causing you
all the trouble!

With your first push, you loose the first allocated TreeNode
created in the constructor.

--
TB @ SWEDEN
 
Reply With Quote
 
 
 
 
mast2as@yahoo.com
Guest
Posts: n/a
 
      07-05-2006
of course ! thank you very much... well the whole class wasn't really
well thought anyway...

I just did the following changes and it seems to work now. Thanks again

#include <iostream>
#include <deque>

using namespace std;

template<typename T>
class TreeNode
{
public:
TreeNode( ) : m_parent( NULL ) {
cout << "new node" << endl;
//cout << "new data" << endl;
//m_data = new T;
}

~TreeNode() {
cout << "delete node\n";
//cout << "delete data\n";
//delete m_data;
}

void setData( T data ) { m_data = data; }
void setParent( TreeNode<T> *node ) { m_parent = node; }
TreeNode<T> *getParent() const { return m_parent; }
T getData() const { return m_data; }
private:
TreeNode<T> *m_parent;
T m_data;

};

template<typename T>
class Tree
{
public:
Tree() : m_depth( 0 ) {}

~Tree() {
cout << "Tree destructor " << m_nodes.size() << endl;
while ( ! m_nodes.empty() ) {
TreeNode<T> *node = m_nodes.back();
m_nodes.pop_back();
delete node;
}
//delete m_currentNode;
}

void push( T data )
{
TreeNode<T> *node = new TreeNode<T>;
cout << "size of Node " << sizeof( *node ) << endl;
if ( m_depth ) {
node->setParent( m_currentNode );
}
node->setData( data );
m_nodes.push_back( node );
m_depth;
m_currentNode = node;
}

void pop() {
if ( m_depth ) {
m_currentNode = m_currentNode->getParent();
m_depth--;
} else {
cout << "Tree::data out of range (abort)" << endl;
}
}

T getCurrentData() const { return m_currentNode->getData(); }
int getTreeSize() const { return m_nodes.size(); }

private:
int m_depth;
TreeNode<T> *m_currentNode;
deque<TreeNode<T> *> m_nodes;
};

int main()
{
Tree<int> tree;
tree.push( 1 );
tree.push( 2 );
return 0;
}

 
Reply With Quote
 
Thomas J. Gritzan
Guest
Posts: n/a
 
      07-05-2006
(E-Mail Removed) schrieb:
> of course ! thank you very much... well the whole class wasn't really
> well thought anyway...
>
> I just did the following changes and it seems to work now. Thanks again


See below.

> #include <iostream>
> #include <deque>
>
> using namespace std;
>
> template<typename T>
> class TreeNode
> {
> public:
> TreeNode( ) : m_parent( NULL ) {
> cout << "new node" << endl;
> //cout << "new data" << endl;
> //m_data = new T;
> }
>
> ~TreeNode() {
> cout << "delete node\n";
> //cout << "delete data\n";
> //delete m_data;
> }
>
> void setData( T data ) { m_data = data; }
> void setParent( TreeNode<T> *node ) { m_parent = node; }
> TreeNode<T> *getParent() const { return m_parent; }
> T getData() const { return m_data; }
> private:
> TreeNode<T> *m_parent;
> T m_data;
>
> };
>
> template<typename T>
> class Tree
> {
> public:
> Tree() : m_depth( 0 ) {}


Add: m_currentNode(0)

>
> ~Tree() {
> cout << "Tree destructor " << m_nodes.size() << endl;
> while ( ! m_nodes.empty() ) {
> TreeNode<T> *node = m_nodes.back();
> m_nodes.pop_back();
> delete node;
> }
> //delete m_currentNode;
> }
>
> void push( T data )
> {
> TreeNode<T> *node = new TreeNode<T>;
> cout << "size of Node " << sizeof( *node ) << endl;
> if ( m_depth ) {
> node->setParent( m_currentNode );
> }
> node->setData( data );
> m_nodes.push_back( node );
> m_depth;


m_depth++;

> m_currentNode = node;
> }
>
> void pop() {
> if ( m_depth ) {
> m_currentNode = m_currentNode->getParent();
> m_depth--;
> } else {
> cout << "Tree::data out of range (abort)" << endl;
> }
> }
>
> T getCurrentData() const { return m_currentNode->getData(); }
> int getTreeSize() const { return m_nodes.size(); }
>
> private:
> int m_depth;
> TreeNode<T> *m_currentNode;
> deque<TreeNode<T> *> m_nodes;
> };


Thats not a tree, its a stack (well, actually a stack is a special kind
of tree).

Why don't you use deque<> or list<> directly, or implement a thin
wrapper over it? You don't need the new/delete stuff:

template <typename T>
class stack
{
public:
stack() {}
~stack() {}

void push(const T& value) { m_data.push_back(value); }
void pop() { m_data.pop_back(); }
const T& top() const { return m_data.back(); }
size_t size() const { return m_data.size(); }

private:
std::deque<T> m_data;
};

(not tryied to compile)

--
Thomas
 
Reply With Quote
 
mast2as@yahoo.com
Guest
Posts: n/a
 
      07-05-2006
Hi Thomas

Yes sorry for some reason the m_depth++ disappeared and ended up being
m_depth is the post (?)

I was using deque originally but them problem is that i need a stack of
objects which are actually pretty big (in terms of memory) and a lot of
them will be saved on that stack I need the same mechanism as
pop_back() and push_back() excepted that I need to keep the entire
"tree" structure until termination of the program. Meaning when I call
pop() i actually want to go one level down in the queue but want to
keep the object in the stack *I don't want to get rid of it with a
pop_back()*

Basically they are a bunch of objects (class Object lets say) that
points to the TreeNodes stored in the Tree object.

Tree<int> atree;
atree.push( 1 );
atree.puhs( 2 );
Object A, B, C;
A.setWith( atree.getCurrentInteger() );
B.setWith( atree.getCurrentInteger() );
atree.pop(); // go down one level but don't get rid of 2 as A and B
points to it
C.setWith( atree.getCurrentInteger() );
atree.puhs( 3 );

A.doSomethingUsingIndex(); // 2
B.doSomethingUsingIndex(); // 2
C.doSomethingUsingIndex(); // 1

It's a tree in the sense that it ends up looking that

A--|
|-B
|-C
|-D
E-|
|-F --|
|-G
|-H
|-I
J

Hope it make sense, Mark

 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Memory leak even after deleting memory pointers from vector cham C++ 5 09-25-2008 10:30 AM
Leak or no leak ?? Richard Heathfield C Programming 4 07-10-2006 11:37 AM
Dynamic memory allocation and memory leak... s.subbarayan C Programming 10 03-22-2005 02:48 PM
Memory leak??? (top reporting high memory usage under Solaris) Mark Probert Ruby 4 02-09-2005 06:13 PM
Wireless Zero Configuration Memory Leak?? =?Utf-8?B?Umlja3NjaHVsdHox?= Wireless Networking 3 01-19-2005 11:26 PM



Advertisments