deleting pointers in a list.

[bejiz]

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;
}

[Leigh Johnston]

Try the following:

void clear( linked_list& A)

/Leigh

[Leigh Johnston]

Node* p_zap = new Node(0,0); // this is a memory leak

[Igor Tandetnik]

Post by bejiz
class linked_list
{
linked_list(): p_begin(0), p_end(0) {};
~linked_list(){};

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

Post by bejiz
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).

Post by bejiz
{
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).

Post by bejiz
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.

Post by bejiz
void push_back( const int& a)

No need to pass int by reference, just make it

void push_back(int a)

[bejiz]

Thank you for your advices, it works better, I guessed that I only
deleted shallow copies but didn't know why.