3

using answers from operator overloading memory leak I added copy constructor and copy assingment operator, changed the operator+() as NathanOliver advised and now I am passing to constructor static array. Still have memory leak and strange thing is that I got this memory leak even when in main there is only class variable initialization, doesn't matter if with parameters or not... Any suggestions? I think cunstructor is valid.

Set::Set(int n, int* array){
   number = n; 
   elems = array;
   std::sort(elems, elems + number);
}

Set::Set(const Set& s){
   number=s.number;
   elems=s.elems;
}
Set& operator=(const Set& X){

   if(this==&X)
     return *this;
   delete [] elems;
   elems=X.elems;
   number=X.number;
   return *this;

I use gcc (tdm64-2) 4.8.1 compiler.

Community
  • 1
  • 1
ssukienn
  • 558
  • 6
  • 22
  • 1
    elems = X.elems just copies the pointer, is this what you want ? – KostasRim Nov 11 '15 at 12:29
  • Your code does not show any memory leak because you don't allocate any dynamic memory in the shown code. To find leaks, search for the code that does allocate and then follow the pointers. – eerorika Nov 11 '15 at 13:26

1 Answers1

4

There are two kinds of copy in c++, Shallow copy and Deep copy. The first just copies the pointers and the later copies the values. By default copy constructor and overload assignment operator do a shallow copy if generated by the compiler. Now lets take a look at your code.

Set::Set(int n, int* array){
   number = n; 
   elems = array;
   std::sort(elems, elems + number);
}

This constructor takes an in array. If this array is allocated on the heap using new then it should be deallocated using delete[]. Now the statement:

 elems = array;

is a shallow copy, it actually copies pointers not values. So if you accidentally delete array in main then elem will become a dangling pointer since it will point to a deleted array. Dereferencing or deleting elem after that will have undefined behaviour.

Now the same applies here:

Set& operator=(const Set& X){

   if(this==&X)
   return *this;
   delete [] elems;
   elems=X.elems;
   number=X.number;
   return *this;
}

You make again a shallow copy, you just copy pointers. In order to fix this you need 

delete[] elems;
elems = new int[n]; //where n is x.elems length
//use memcpy() function or a for loop to copy the values of x.elems

Finally add a class destructor to your class:

~Set() {
     if(elems != nullptr) //check for empty array
         delete[] elems;
 }

Have in mind bjarne stroustrup the creator of c++ says that you should follow RIII (called R tripple i) . Basicly when you create a class always use new in constructor and delete in destructor. Avoid new/delete in any other function. Passing pointers can be tricky.

KostasRim
  • 2,053
  • 1
  • 16
  • 32