Prev: Linking with static libs built with MinGW
Next: volatile keyword and memory barriers
From: bejiz on 7 Jan 2010 11:36
Hello,

I try to delete the elements of a linked list with the fonction clear
but according to the result only the first element gets deleted, after
there are only 0's. Perhaps do you know what to do about it.
Thanks.


#include <iostream>
using namespace std;



class linked_list
{
public:
linked_list(): p_begin(0), p_end(0) {};
~linked_list(){};
void clear( linked_list A)
{
while(p_begin!=0)
{
Node* p_zap = new Node(0,0);
p_zap = A.p_begin;
cout << " delete : " << p_zap->data << endl;
p_begin = p_begin->p_next;
delete p_zap;
}
}

void push_back( const int& a)
{
if(!p_begin)
{
Node* temp = new Node(a,0);
p_begin = p_end = temp;
}
else
{
Node* temp = new Node(a,0);
p_end->p_next = temp;
p_end = temp;
}
}

friend
ostream& operator<<( ostream& xout, const linked_list& A )
{
Node* a = A.p_begin;
if (a==0){
xout << " empty " << endl;
return xout;
}
else
xout << a->data << endl;
while((a->p_next)&&a)
{
a = a->p_next;
xout << a->data << endl;
}
return xout;
}

private:
struct Node
{
int data;
Node* p_next;
Node(int val, Node* p = 0): data(val), p_next(p) {};
}*p_begin, *p_end;


};

int main()
{
linked_list A;
A.push_back(23);
A.push_back(233);
A.push_back(73);
cout << A ;
A.clear(A);
cout << A;

return 0;
}
From: Leigh Johnston on 7 Jan 2010 11:49
Try the following:

void clear( linked_list& A)

/Leigh

From: Leigh Johnston on 7 Jan 2010 11:51
Node* p_zap = new Node(0,0); // this is a memory leak
From: Igor Tandetnik on 7 Jan 2010 12:02
bejiz <bruno.julier(a)wanadoo.fr> wrote:
> class linked_list
> {
> public:
> linked_list(): p_begin(0), p_end(0) {};
> ~linked_list(){};

You'd probably want to call clear() from destructor.

> void clear( linked_list A)

Why does clear() take a parameter? It's a method of linked_list - shouldn't it work on the instance it's called on?

Also, it takes its parameter by value, meaning it works on a copy of linked_list. But it's a shallow copy - you now have two instances of linked_list whose p_begin and p_end members point to the same set of nodes. You delete those nodes and update p_begin and p_end in the temporary copy of linked_list, but the original remains unchanged: its p_begin and p_end members are now dangling pointers (pointing to data that's already been deallocated).

> {
> while(p_begin!=0)
> {
> Node* p_zap = new Node(0,0);
> p_zap = A.p_begin;

You allocate a new instance of Node, make p_zap point to it - and then immediately make it point to something else. Thus, you leak that Node(0, 0).

> cout << " delete : " << p_zap->data << endl;
> p_begin = p_begin->p_next;
> delete p_zap;
> }

You probably want to set p_end = NULL here, otherwise it still points to a now-deleted last node.

> void push_back( const int& a)

No need to pass int by reference, just make it

void push_back(int a)

--
With best wishes,
Igor Tandetnik

With sufficient thrust, pigs fly just fine. However, this is not necessarily a good idea. It is hard to be sure where they are going to land, and it could be dangerous sitting under them as they fly overhead. -- RFC 1925

From: bejiz on 7 Jan 2010 15:27
Thank you for your advices, it works better, I guessed that I only
deleted shallow copies but didn't know why.
 | 
Pages: 1
Prev: Linking with static libs built with MinGW
Next: volatile keyword and memory barriers