Returning a copy with overloaded operators

Question

I have a class Foo for which I've overloaded the + operator as follows:

Foo Foo::operator+(const Bar &b)
{
    Foo copy = (*this);
    if (someCondition) return copy;
    //snip
}

To me, this looks reasonable. However, whenever I am returning copy, Visual Studio alerts me of an error which 'may be due to a corruption of the heap'. Is there something wrong with what I've done?

edit: updating with more info.

The error message:

Windows has triggered a breakpoint in sample.exe.

This may be due to a corruption of the heap, which indicates a bug in sample.exe or any of the DLLs it has loaded.

This may also be due to the user pressing F12 while sample.exe has focus.

The output window may have more diagnostic information.

The copy constructor:

Foo::Foo(const Foo&p)
{
    some_pointer = p.get_some_pointer();
    some_value = p.get_some_value();
}

The code it breaks to:

//within dbgheap.c
    extern "C" _CRTIMP int __cdecl _CrtIsValidHeapPointer(
            const void * pUserData
            )
    {
            if (!pUserData)
                return FALSE;

            if (!_CrtIsValidPointer(pHdr(pUserData), sizeof(_CrtMemBlockHeader), FALSE))
                return FALSE;

            return HeapValidate( _crtheap, 0, pHdr(pUserData) );
    }

If you delete some_pointer in the destructor, that will be a problem. Also how about your assignment operator? I think Foo copy = (*this); is an assignment rather than copy. — ali_bahoo
– ali_bahoo, Commented Mar 4, 2011 at 8:01
@sad_man, @Andrew: Foo copy = (*this) invokes the copy constructor. — Björn Pollex
– Björn Pollex, Commented Mar 4, 2011 at 8:03
@sad_man : Yes, it invokes copy constructor even if the (copy) assignment operator is overloaded explicitly. — Mahesh
– Mahesh, Commented Mar 4, 2011 at 8:07

David Rodríguez - dribeas · Accepted Answer · 2011-03-04 08:58:31Z

3

That type of error is usually associated with multiple deletions (or frees) of the same pointer, or with some more obscure situations (acquiring from one heap and releasing to a different heap, but that is probably not the case here).

The first thing I would do is look a the destructors and check that you are not shallow copying and doubly deleting. For example with the following code:

// buggy!!!
struct test {
    int * data;
    test() : data( new int[5] ) {}
    ~test() { delete [] data; }
    test( test const & rhs ) : data( rhs.data ) {}
    test& operator=( test const & rhs ) {
       data = rhs.data;
    }
};
int main() {
    test t1;          // 5 ints allocated int t1.data
    test t2( t1 );    // no memory allocated, t2.data == t1.data
} // t2 out of scope: t2.~test() => delete t2.data
  // t1 out of scope: t1.~test() => delete t1.data but both are the same: double delete

If this is the case, you can decide whether you want to have shallow copies or make deep copies. In the second case the copy constructor (and assignment operator) should allocate their own memory, while in the second case you must ensure that the memory is not released twice.

As always with pointers, it is better to delegate resource management to external (pre-built) classes. In the case of unique ownership (and deep copies) you should probably use std::auto_ptr (or std::unique_ptr in C++0x -- or the boost variants). In the second case, using boost::shared_ptr (or std::shared_ptr in C++0x) will ensure that the data is shared and only deleted once.

answered Mar 4, 2011 at 8:58

David Rodríguez - dribeas

209k23 gold badges304 silver badges496 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

Björn Pollex Over a year ago

And if the class does not own the object the pointer points to, prefer a reference.

visitor Over a year ago

@Space: A preference that will effectively render the class non-assignable (which would perhaps be weird for a class with arithmetic operators)...

Björn Pollex Over a year ago

@visitor: Right, didn't consider that.

Collectives™ on Stack Overflow

Returning a copy with overloaded operators

1 Answer 1

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related