need little help of bug fixers.

[vkash]

here is a code which add data of two classes using operator overloading. But it hangs due to double calling of destructor for same pointer. But i think it should not so.please tell me how to remove this problem from this code.(error is most probably in red region)

#include<iostream>
using namespace std;
class x
{
    private:
    int *******;
    int* pointer;

    public:
    x(int,int);
    x(const x&);
    ~x();
    x operator+(x x1) const;
    void get() const;
    int geti()
    {
        return *******;
    }
    int getp()
    {
        return (*pointer);
    }
    void change(int a,int b)
    {
    *******=a;
    pointer=new int(b);
    cout<<"\t\t"<<pointer<<endl;
    cout<<"change"<<endl;
    }
};
x::x(int *******x=1,int pointerx=1)
{
    try
    {
         *******=*******x;
         pointer=new int(pointerx);
         cout<<"constrctor"<<endl;
    }
    catch(bad_alloc &ex)
    {
        cout<<"ERROR "<<ex.what()<<endl;
    }

}
x::x(const x & x5 )
{
    *******=x5.*******;
    int*tmp=new int(*(x5.pointer));
    pointer=tmp;
    cout<<"copy constructor"<<endl;
}
x::~x()
{
    cout<<"destructor  ";
    cout<<*******<<endl;
    delete pointer;
}
void x::get() const
{
    cout<<*******
    <<endl
    <<*pointer
    <<endl;
}
/*x x::operator+(x xn) const
{
    cout<<"we get this data\n";
    cout<<*******<<endl<<*pointer<<endl;
    xn.get();
    cout<<"that's all\n";
    int tmp*******=xn.*******+*******;
    int tmppointer=((*(xn.pointer))+(*(pointer)));

    x *returnx=new x(tmp*******,tmppointer);
    cout<<"addition coplete"<<endl;
    return (*returnx);
}*/
x operator+(x,x);
int main()
{
    x x1(1,1);
    x x2(10,20);
    x1.get();
    x2.get();
    x x3;
    x3=x1+x2;
    x1.get();
    x2.get();
    x3.get();
    cout<<"sum of 2 and 1 is "<<2+1<<endl;
    return 0;
}
x operator+(x x1f,x x2f)
{
    cout<<"add function..."<<endl;
    int xir=x1f.geti()+x2f.geti();
    int xprv=x1f.getp()+x2f.getp();
    cout<<xir<<"\t"<<xprv<<endl;
    x xr(xir,xprv);
    cout<<"returning..."<<endl;
    return xr;
}
[/color]
//why these guys(codeguru mangers) 
put ******in place of      int i ger (remove space ) 
this is not abusive word)???????all the places where you see 
******  in this post i write their inti ge r(without space)

as you can see their are two overload for addition but one is hidden. I hide that because in returning of that function destructor is called for all the class x variable and all pointer loose their data.
If i call any class function(operator+ of class x) using class variable then it's destructor(main caller like x1.get() not destructor) should not called at the last of that function but in case of operator+ (of class x) this happen destructor is called for all.why. Is it done in special cases like operator overloading???

There should not any error in second operator+(after main function). Error is due to calling of destructor of xprv is deleted at the end of this function but it should not matter pointer value in xr because this is created dynamically in constructor.(but i am wrong why???)
thanks to all helper.

[Philip Nicoletti]

1) just change the name so the "******" don't show up. It is difficult
to follow the code otherwise.

2) you need to supply operator =.

If you need one of : copy constructor , operator = , destructor ...
you usually need all 3.

[vkash]

1) just change the name so the "******" don't show up. It is difficult
to follow the code otherwise.

2) you need to supply operator =.

If you need one of : copy constructor , operator = , destructor ...
you usually need all 3.

philip I have no intension to show up i put real code but "int ig ers"(remove space) word s appearing as ******* I don't know why this happening.
(Type int ig er(without space ) in any post you will see that it appear as ******* it's not my fault.)

I don't understand second point can you please explain in better way(briefly).

[Paul McKenzie]

I don't understand second point can you please explain in better way(briefly).

If you do not know what a copy-assignment operator is, then you need to know the basics. You should not be attempting to write overloaded operators if you don't understand these principles:

class foo
{
    private:
         int *ptr;

    public:
         foo() { ptr = new int; }
        ~foo() { delete ptr; }
};

int main()
{
   foo f1;
   foo f2 = f1;  // initialization using copy construction
   foo f3;
   f3 = f1;   // assignment
}

This code has memory leaks and attempts to delete the same pointer three times. What is missing from the foo() class to make the main code run correctly?

1) It is missing a user-defined copy constructor to properly handle the pointer member whein initializing a new object from an existing object.

