CODEWITHSUNDEEP
c++segmentation-fault
EmptyData
EmptyData

Reputation: 2576

how to remove segmentation fault while deleting list of class pointers

I have two classes

class A{};

class B{
Private:
 list<A*> * mylist;
 remove();

};

void B:: remove() // To remove list mylist
{
  list<A*>::iterator iter = mylist->begin;
  for(;iter!=mylist->end;)
  {
    list<A*>::iterator iter1 = iter++;
    --iter;
    delete (*iter); 
    mylist->erase(iter);
    iter = iter1;
  }

}

I am getting segmentation fault in remove function , Please correct me where i am doing wrong.

Upvotes: 0

Views: 1077

Answers (3)

Robᵩ
Robᵩ

Reputation: 168626

This program does not segfault and it runs successfully under valgrind:

#include <list>

class A{};

class B{
public:
  B() {
    mylist = new std::list<A*>;
    mylist->push_back(new A);
    mylist->push_back(new A);
  }
 ~B() { remove(); }

private:
 std::list<A*> * mylist;
 void remove();
};

void B:: remove() // To remove list mylist
{
  std::list<A*>::iterator iter = mylist->begin();
  for(;iter!=mylist->end();)
  {
    std::list<A*>::iterator iter1 = iter++;
    delete (*iter1); 
    mylist->erase(iter1);
  }
  delete mylist;
  mylist = 0;
}

int main () { B b; }

Nevertheless, I would be very upset if you wrote that program for me.

  • It violates the rule of three.
  • It news an std::list. Never new a standard container, it just wastes space.
  • It holds naked pointers with confusing ownership semantics. Prefer std::list<A>. If you absolutely need pointers, use std::list<std::shared_ptr<A> >.
  • It does not provide exception safety.

If you could modify your data structure, here is how I would write your program. Note the lack of explicit destructor, copy constructor, and assignment operator. Everything just works.

#include <list>

class A{};

class B{
public:
  B() {
    mylist.push_back(A());
    mylist.push_back(A());
  }

private:
 std::list<A> mylist;
 void remove();
};

void B:: remove() // To remove list mylist
{
  mylist.clear();
}

int main () { B b; }

Upvotes: 0

WhozCraig
WhozCraig

Reputation: 66204

This code appears to wipe the list. That being said..

for (list<A*>::iterator it = mylist->begin();
   it != mylist->end(); delete *it++);

mylist->clear();

or did I miss something?

Upvotes: 2

Apeirogon Prime
Apeirogon Prime

Reputation: 1238

Never reinvent the wheel...

void B::remove()
{
    mylist.clear();
}

Upvotes: 0

Related Questions