pointer initialization in assignment operator

[can-mohan]

Hi,

While executing below code i am getting error like "memory clobbered before allocated block" and due to this assignment is not happening. could you let me know what can be reason for the same. I do understand it is not good to use raw pointer but in current scenario i don't have other than any option.

#include<iostream.h>


using namespace std;

class Derived1 
{ 
int *m_p;
public:
Derived1 (int *m_p):m_p(m_p){};
Derived1& operator =( const Derived1 &obj);
void show()const  {cout<<"integer address="<<(void*)m_p<<endl;
                   cout<<"integer="<<*m_p<<endl;
}

~Derived1(){ 

delete m_p;

}
};




 Derived1& Derived1::operator=(const Derived1 &obj)
       {
         if (this!=&obj)
           {
        cout<<"We are Here";
        int *orig =m_p;
        m_p=new int (*obj.m_p);
        delete orig;

           }   
        cout<<"objects are same";
        return *this;

       }

int main()
{
 int x=110;
int y=20; 
  Derived1 s1(&x) ;

  s1.show();
  Derived1 s2(&y);

  s2.show();
  s2=s1;
  s1.show();
  s2.show();




}

[Banfa]

Problem 1: is at lines 45 and 48. You are passing pointers to objects that are on the stack but your class assumes that the objects are on the heap and treats the pointer accordingly. Calling delete on a pointer to a stack object is always going to fail.

In fact you could make your class safer by not passing a pointer in the constructor at all, if it needs to use a pointer internally that is fine, pass an int by value and have the constructor allocate the memory for it internally, that is keep all the memory management hidden inside the class. If in reality the code is using something a little more complex that an int then pass a const reference, leave the calling code resonsible for its memory and you class responsible for its internal memory. If the design requires that the pointer is passed in then use a smart pointer and let the pointer handle the memory deallocation.

Problem 2: You have no copy constructor. The posted code makes no use of it so it has shown up as a problem, however best practice is if you need an assignment operator then you need a copy constructor and vice versa as generally they do very similar operations.

[Moschops]

You can only use delete on a pointer if the memory it points to was allocated by new.

[priyanka.choudhary.90038]

I hope below code answer your quetion.

#include<iostream.h>


using namespace std;

class Derived1 
{ 
int *m_p;
public:
Derived1(int val=0):m_p(new int (val)){*m_p=val;}
Derived1& operator =( const Derived1 &obj);
void show()const  {cout<<"integer address="<<(void*)m_p<<endl;
                   cout<<"integer="<<*m_p<<endl;
}

~Derived1(){ 

//delete m_p;

}
};




 Derived1& Derived1::operator=(const Derived1 &obj)
       {
         if (this!=&obj)
           {
        cout<<"We are Here";
        int *orig =m_p;
        m_p=new int (*obj.m_p);
        delete orig;

           }   
        cout<<"objects are same";
        return *this;

       }

int main()
{

 Derived1 s1(10) ;

  s1.show();

  Derived1 s2(20);

  s2.show();
  s2=s1;
  s1.show();
  s2.show();

}

[can-mohan]

Thanks priyanka. It is working as expected so now i don't require setter in my class like void setval(int) { *m_p=val;} to initialize the pointer