2) It is missing an operator = (copy assignment operator) to handle assigning an existing object from an existing object.

So let's skip the asterisks for now, as all that other code you posted contains mostly noise that has nothing to do with your issue. Just fix the class above so that it works correctly. Once you do that, then it should be understandable as to what makes a class have correct copy semantics.

Regards,

Paul McKenzie

[vkash]

If you do not know what a copy-assignment operator is, then you need to know the basics. You should not be attempting to write overloaded operators if you don't understand these principles:

class foo
{
    private:
         int *ptr;

    public:
         foo() { ptr = new int; }
        ~foo() { delete ptr; }
};

int main()
{
   foo f1;
   foo f2 = f1;  // initialization using copy construction
   foo f3;
   f3 = f1;   // assignment
}

This code has memory leaks and attempts to delete the same pointer three times. What is missing from the foo() class to make the main code run correctly?

1) It is missing a user-defined copy constructor to properly handle the pointer member whein initializing a new object from an existing object.

2) It is missing an operator = (copy assignment operator) to handle assigning an existing object from an existing object.

So let's skip the asterisks for now, as all that other code you posted contains mostly noise that has nothing to do with your issue. Just fix the class above so that it works correctly. Once you do that, then it should be understandable as to what makes a class have correct copy semantics.

Regards,

Paul McKenzie

I have modified code see this. this time with no *******

 
#include<iostream>
using namespace std;
class x
{
    private:
    int i;
    int* p;

    public:
    x(int xi,int xp);
    x(const x&);
    ~x();
    void get() const;
    int geti() const ;
    int getp() const;
    x& operator=(const x&);
    x& operator+(const x&);
    void change(int ,int);
};
x::x(int xi=1,int xp=1)
{
    cout<<"constructor start..."<<endl;
    i=xi;
    p=new int(xp);
    cout<<"constructor  end..."<<endl;
}
x::x(const x& xc)
{
    cout<<"copy constructor start ..."<<endl;
    int it=xc.geti();
    int pt=xc.getp();
    this->change(it,pt);
    cout<<"copy constructor  end  ..."<<endl;
}
x::~x()
{
    cout<<"destructor start..."<<endl;
    delete p;
    cout<<"destructor  end ..."<<endl;
}
void x::get() const
{
    cout<<"void get() start ..."<<endl;
    cout<<"*******:   "<<i<<endl;
    cout<<"pointer:   "<<*p<<endl;
    cout<<"void get()  end ... "<<endl;
}
int x::geti() const
{
    return i;
}
int x::getp() const
{
    return (*p);
}
void x::change(int a,int b)
{
    cout<<"void change() start ..."<<endl;
    i=a;
    delete p;
    int*op=new int(b);
    p=op;
    cout<<"void change()  end  ..."<<endl;
}
x& x::operator=(const x& xn)
{
    cout<<"operator= start..."<<endl;
    int it=xn.i;
    int pt=xn.getp();
    change(it,pt);
    cout<<"result"<<endl<<i<<"\t"<<*p<<endl;
    return *this;
}
x& x::operator+(const x& xop)
{
	cout<<"opearator+ start...\n";
    int itmp=this->geti()+xop.geti();
    int ptmp=this->getp()+xop.getp();
    x* xreturn=new x(itmp,ptmp);
	cout<<"operator+ end  ...\n";
    return *xreturn;
}
int main()
{
    //x* x1=new x(10,20);
    x x2;
    /*cout<<"first round(view orignal)"<<endl;
    x1->get();
    x2.get();
    cout<<"\n\nsecond round(x2=x1 then view)"<<endl;
    x2=(*x1);
    x1->get();
    x2.get();
    cout<<"\n\nthird round(delete x1)"<<endl;
    delete x1;
    //x1->get();
    x2.get();
    int i;
    cout<<"read then clear screen(press any key)";
    cin>>i;
    system("cls"); */
    x x3;
    x x4(12,13);
    x3=(x4+x2);
    cout<<"result"<<endl;
    x3.get();
    return 0;
}

tell me is it causing memory leak. because destructor for highlighted part of program is not called But destructor for x3 is called. so is there any memory leak or not?
If there is any other error then please tell me about that.

thanks paul and philip for helping.

[S_M_A]

Regarding memory leaks (and how to detect them) see this thread http://www.codeguru.com/forum/showthread.php?t=507470

[vkash]

Regarding memory leaks (and how to detect them) see this thread http://www.codeguru.com/forum/showthread.php?t=507470

very good link.
i tr these code lines in my code

