C++ operator overloads and destructors

Question

struct Node
{
int value
Node* next;
}
typedef List Node*

const Set operator +(const Set& a, const Set& b)
{
    Set aSet;
    List newList = mergeListsCopy(a.list, b.list);
    aSet.list = newList;
    return aSet;
}

class Set
{
public:
//method decs
private:
    List list;
};

Set::~Set()
{
    list = deleteList(list);    
}

The internals of this code work perfectly fine, mergeListsCopy creates a new list from two singly linked lists and assignes the pointer to the list which is a private variable of aSet.

The problem is when aSet is returned, aSet.list is some strange poison address( in this case 0xf).

When I ran it through the debugger a Set was created in the scope of the operator overload and but two references to this set were also created locally both using the symbol aSet, before the return occurred, the program jumped to the destructor, presumably for the extraneous Set, but since there is only one Set it gets destroyed.

When I comment out my destructor this problem goes away. What did I do wrong?

NB operator methods are friends used function calls work. – awiebe Mar 26 '12 at 07:15 — awiebe, Mar 26 '12 at 07:15

score 3 · Accepted Answer · edited May 23 '17 at 12:20

3

You need to follow the Rule of Three.

If you need to explicitly declare either the destructor, copy constructor or copy assignment operator yourself, you probably need to explicitly declare all three of them.

It is most likely that temporary nameless objects get created(by calling the implicit compiler generated copy constructor) during the course of execution of your program and when those temporary objects get destroyed by call to destructor it ends up messing your linked list.

edited May 23 '17 at 12:20

Community

1
1

answered Mar 26 '12 at 07:14

Alok Save

202,538
53
430
533

So if I have a destructor, then it won't work unless I also supply a copy constructor? I'm confused. I suppose I should have also mentioned that I have specified a default constructor, it just sets Set.list to NULL. – awiebe Mar 26 '12 at 07:19
Actually, You need to apply the **Rule of Three**, to both your classes, `Set` & `Node` as well(I believe it would need declaring your own one of the Big three mentioned above). The problem seen here is due to not applying Rule of Three to `Set` class. – Alok Save Mar 26 '12 at 07:21
1

@awiebe: You don't provide a copy constructor! the copy constructor creates a shallow copy of your pointers during temporary object creation(pass by value etc) which is further destroyed in the call to destructor thus invalidating and making your actual object a dangling pointer. You will need to provide a copy constructor which deos the deep copy of the pointers invloved. – Alok Save Mar 26 '12 at 07:23
@awiebe: It does not necessarily have to be deep copy. Reference-counting the list is probably preferable if you are going to pass the Set around by value. Also if you can use C++11, you could only provide move-constructor that takes over and delete the copy-constructor. Than you can still use temporaries by value, but avoid any implicit deep copies. – Jan Hudec Mar 26 '12 at 07:59

C++ operator overloads and destructors

1 Answers1