Copy Assignment Operator Memory Leaks

On By In 1

Dynamic Allocation and Copy Constructors

Dr. Lawlor, CS 202, CS, UAF

(See Gaddis Chapter 9.8 for more info on new and delete, and Chapter 14.4 and 14.5 for info on copy constructors.)

So the basic dynamic allocation syntax is pretty simple: you call "new" to get a pointer, use the space, and finally call "delete" to give up that space.

The typical use is to allocate arrays at runtime:
  • "someptr=new int[n];" allocates space for an array of n integers, and returns you a pointer to the first integer.  Unlike just declaring an array, dynamic allocation with new allows "n" to be a runtime variable.
  • "delete[] someptr;" frees the array.  The "[]" looks like some sort of weird typo, but it indicates you're deleting the whole array.
You can also allocate a single object, although this isn't nearly as common:
  • "someptr=new int;" allocates space for one single integer, and returns you a pointer to the integer.
  • "delete someptr;" frees up the space again.  You really need to free any space you allocate, especially in a long-running program.
Note that for both array and individual allocations, you get a bare pointer back.  This is annoying, because it means you MUST remember whether you've got an array or an individual object, so you can access the space properly.

Here's an example of array allocation:
int foo(void) {
int n=5; cin>>n; // how many integers?
int *ptr=new int[n]; // make an array
for (int i=0;i<n;i++) // write data in
ptr[i]=10*i;
for (int i=0;i<n;i++) // read data back out
cout<<"ptr["<<i<<"]="<<ptr[i]<<"\n";
delete[] ptr; // free allocated space
return 0;
}

(Try this in NetRun now!)

Here's an example of allocating a single integer:
int foo(void) {
int *ptr=new int; // make a pointer
*ptr=7; // write data in
int retval=*ptr; // read data back out
delete ptr; // free allocated space
return retval;
}

(Try this in NetRun now!)

Unfortunately, there are a bunch of things you MUST do with dynamic allocations, and the compiler often can't detect any of these!
  • Setup: You MUST initialize your pointers before using them.  Luckily, the compiler can usually warn you about uninitialized pointers, and uninitialized pointers usually crash immediately.
  • Embezzlement: You MUST access your pointers within the array bounds.  If you asked for [10] elements, just reading from [13] might cause you to crash, or you might read garbage.  Writing is even worse--if you don't crash, you'll overwrite some other part of the program, which will then crash at some unknown later date.
  • Amnesia: You MUST remember to call delete.  If you don't call delete, memory marked as being in use will build up in your program (a "memory leak"), until the machine runs out of memory or your program exits.
  • Doppleganger: You MUST call the correct version of delete: "delete[]" for arrays, and plain "delete" for individual pointers.  Unfortunately, the compiler doesn't detect when you use the wrong delete; it just silently screws up memory so your program crashes sometime in the distant future.
  • Overkill: You MUST not call delete more than once on the same pointer.  You can protect against this by zeroing out your pointers after deleting them (like "delete[] someptr; someptr=0;").  This "double delete bug" is actually common enough that some machines' "delete" has explicit code to check for it.  But it's really hard to detect if you allocate some space and delete it, then somebody else allocates the same space and you then delete their space!
  • Zombies: You MUST not access a pointer after you've deleted it.  Unfortunately, these "living dead" pointers usually work, and some of your data is often still there, but of course that space could be reused by anybody else at any time, resulting in hideous weird crashes.
Because bare pointers are so error-prone, it's common to wrap them in a nicer class interface.

Building a "Wrapper Class" for Nicer Pointers

Here's a simple class that puts a slightly nicer interface onto an array of integers.  The constructor calls "new", there's an overloaded bracket operator to check accesses to the elements of the array, and the destructor calls "delete".
// A nice wrapper around a dynamically allocated array of integers.
class SmartArray {
private:
int *data;
int length;
public:
SmartArray(int len) {
data=new int[len]; // allocate space
length=len;
}
int &operator[](int index) {
if (index<0 || index>=length) { // bounds-check the array index
cout<<"Index "<<index<<" out of bounds!\n";
exit(1);
}
return data[index];
}
~SmartArray() {
delete[] data; // free space
data=0; /* zero out our pointer, to indicate we're gone */
length=-1;
}
};