//at head
#define _CRTDBG_MAPALLOC
#include <stdlib.h>
#include <crtdbg.h>
. ..  .. . . .   .. . .. .   .
 . .. . . . . . . . .

//just before end of return of main function 
_CrtDumpMemoryLeaks();

results in output window is amazing it says that a lot of memory leaked

Detected memory leaks!
Dumping objects ->
{158} normal block at 0x00354B58, 4 bytes long.
 Data: <!   > 21 00 00 00 
{157} normal block at 0x00354CF8, 4 bytes long.
 Data: <!   > 21 00 00 00 
{156} normal block at 0x00354CB0, 8 bytes long.
 Data: <     L5 > 16 00 00 00 F8 4C 35 00 
{155} normal block at 0x00354B98, 4 bytes long.
 Data: <    > 0D 00 00 00 
{141} normal block at 0x00354A38, 4 bytes long.
 Data: <    > 14 00 00 00 
Object dump complete.
The program '[2408] class.exe: Native' has exited with code 0 (0x0).

i have done all things as written in web page but it is not giving information about the line where memory leak occur. can you tell me why?

[Paul McKenzie]

very good link.
i tr these code lines in my code

You need to fix other issues with your program.

x::x(const x& xc)
{
    cout<<"copy constructor start ..."<<endl;
    int it=xc.geti();
    int pt=xc.getp();
    this->change(it,pt);

void x::change(int a,int b)
{
    cout<<"void change() start ..."<<endl;
    i=a;
    delete p;

You are deleting an uninitialized pointer.

As to your other issues:

x& x::operator+(const x& xop)

Why are you returning a reference? An operator + is supposed to return an x object, not an x reference. Here is the correction.

x x::operator+(const x& xop)

Now look inside that function -- you're doing this:

    x* xreturn=new x(itmp,ptmp);  // Wrong!

There is no reason to use "new", and you doing this is the cause of the memory leaks. You need to remember one thing -- whatever you use to call "new", delete must be issued. So tell us -- where is the "delete" for this xreturn object? Is it in your code anywhere? No.

Here is the correction:

x x::operator+(const x& xop)
{
  int itmp=this->geti()+xop.geti();
  int ptmp=this->getp()+xop.getp();
  return x(temp, ptmp);
}

No call to new whatsoever. How does this work? An x object is created locally, and is returned since the function returns an x. You don't need "xreturn" or anything like that. When the object is returned, the copy constructor takes over and creates another object that gets sent back to the caller.

Regards,

Paul McKenzie

[vkash]

x x::operator+(const x& xop)
{
  int itmp=this->geti()+xop.geti();
  int ptmp=this->getp()+xop.getp();
  return x(temp, ptmp);
}

this is a new thing for me but it is little bit different than that of general return. It does not work in returning by reference.
see it i try this before making making dynamic class in operator+ function

    cout<<"opearator+ start...\n";
    int itmp=this->geti()+xop.geti();
    int ptmp=this->getp()+xop.getp();
    x wert(itmp,ptmp);
    cout<<"operator+ end...\n";
    return wert;//x(itmp,ptmp);

It hangs in middle of execution a screen appears say class.exe has stopped working then i have to clik on close program.
reason for hang: after coming wert to main function it is temporary data so when we call operator= it makes error
What is difference in two returns.
Due this error i make return data in dynamic memory.

since any of my data is not greater than 100 so can i make this change in code to cure void x::change()

void x::change(int a,int b)
{
    cout<<"void change() start ..."<<endl;
    i=a;
    if(  (*p<-1000) || (*p>1000)  )
         delete p;
    else
        cout<<"class members are not initialized \a\a\a"<<endl;
    int*op=new int(b);
    p=op;
    cout<<"void change()  end  ..."<<endl;
}

this is not the ultimate cure but work in this code.

thanks paul for answering/helping .

[vkash]

I have got the solution of my own question correct code which will work fine is here

#define _CRTDBG_MAP_ALLOC
#include <stdlib.h>
#include <crtdbg.h>
#include<iostream>
using namespace std;
class x
{
    private:
    int i;
    int* p;

    public:
    x(int xi,int xp);
    x(const x&);
    ~x();
    void get() const;
    int geti() const ;
    int getp() const;
    x& operator=(const x&);
    x operator+(const x&);
    void change(int ,int);
	void tk()
	{
		cout<<"tk\n";
		i =1;
		p=new int(0);
	}
};
static int m(0);
x::x(int xi=1,int xp=1)
{
	
    cout<<"constructor start..."<<endl;
    i=xi;
    p=new int(xp);
	m++;
    cout<<"constructor  end..."<<m<<endl<<endl;
	
}
x::x(const x& xc)
{
    cout<<"copy constructor start ..."<<endl;
	//cout<<"going to copy in "<<i<<"  "<<*p<<endl;
    int it=xc.geti();
    int pt=xc.getp();
	this->tk();
    this->change(it,pt);
	m++;
    cout<<"copy constructor  end  ..."<<endl;
}
static int j;
x::~x()
{
	
    cout<<"destructor start..."<<endl;
    delete p;
	j++;
    cout<<"destructor  end ..."<<j<<endl<<endl;
	
}
void x::get() const
{
    cout<<"void get() start ..."<<endl;
    cout<<"*******:   "<<i<<endl;
    cout<<"pointer:   "<<*p<<endl;
    cout<<"void get()  end ... "<<endl<<endl;
}
int x::geti() const
{
    return i;
}
int x::getp() const
{
	int a=*p;
    return a;
}
void x::change(int a,int b)
{
    cout<<"void change() start ..."<<endl;
    i=a;
    delete p;
    int*op=new int(b);
    p=op;
    cout<<"void change()  end  ..."<<endl;
}
x& x::operator=(const x& xn)
{
    cout<<"operator= start..."<<endl;
    int it=xn.i;
    int pt=xn.getp();
    change(it,pt);
    cout<<"result"<<endl<<i<<"\t"<<*p<<endl;
    return *this;
}
x x::operator+(const x& xop)
{
	cout<<"opearator+ start...\n";
    int itmp=this->geti()+xop.geti();
    int ptmp=this->getp()+xop.getp();
    //x* xreturn=new x(itmp,ptmp);
	x tr(itmp,ptmp);
	cout<<"operator+ end  ...\n";
    return tr;//x(itmp,ptmp);//*xreturn;
}
int main()
{
    x x3;
	x x2(10,20);
    x x4(100,200);
    x3=(x4+x2);
    cout<<"result"<<endl;
    x3.get();
	cout<<m<<"   "<<j<<endl;
	_CrtDumpMemoryLeaks();
    return 0;
}

i use static variable to get information that is there any kind of memory leak. as i found number of times constructor and copy constructor called is equal to number of times destructor called but still there are memory leak reported by header and _CrtDumpMemoryLeaks(); .
Dynamic memory is also created by tk function but that is deleted by change and the memory created by change is deleted by destructor(i may wrong in this paragraph)
see what comes in my output windows

'class.exe': Loaded 'E:\Users\Vikash Chandola\Documents\Visual Studio 2008\Projects\class\Debug\class.exe', Symbols loaded.
'class.exe': Loaded 'E:\Windows\System32\ntdll.dll'
'class.exe': Loaded 'E:\Windows\System32\kernel32.dll'
'class.exe': Loaded 'E:\Windows\System32\KernelBase.dll'
'class.exe': Loaded 'E:\Program Files\AVAST Software\Avast\snxhk.dll'
'class.exe': Loaded 'E:\Windows\winsxs\x86_microsoft.vc90.debugcrt_1fc8b3b9a1e18e3b_9.0.30729.1_none_bb1f6aa1308c35eb\msvcp90d.dll'
'class.exe': Loaded 'E:\Windows\winsxs\x86_microsoft.vc90.debugcrt_1fc8b3b9a1e18e3b_9.0.30729.1_none_bb1f6aa1308c35eb\msvcr90d.dll'
Detected memory leaks!
Dumping objects ->
{144} normal block at 0x005149B0, 4 bytes long.
 Data: <    > DC 00 00 00 
{140} normal block at 0x00514AC8, 4 bytes long.
 Data: <    > C8 00 00 00 
{139} normal block at 0x00514A88, 4 bytes long.
 Data: <    > 14 00 00 00 
Object dump complete.
The program '[2964] class.exe: Native' has exited with code 0 (0x0).

this output also not give information about the line of memory leak.
so here is only one thing remain that is memory leak where is that in program?

[D_Drmmr]

int main()
{
    x x3;
	x x2(10,20);
    x x4(100,200);
    x3=(x4+x2);
    cout<<"result"<<endl;
    x3.get();
	cout<<m<<"   "<<j<<endl;
	_CrtDumpMemoryLeaks();
    return 0;
}

You call _CrtDumpMemoryLeaks before the destructors of the local variables in main are called. The easiest way to prevent this is to create an additional block inside main.

int main()
{
    {
        x x3;
        // etc.
    }
    _CrtDumpMemoryLeaks();
}

That way, all local variables in the block are destroyed before _CrtDumpMemoryLeaks is called.

[Paul McKenzie]

I have got the solution of my own question correct code which will work fine is here

All you had to do was to initialize the p pointer to NULL in the copy constructor. There is no need to introduce tk or any other member variables.

x::x(const x& xc) : p(0)
{
    int it=xc.geti();
    int pt=xc.getp();
    this->change(it,pt);
}

Also, your operator + must work for this:

x x::operator+(const x& xop)
{
    int itmp=this->geti()+xop.geti();
    int ptmp=this->getp()+xop.getp();
    return x(tmp, ptmp);
}

If it doesn't, then that means you still have bugs in your code. Changing the code around to create a temporary is not a solution -- all you're doing is moving the bug somewhere else in the code.

Regards,

Paul McKenzie`

[Paul McKenzie]

Due this error i make return data in dynamic memory.

You need to first understand why an error exists before coding changes. All you did when you changed the code to do dynamic allocation is to make the program more wrong than it already was.

since any of my data is not greater than 100 so can i make this change in code to cure void x::change()

Even though you changed it, the pattern that I'm seeing when you're writing code is that you come up with whatever you think is OK, but not research if it really is OK. That is not the way to learn C++.

void x::change(int a,int b)
{
    cout<<"void change() start ..."<<endl;
    i=a;
    if(  (*p<-1000) || (*p>1000)  )
         delete p;
    else
        cout<<"class members are not initialized \a\a\a"<<endl;
}

You never delete dynamic memory conditionally, while at the same time you're always unconditionally allocating it. What if I constructed the x object like this?

x foo(1000000, 1000000);

Your constructor that takes two arguments gladly accepts those arguments. Now if "change" is called, you are conditionally deleting the pointer p, which is wrong -- p was allocated in the two argument constructor. Thus you now have a memory leak.

Regards,

Paul McKenzie

[vkash]

Reply to paul

All you had to do was to initialize the p pointer to NULL in the copy constructor. There is no need to introduce tk or any other member variables.

x::x(const x& xc) : p(0)
{
    int it=xc.geti();
    int pt=xc.getp();
    this->change(it,pt);
}

this is good idea i forget to use such things next time i will use it if required.

Also, your operator + must work for this:

x x::operator+(const x& xop)
{
    int itmp=this->geti()+xop.geti();
    int ptmp=this->getp()+xop.getp();
    return x(tmp, ptmp);
}

yeah it works fine. i try to know why other are not working that's why i try others.

You need to first understand why an error exists before coding changes. All you did when you changed the code to do dynamic allocation is to make the program more wrong than it already was.

I know why it was wrong there when i return a class x data from operator+ to main then it is temporary just as it comes to main it is sent to operator= is called which is taking a temporary data this makes error(in some cases wrong deletion of data). other sources of error are deleting that memory which is even not initialized that's why i put tk function(to initialize if not initialized) .

Even though you changed it, the pattern that I'm seeing when you're writing code is that you come up with whatever you think is OK, but not research if it really is OK. That is not the way to learn C++.

you are also right here. i make these cross cuts(i mean wrong way of removing bugs) in program because i did not found any other idea. I work in this code for 5 hours even after that this code makes a lot error(hang) then making cross cuts(as i did) is only idea come in mind. next time i will try to not use such bad methods(as i use in this like dynamic memory).


we can change constructor to not take value greater than 100 less than 0

x::x(int xi=1,int xp=1)
{
	bool d(0);
	if((xi>100)||(xp>100)||(xi<0))||(xp<0))
	{
		cout<<"setting by default large data \n";
	d=1
	}
	
    cout<<"constructor start..."<<endl;
    i=xi;
    p=new int(xp);
	if(d=1)
	{
		i=1;
		p=new int(1);
	}
	m++;
    cout<<"constructor  end..."<<m<<endl<<endl;
	
}
//i did not test it

I think this time constructor is made correctly. If execution enter first if then middle part is stuff but i don't know how to still let that to not work in this case one can make other function which called if d=0,d=1 one time one will b executed but it will increase code length(making them inline will boost execution i think so).
Now i think all bugs are fixed with your copy constructor and constructor that i propose. In all thing i write here i may wrong so (as you do usually) tell me if i am wrong.
special thanks for telling me that i am moving in wrong path of study.


reply to D_Drmmr
thanks for this idea it works. I was searching for a tool to detect memory leak since last month. S_M_A and you give me this tool. thanks to both guys.

[S_M_A]

vkash, read the entire thread. There's no need to call _CrtDumpMemoryLeaks(). The easiest way that I've found to enable CRT memory leak detection is as described in my post #12 of that thread.

In post #31 you also find an experimental project made in an attempt to solve John E's issue. If you use that project as an example the memleaks show up with source line references and you just have to double-click that line or press F4 to jump from place entry to entry.