Operators overloading : memory leaks

Question

This is something I can't understand. Let's say that I overload operator & to make and and between two objects, and operator = to make assignments (making a copy of the object). Let's suppose that I have a class named A, and this code:

A a1,a2,a3;
// initialize a1,a2,a3
a1=a2&a3;

If a1 was already allocated, when I assign a1 to a2&a3, there is a memory leak? is the new object created without deleting the old one ?

PS: Operators overloading using two arguments of type A (I haven't defined a class A, this is an example), returning an arguments of type A, defined as friend of the class.

The code:

It's a very long code, so I used pastebin: http://pastebin.com/42TnThfC But since it's long, I also post the most significative parts:

template<class T>
List<T>& List<T>::operator= (List<T>& l)
{
    List<T>* ptr=l.next;
    if(this!=&l)
    {
        resize(0);
        while(!ptr->end)
        {
            push(ptr->info);
            ptr=ptr->next;
        }
    }
    return *this;
}

template <class T>
List<T>& operator& (List<T>& l1, List<T>& l2) throw()
{
    List<T>* temp,*ptr=l1.next;
    temp=new List<T>();
    try
    {
        if( (l1.end)^(l2.end) )
            throw excp1;
    }
    catch (char *s)
    {
        cout << "Exception: "<<s<<endl;
    }
    if(l1.end)
    {
        while(!ptr->end)
        {
            if(l2.in(ptr->info))
                temp->push(ptr->info);
            ptr=ptr->next;
        }
    }
    else
    {
        if(l1.info==l2.info)
            temp->push(l1.info);
    }
    return *temp;
}

It's a list, push pushes an item, an example of main:

int main(int arcg, char **argv)
{
    List<T>l1,l2,l3;
    for(int i=0;i<10;i++)
    {
        l1.push(i);
        l2.push(i);
        l3.push(i);
    }
    l3=l1&l2;
}

This case l3 is already l1&l2, but will this cause a memory leak?

We need more code. Like the assignment operator and the & operator to be able to help you. — Eric Fortin, Jan 29 '12 at 17:57

score 2 · Accepted Answer · answered Jan 29 '12 at 18:04

No, it should be fine. The original a1 will be destroyed and replaced with a new one (which is the result of a2 & a3).

However you should ensure that, if A contains pointers, the copy constructor, assignment operator and destructor are properly defined for that type, otherwise "the original a1 will be destroyed" may not be enough!

score 0 · Answer 2 · answered Jan 29 '12 at 17:59

0

In class A, assign operator you have to delete original data

class A {
    A &operator=(const A &orig){
        if( &orig == this){
            return *this;
        }

        // here
        if(data){
           delete []data;
        }

       // copy original data
    }
}

answered Jan 29 '12 at 17:59

Vyktor

20,559
6
64
96

missing a check for assigning to self and should probably copy before the delete. – Jan 29 '12 at 17:59
@EthanSteinberg thanks, missed that. Is this okay or do you have better idea? – Vyktor Jan 29 '12 at 18:01
@Als I'm sorry I don't understand the question – Vyktor Jan 29 '12 at 18:03
2

Here yo go: [Copy and Swap Idiom](http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) – Alok Save Jan 29 '12 at 18:04
1

Note that actually needing the self-assignment check for having correct semantics is an indication of an exception safety problem! I haven't a formal proof but as far as I can tell exception-safe assignment operators don't need to guard against self-assignment and those assignment operator implementations which depend on the check are not exception-safe. The above code is an example of this: the object goes into bad state if an exception is thrown after the data is deleted. – Dietmar Kühl Jan 29 '12 at 18:05

score 0 · Answer 3 · answered Jan 29 '12 at 17:59

0

I believe there is only a memory leak if you do something like:

A *a1 = new A(1);
a1 = a2 & a3; // Don't do this, it doesn't work

Simply having A a1, a2, a3; it is impossible to leak these if you declared them like this. The function statically allocates space for these objects when the function is called and they are automatically deallocated when the function is done executing (during the pop off the stack).

answered Jan 29 '12 at 17:59

lukecampbell

14,728
4
34
32

That wouldn't compile though. You would do `*a1 = a2&a3;` which is legitimate. – David Jan 29 '12 at 18:06

Dietmar Kühl · Answer 4 · 2012-01-29T18:09:02.197

0

The responsibility of the assignment operator is to make the object on the left-hand side identical to the one on the right-hand side. If it allocated memory and is about to replace that with something constructed from the right-hand side then it better releases this memory. In general, I found it easiest to implement the assignment operator in terms of the copy constructor and a swap() member function:

T& operator= (T const& other) {
    T(other).swap(*this);
    return *this;
}

The swap() member function just does a memberwise swap.

edited Jan 29 '12 at 18:09

answered Jan 29 '12 at 18:01

Dietmar Kühl

150,225
13
225
380

1

I think the `other` should be pass by copy for this to be copy and swap. – Alok Save Jan 29 '12 at 18:03
@Als: No; he created a copy explicitly. `T(other)` – Lightness Races in Orbit Jan 29 '12 at 18:04
What **should** have been done is to pass `other` by `const&`! I'll fix that. Passing by value should work and avoid requiring the explicit copy and off-hand I don't recall why I generally don't do this (maybe it is just a wrong habit I should fix, though). – Dietmar Kühl Jan 29 '12 at 18:08

Operators overloading : memory leaks

4 Answers4