int foo(void) {
int n=5; cin>>n;
SmartArray arr(n); // array has n integers
for (int i=0;i<n;i++) arr[i]=10*i;
for (int i=0;i<n;i++) cout<<"arr["<<i<<"] = "<<arr[i]<<"\n";
return 0; // destructor automatically deallocates array!
}

(Try this in NetRun now!)

The nice part about this is:
  • The array remembers how long it is, and checks to make sure each access is inside this range.
  • The destructor will be called automatically by the compiler, so you can't possibly forget it.
This is an example of a standard C++ trick called Resource Aquisition Is Initialization (RAII): the constructor allocates, and the destructor deallocates.

Copy Constructor and Assignment Operator

One problem with the above "SmartArray" class is that the C++ compiler automatically (and stupidly) allows people to make a simple shallow copy of a SmartArray object.  Unfortunately, the two copies share the same pointer, so the pointer will be deleted twice!  For example:
class SmartArray {
private:
int *data;
int length;
public:
SmartArray(int len) {
data=new int[len]; // allocate space
length=len;
cout<<"Running SmartArray constructor: data="<<data<<"\n";
}
~SmartArray() {
cout<<"Running SmartArray destructor: data="<<data<<"\n";
delete[] data; // free space
data=0; /* zero out our pointer, to indicate we're gone */
length=-1;
}
};

int foo(void) {
SmartArray arr(2); // array has 2 integers
if (2==2) { // make a copy of the array
cout<<"Making another SmartArray...\n";
SmartArray evilArr=arr; // compiler-generated assignment operator!
// evilArr's destructor will delete arr's pointer!
}
cout<<"Returning from foo...\n";
return 0;
// uh oh! arr's destructor calls delete *again*!
}

(Try this in NetRun now!)

There are two different ways the compiler might make a copy of "arr":
  • "SmartArray somebody(arr);" makes a new SmartArray as a copy of "arr"'s values.  This is called a "copy constructor".
  • "somebody = arr;" overwrites an existing SmartArray with an assignment of "arr"'s values.  This is called an "assignment operator".
Unfortunately, the compiler's automatically generated copy constructor and assignment operator WILL cause big problems in any class with pointers.  And people tend to copy and assign classes a lot, for example to pass them into a function or return them.  So you often need to write your own copy constructor and assignment operator.  This is known as the "Law of the Big Three": if you've got any one of a destructor, copy constructor, or assignment operator, then you probably need all three of them.

One weird trick is to declare a private copy constructor and assignment operator.  That way nobody can call them.  If nobody calls them, you don't even need a body for these functions:
class SmartArray {
private:
int *data;
int length;
// You can't copy or assign SmartArrays (yet)
SmartArray(const SmartArray &from); // copy constructor
SmartArray& operator=(const SmartArray &from); // assignment operator
public:
... rest of stuff ...
};

(Try this in NetRun now!)

This makes any attempt to copy or assign SmartArrays a compile error, which is way better than getting a horrible crash at runtime. 

If you're really ambitious, you can write the copy constructor and assignment operator to do the right thing, making a new copy of the class's data.  This is trickier than it sounds, especially if speed is important, or for the case where some joker assigns an instance to itself (self assignment, like "x=x;").   The question is, to implement "a=b;" for arrays, do you just copy the pointers, a "shallow copy" like the compiler does?  If so, how do you keep track of when to delete the array data?  (There are some cool implementations like "reference counting" and "garbage collection" out there.)  Or do you just copy all the data in the array?  This is called a "deep copy", which uses more time and space, but is a little easier to write.

