CODEWITHSUNDEEP
c++inheritancedeep-copy
Airwavezx
Airwavezx

Reputation: 957

Basic deep copy (op overloading)

I am making a simple class, Person, which inherits from an abstract class, Object. I'm in the part where I'm learning about deep copy, but I can't seem to get this simple piece of code to work:

Person.h

#include <iostream>
#include <cstring>  
#ifndef _PERSON_H_
#define _PERSON_H_
using namespace std;


class Object {
protected:
    //This pure virtual func. sends appropriate  ostream
    //to operator<<'s override. Must be implemented in each
    //derived class accordingly.
    virtual void print(std::ostream&) const =0;
public:
    //Override ostream operator.
    friend ostream& operator<<(ostream& os, const Object& obj);
    friend ostream& operator<<(ostream& os, const Object* obj);
    virtual ~Object();
};

class Person : public Object {
private:
    char* m_name;
    virtual void print(std::ostream& os) const;
    Person(); //What's your default name?
public:
    Person(char* name);
    Person(const Person& p);
    Person& operator=(const Person &rhs);
    virtual ~Person();
};

#endif // _PERSON_H_

Person.cpp

#include "Person.h"


ostream& operator<<(ostream& os, const Object& obj) {
    obj.print(os);
    return os;
}

ostream& operator<<(ostream& os, const Object* obj) {
    obj->print(os);
    return os;
}
Object::~Object() { };

//Standard ctor
Person::Person(char* name) {
    m_name = new char[strlen(name)];
    strncpy(m_name, name, strlen(name));
}
//Copy ctor
Person::Person(const Person& p) {
    m_name = new char[strlen(p.m_name)];
    strncpy(m_name, p.m_name, strlen(p.m_name));
}


//dtor
Person::~Person() {
    delete [] m_name;
}
//operator= overload
Person& Person::operator=(const Person &rhs) {
    if (this == &rhs) return * this; //Self copy check.
    delete [] m_name;
    m_name = new char[strlen(rhs.m_name)];
    strncpy(m_name, rhs.m_name, strlen(rhs.m_name));
    return * this;
}

//private
void Person::print(std::ostream& os) const{
    os << m_name;
}

int main() {
    //Person p1("Anis");
    //Person p2("Bassam");
    Person * p1 = new Person("Anis");
    Person * p2 = new Person("Bassam");
    p1 = p2;
    delete p1; 
    delete p2;
    return 0;
}

Output:

a.out(4839,0x7fff7420b300) malloc: *** error for object 0x7f914b404ad0: pointer being freed was not allocated

gdb bt:

#0  0x00007fff8d7b2282 in __pthread_kill ()
#1  0x00007fff8b5fd4c3 in pthread_kill ()
#2  0x00007fff8e528b73 in abort ()
#3  0x00007fff83739937 in free ()
#4  0x0000000100000b51 in Person::~Person (this=0x1001048e0) at Person.cc:29
#5  0x0000000100000b85 in Person::~Person (this=0x1001048e0) at Person.cc:28
#6  0x0000000100000ba8 in Person::~Person (this=0x1001048e0) at Person.cc:28
#7  0x0000000100000e46 in main () at Person.cc:54

Program crashes on delete p2;, according to gdb. In the book they warned us about this situation, so I followed the fix they offered, which was to overload operator= and making sure to have copy ctor. What am I missing? Thanks in advance.

p.s. I'm only interested in deep-copy issues at the moment. Templates and generics come in later chapter.

Upvotes: 0

Views: 215

Answers (3)

James Adkison
James Adkison

Reputation: 9602

Do you think this line uses your Person::operator= function? It doesn't. This line is dealing with pointers to objects not the objects themselves (i.e., this is simple pointer assignment).

p1 = p2;

The following line would invoke your Person::operator= function.

*p1 = *p2;

I'm not sure why you got the error you received. I would have guessed that you would have received something about double free/delete.

Upvotes: 2

user2512323
user2512323

Reputation:

After p1=p2, both variables point to the same object. delete p1 deletes this object. So delete p2 tries to delete this object again.

Operator= can't help you here, as you assign pointers, not objects. Probably you want *p1 = *p2.

Upvotes: 3

juanchopanza
juanchopanza

Reputation: 227578

There may be other errors, but at the very least you have undefined behaviour whenever you call strlen on m_name or when you do os << m_name;, because you don't reserve enough space for a null-terminated string. This allocation is wrong:

m_name = new char[strlen(name)];

You need

m_name = new char[strlen(name) + 1];

to allow for the null terminator. You must also ensure that the null termination is set.

Upvotes: 1

Related Questions