C++:Segmentation Fault When Attempting to Copy a Class via pointers and functions

Question

The object of this last step of the code is to create a temporary instance of a class, use a set on a private data member of the class, print out the instance while it remains in scope and then use a destructor that takes care of the pointer and the class instance. I am getting the segmentation fault error, I've been getting it for a while even with a lot of changes so Its frustrating me greatly .

 #include <cstdlib>
 #include <iostream>
 #include <cstring>

using namespace std;


//Class Definition

class Box{

      //Private data members
      private:
      int height;
      int width;
      int depth;
      char *name;
      void pri_setname(char *n);

      public:
      //Public constructors and deconstructor
      Box(int,int,int,char *);
      Box(const Box &obj);
      ~Box();
      //Public function prototypes
      void width_set(int w);
      void height_set(int h);
      void depth_set(int d);
      void name_set(char *n);
      void name_print();
      int volume_print();
      void objCreateKeep(Box **, char *n);
      void objCreateTmp(char *n);
};

//Default constructor
Box::Box(int h1 = 1,int w1 =1, int d1 =1,char *n1 = "Blue Box")
{
  strcpy(name,n1);
  height = h1;
  width = w1;
  depth  = d1;

}
//Copy constructor
Box::Box(const Box &obj)
{
  name = new char[25];
  strcpy(name,obj.name);
  height = obj.height;
  width = obj.width;
  depth = obj.depth;
}
//Destructor
Box::~Box()
{
  delete [] name;
  cout<<"Destructor invoked, name pointer is deallocated";
}

void Box::pri_setname(char *n)
{
   strcpy(name,n);  
}

//Set the width of Box()
void Box::width_set(int w)
{
     width = w;
}

//Set the height of Box()
void Box::height_set(int h)
{
     height = h;
}

//Set the depth of Box()
void Box::depth_set(int d)
{
     depth = d;
}

//Set the name of Box()
void Box::name_set(char *n)
{
    name = new char[30];
    strcpy(name,n);
    pri_setname(name);
}

//Prints the name of the box
void Box::name_print()
{
  cout<<"Box Name: "<<name;
}

//Calculate and Print volume of Box()
int Box::volume_print()
{
   int volume = 0;
   volume = height * width * depth;
   return volume ;
}


void Box::objCreateTmp(char *n)
{
   Box tmp;
   tmp.name_set(n);
   tmp.name_print();
   tmp.~Box();
}

void Box::objCreateKeep(Box **pp, char *n)
{
  Box *p = new Box;
  pp = &p;
  p->objCreateTmp(n);
  delete p;
}


int main(int argc, int argv[])
{

    //Check for correct # of cmd line args
    /*
    if(argc != 3)
    {
      cout<<"Wrong number of arguments";
    }
    */
/*
    Box a;
    a.height_set(argv[1]);
    a.width_set(argv[2]);
    a.depth_set(argv[3]);

    Box b = a;
    Box c = a;

    //Set the names of box B and C
    b.name_set("Red Box");
    c.name_set("Orange Box");

    a.name_print();
    b.name_print();
    c.name_print();


*/
    Box *keep;
    Box **pp;

    keep->objCreateKeep(pp,"Blue Box");
    keep->objCreateKeep(pp,"Red Box");
    keep->objCreateKeep(pp,"Orange Box");


    system("PAUSE");
    return(0);
}

> keep->objCreateKeep(pp,"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"); Now I have your credit card details. Morale of the story: Don't use strcpy. Stop. No. Don't. Not ever. — SecurityMatt, Feb 15 '13 at 04:01
You haven't even allocated `keep` by new yet!!, you can't use it — billz, Feb 15 '13 at 04:02
@billz - I think you caught the primary problem - but as you can see from other comments/answers, this code could use a lot of fixing... — Floris, Feb 15 '13 at 04:04
Sorry, i know this code is a mess. If the initial error hadnt convinced me that what I had was completely wrong. Whatever help I can get is greatly appreciated. — user2074233, Feb 15 '13 at 04:09
have a read rule of three, as you have raw pointers as member: http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three — billz, Feb 15 '13 at 04:12

Aesthete · Answer 1 · 2013-02-15T04:16:02.497

0

In addition to the other answers here:

tmp.~Box(); - no.

The destructor is called when it goes out of scope.

delete [] name;

Is called in your destructor, and now it will be called twice. Bad.

edited Feb 15 '13 at 04:16

answered Feb 15 '13 at 04:01

Aesthete

18,622
6
36
45

That was the original error, I didn't realize that my implicit use of the destructor meant it would be used twice. – user2074233 Feb 15 '13 at 04:18

score 0 · Answer 2 · answered Feb 15 '13 at 04:02

0

In the line

pp = &p;

You assign the address of p to pp

When you delete p, it doesn't matter that you remember where it lives...

p->objCreateTmp(n);
delete p;

answered Feb 15 '13 at 04:02

Floris

45,857
6
70
122

At some point I was much closer to the solution then I started getting this error and changed a lot of stuff and added a lot of clutter. The debugger doesnt help and I have no clue how many mistakes I've made since the initial error. It compiles, run time not so much. – user2074233 Feb 15 '13 at 04:07
do you know how to compile with `-g` and use `gdb`? That is probably a great place to start. The other thing to do is - take out "everything" and start with the smallest possible problem; get it to work, then build from there. In your case - just try to create one object, nothing fancy. As @billz pointed out - you can't even call your first method as `keep` isn't initialized. Start there, take little steps. – Floris Feb 15 '13 at 04:10

C++:Segmentation Fault When Attempting to Copy a Class via pointers and functions

2 Answers2