Here, I've written a deep copy implementation, and added some new little utility methods called "alloc" and "free" to do the data allocation.
/*
A nice wrapper around a dynamically allocated array of integers.
*/
class SmartArray {
public:
SmartArray(int len) { alloc(len);} // ordinary constructor

SmartArray(const SmartArray &from) { // copy constructor
alloc(from.length);
for (int i=0;i<length;i++) data[i]=from[i];
}

SmartArray& operator=(const SmartArray &from) { // assignment operator
if (data==from.data) return *this; // self assignment!
free(); // throw away our old data
alloc(from.length);
for (int i=0;i<length;i++) data[i]=from[i];
return *this;
}

// Array indexing:
int &operator[](int index) { check(index); return data[index]; }

// Constant array indexing:
const int &operator[](int index) const { check(index); return data[index]; }

// Destructor
~SmartArray() { free(); }
private:
int *data;
int length;
void alloc(int len) {// allocate space for len bytes of data
data=new int[len];
length=len;
}
void free(void) {// deallocate data
delete[] data; // free space
data=0; /* zero out our pointer, to indicate we're really gone */
length=-1;
}
void check(int index) const {// bounds-check array index
if (index<0 || index>=length) { // bounds-check the array index
cout<<"Index "<<index<<" out of bounds!\n";
exit(1);
}
}
};

int foo(void) {
int n=5; cin>>n;
SmartArray arr(n); // array has n integers
for (int i=0;i<n;i++) arr[i]=10*i;
if (2==2) { // make a copy of the array
cout<<"Making another SmartArray...\n";
SmartArray evilArr=arr; // our own assignment operator (OK)
// evilArr's destructor will delete its own separate pointer
}
for (int i=0;i<n;i++) cout<<"arr["<<i<<"] = "<<arr[i]<<"\n";
cout<<"Returning from foo...\n";
return 0;
}

(Try this in NetRun now!)

Clearly, this is not something you want to write very often.  So you should re-use something like "SmartArray" rather than writing it from scratch each time!

Copy Constructor and Assignment Operator

Hey guys, I have been fighting with this for hours, but cannot seem to fix it.I am trying to make a copy constructor and an assignment operator use other classes one.

I have a circular dependency of a Stack, then implement a queue in terms of a stack, then a list in terms of a queue.

here is the error I am getting.

here is the List copy constructor.



I tried dynamic binding but could not get it
Why are you comparing against ?


I am not yet that good with c++.But I was taught that makes sure you dont copy itself
bookerb6 wrote:
But I was taught that makes sure you dont copy itself

Yes. You must prevent the copying if and are the same.

is a pointer to the current object (this means it holds the current object's memory address).
You check it against the object's memory address.
You do not check it against the memory address of , which is a member variable of .

Also, be warned that in the C++ library there exists a container named .
You should probably not use the name "queue" in your code, especially if you are .
Unfortunatelly I am given the .h files and have to implement the .cpp files.I also have a problem with the ostream operator.



here I am attempting to print the results, but not deleting the queue.But it seems like list.queue(num); does not work.I am confused now guys.What am I really doing wrong
You are overusing dynamic memory allocation (pointers + ).
Then you have memory leaks because you don't deallocate the memory by 'ing the pointers.



Another thing: the output operation should not modify the object which is output.
In this case, the code should work even if we pass by reference.
(Also just as in the case of vs , there's vs , you've been warned. Twice.)



Why did you not implement a copy constructor (and copy assignment operator) for your queues?

Then you would not have to modify the parameter, you could simply do:



Edit: you are overusing dynamic memory allocation in your class template as well.
Why is a pointer?
Thanks alot Catfish.
It seems to be working now.The reason I dont use the assignment operator is because for some reason the List one is not working well.

I am confused as to how can that occur.The stack and the queue one are working fine, but not the List one.




The Stack and the queue is fine, but the List is not.How is that possible, because it uses the other two.
Both Queue<T>::operator= and List<T>::operator= leak memory. In the case of the List twice as much memory is leaked as it also uses the Queue version. If there is anything wrong with List<T>::operator=, it is also wrong with Queue<T>::operator=.

Does it not seem a little perverse that Stack uses a linked list for it's implementation, and you're ultimately using a Stack for a linked list implementation? It would make a lot more sense for the List to stand alone, and to implement Queue in terms of the List, and Stack in terms of the Queue. You usually want to start with the general and proceed to the specific (or more restricted) which is the opposite of what you're doing here.

Topic archived. No new replies allowed.

0 comments

Leave a Reply

Your email address will not be published. Required fields are marked *