problem with destructor

**Gernot Frisch** · Jul 22 '05, 05:15 PM

Re: problem with destructor

"bArT" <bart212@poczta .onet.pl> schrieb im Newsbeitrag
news:ceb496$qs2 $1@nemesis.news .tpi.pl...[color=blue]
> Hi!
> I have a problem with such situation. I have a class like below and[/color]
have[color=blue]
> some pointers (ptr1 and ptr2). I dynamically allocate memory in[/color]
constructor[color=blue]
> and I free in destructor. For one pointer there is some error when
> dustroctor delete objects. Why? What should I have to change?
>
> Please help.
>
> bart
>
> class A
> {
> char *ptr1;
> char *ptr2;[/color]

A() {ptr1=NULL; ptr2=NULL;}

[color=blue]
> A(char *string1, char *string2)
> {[/color]

ptr1=NULL; ptr2=NULL; // because string1 or string2 might be
NULL??
[color=blue]
> if (string1 != NULL)
> {
> ptr1 = new char[strlen(string1) +1];
> strcpy(ptr1, string1);
> }
> if (string2 != NULL)
> {
> ptr2 = new char[strlen(string2) +1];
> strcpy(ptr2, string2);
> }
> }
> virtual ~A()
> {
> if (ptr1 != NULL)
> delete [] this->ptr1;
> if (ptr2 != NULL)
> delete [] this->ptr2;[/color]

ptr1=NULL; ptr2=NULL; // Always do this after delete[] - at
least in debug versions.
[color=blue]
> }
> // other methods. I use there ptr1 and ptr2
> }
>
> main()
> {
>
> A MyClass("text1" , "text2")
> return 0;
> }
>
>[/color]

HTH,
Gernot

**Victor Bazarov** · Jul 22 '05, 05:15 PM

Re: problem with destructor

bArT wrote:[color=blue]
> I have a problem with such situation. I have a class like below and have
> some pointers (ptr1 and ptr2). I dynamically allocate memory in constructor
> and I free in destructor. For one pointer there is some error when
> dustroctor delete objects. Why? What should I have to change?[/color]

You need to adhere to the "Rule of Three". Look it up.
[color=blue]
>
> Please help.
>
> bart
>
> class A
> {
> char *ptr1;
> char *ptr2;
> A(char *string1, char *string2)[/color]

In order for your code to work well, 'string1' and 'string2' here
should be 'const char*'.
[color=blue]
> {
> if (string1 != NULL)
> {
> ptr1 = new char[strlen(string1) +1];
> strcpy(ptr1, string1);
> }
> if (string2 != NULL)
> {
> ptr2 = new char[strlen(string2) +1];
> strcpy(ptr2, string2);
> }[/color]

That is all fine, with one exception: what if 'string1' is NULL?
What value does 'ptr1' get? Same for 'ptr2'. In most cases they
will get garbage in them, which will cause an attempt to delete[]
the memory, which in turn will cause undefined behaviour.

Make sure ptr1 and ptr2 are initialised to 0 before the body:

A(const char* s1, const char* s2) : ptr1(0), ptr2(0)
{
if (s1 ...
[color=blue]
> }
> virtual ~A()[/color]

Do you _really_ need it virtual?
[color=blue]
> {
> if (ptr1 != NULL)
> delete [] this->ptr1;
> if (ptr2 != NULL)
> delete [] this->ptr2;[/color]

Checking is unnecessary. 'this->' is unnecessary.
[color=blue]
> }
> // other methods. I use there ptr1 and ptr2
> }[/color]

;
[color=blue]
>
> main()[/color]

int main()
[color=blue]
> {
>
> A MyClass("text1" , "text2")
> return 0;
> }
>
>[/color]

The syntax errors suggest that you typed your code into the message
instead of copying it from your real project. That might simply not
show the place where the problem was. Next time post _real_ code.

Victor

**Andre Kostur** · Jul 22 '05, 05:15 PM

Re: problem with destructor

"Gernot Frisch" <Me@Privacy.net > wrote in news:2msittFqco t4U1@uni-
berlin.de:
[color=blue]
>
> "bArT" <bart212@poczta .onet.pl> schrieb im Newsbeitrag
> news:ceb496$qs2 $1@nemesis.news .tpi.pl...[color=green]
>> Hi!
>> I have a problem with such situation. I have a class like below and[/color]
> have[color=green]
>> some pointers (ptr1 and ptr2). I dynamically allocate memory in[/color]
> constructor[color=green]
>> and I free in destructor. For one pointer there is some error when
>> dustroctor delete objects. Why? What should I have to change?[/color][/color]

You should first tell us the error....
[color=blue][color=green]
>> class A
>> {
>> char *ptr1;
>> char *ptr2;[/color]
>
> A() {ptr1=NULL; ptr2=NULL;}[/color]

One should prefer the initializer lists to explicit assignment. Doesn't
make much of a different for pointers, but matters more for rich classes.
Better to get into the habit:

A() : ptr1(NULL), ptr2(NULL) {};
[color=blue][color=green]
>> A(char *string1, char *string2)
>> {[/color]
>
> ptr1=NULL; ptr2=NULL; // because string1 or string2 might be
> NULL??[/color]

Same as above:

A(char * string1, char * string2) : ptr1(NULL), ptr2(NULL)
[color=blue][color=green]
>> if (string1 != NULL)
>> {
>> ptr1 = new char[strlen(string1) +1];
>> strcpy(ptr1, string1);
>> }
>> if (string2 != NULL)
>> {
>> ptr2 = new char[strlen(string2) +1];
>> strcpy(ptr2, string2);
>> }
>> }
>> virtual ~A()
>> {
>> if (ptr1 != NULL)[/color][/color]

This test isn't necessary. Performing a delete or delete[] on a NULL
pointer is always safe (it does nothing, but it's safe).
[color=blue][color=green]
>> delete [] this->ptr1;
>> if (ptr2 != NULL)
>> delete [] this->ptr2;[/color]
>
> ptr1=NULL; ptr2=NULL; // Always do this after delete[] - at
> least in debug versions.
>[color=green]
>> }
>> // other methods. I use there ptr1 and ptr2
>> }
>>
>> main()[/color][/color]

int main()

There are no implicit return types in C++.
[color=blue][color=green]
>> {
>>
>> A MyClass("text1" , "text2")
>> return 0;
>> }[/color][/color]

Offhand, did _this_ specific program cause a failure? If not, post the
shortest, compilable program that exhibits the problem.

One common error is that somewhere along the line you might be causing
the copy constructor of A to be invoked, and your copy constructor
currently doesn't do the right thing. As a general hint, anytime you're
playing with dynamically allocated memory in a class (or pretty much any
other "dynamic" resource), you are likely going to need to supply your
own Constructor, Copy Constructor, Assignment operator, and Destructor.
(Or perhaps declare them as private and don't supply a body if you
explicitly want to disallow copy construction, for example)

**bArT** · Jul 22 '05, 05:15 PM

Re: problem with destructor

Doesn't work. I have an error "DAMAGE: after Normal block ....". This is the
error to the same pointer ptr1. When I click Retry I have cursor in
DBGHEAP.H.
What's wrong? I have never had such problem???

bart
[color=blue][color=green]
> > class A
> > {
> > char *ptr1;
> > char *ptr2;[/color]
>
> A() {ptr1=NULL; ptr2=NULL;}
>
>[color=green]
> > A(char *string1, char *string2)
> > {[/color]
>
> ptr1=NULL; ptr2=NULL; // because string1 or string2 might be
> NULL??
>[color=green]
> > if (string1 != NULL)
> > {
> > ptr1 = new char[strlen(string1) +1];
> > strcpy(ptr1, string1);
> > }
> > if (string2 != NULL)
> > {
> > ptr2 = new char[strlen(string2) +1];
> > strcpy(ptr2, string2);
> > }
> > }
> > virtual ~A()
> > {
> > if (ptr1 != NULL)
> > delete [] this->ptr1;
> > if (ptr2 != NULL)
> > delete [] this->ptr2;[/color]
>
> ptr1=NULL; ptr2=NULL; // Always do this after delete[] - at
> least in debug versions.
>[color=green]
> > }
> > // other methods. I use there ptr1 and ptr2
> > }
> >
> > main()
> > {
> >
> > A MyClass("text1" , "text2")
> > return 0;
> > }
> >
> >[/color]
>
> HTH,
> Gernot
>
>[/color]

**tom_usenet** · Jul 22 '05, 05:15 PM

Re: problem with destructor

On Thu, 29 Jul 2004 17:12:01 +0200, "bArT" <bart212@poczta .onet.pl>
wrote:
[color=blue]
>Hi!
>I have a problem with such situation. I have a class like below and have
>some pointers (ptr1 and ptr2). I dynamically allocate memory in constructor
>and I free in destructor. For one pointer there is some error when
>dustroctor delete objects. Why? What should I have to change?
>
>Please help.
>
>bart
>
>class A
>{
> char *ptr1;
> char *ptr2;
> A(char *string1, char *string2)
> {
> if (string1 != NULL)
> {
> ptr1 = new char[strlen(string1) +1];
> strcpy(ptr1, string1);
> }
> if (string2 != NULL)
> {
> ptr2 = new char[strlen(string2) +1];
> strcpy(ptr2, string2);
> }
> }[/color]

The first thing to point out is that any class that provides explicit
resource management for more than one resource is hard to write
correctly - in your case, you're controlling two different char*'s,
which means you have exception safety problems (remember, new can
throw an exception). To get around this (and your other problems),
just use std::string.

Here's the string version:

#include <string>

class A
{
std::string s1;
std::string s2;

A(char const* string1, char const* string2)
:s1(string1 != 0? string1: ""),
s2(string2 != 0? string2: "")
{
}

//other methods
};

And that's it, since the destructor, copy constructor and assignment
operator are all written for you by the compiler.

The moral is: avoid explicit memory management - it's error prone,
makes code verbose and exception unsafe and is a particularly bad idea
for beginners.

Tom

**bArT** · Jul 22 '05, 05:15 PM

Re: problem with destructor

Doesn't work.

[color=blue]
> In order for your code to work well, 'string1' and 'string2' here
> should be 'const char*'.[/color]

const char* doesn't change anything;

[color=blue]
> That is all fine, with one exception: what if 'string1' is NULL?
> What value does 'ptr1' get? Same for 'ptr2'. In most cases they
> will get garbage in them, which will cause an attempt to delete[]
> the memory, which in turn will cause undefined behaviour.[/color]

There isn't such possibility. All strings in constructor are not NULL.
[color=blue]
> The syntax errors suggest that you typed your code into the message
> instead of copying it from your real project. That might simply not
> show the place where the problem was. Next time post _real_ code.[/color]

My real class is big and there is a lot of code. Generally i use dynamically
alocated char arrays. I copy there strings or append in methods and simply
in destructor I want to free memory.

bart

**John Harrison** · Jul 22 '05, 05:15 PM

Re: problem with destructor

On Thu, 29 Jul 2004 17:12:01 +0200, bArT <bart212@poczta .onet.pl> wrote:
[color=blue]
> Hi!
> I have a problem with such situation. I have a class like below and have
> some pointers (ptr1 and ptr2). I dynamically allocate memory in
> constructor
> and I free in destructor. For one pointer there is some error when
> dustroctor delete objects. Why? What should I have to change?
>
> Please help.
>
> bart
>
> class A
> {
> char *ptr1;
> char *ptr2;
> A(char *string1, char *string2)
> {
> if (string1 != NULL)
> {
> ptr1 = new char[strlen(string1) +1];
> strcpy(ptr1, string1);
> }
> if (string2 != NULL)
> {
> ptr2 = new char[strlen(string2) +1];
> strcpy(ptr2, string2);
> }
> }
> virtual ~A()
> {
> if (ptr1 != NULL)
> delete [] this->ptr1;
> if (ptr2 != NULL)
> delete [] this->ptr2;
> }
> // other methods. I use there ptr1 and ptr2
> }
>
> main()
> {
>
> A MyClass("text1" , "text2")
> return 0;
> }
>[/color]

The code you posted does not compile, but with a few changes it will
compile and run correctly.

If you want help with your specific problem instead of general advice on
how to fix the many issues with your code then I suggest you post the
*real* code.

john

**Karl Heinz Buchegger** · Jul 22 '05, 05:15 PM

Re: problem with destructor

bArT wrote:[color=blue]
>
> Doesn't work.
>[color=green]
> > In order for your code to work well, 'string1' and 'string2' here
> > should be 'const char*'.[/color]
>
> const char* doesn't change anything;
>[color=green]
> > That is all fine, with one exception: what if 'string1' is NULL?
> > What value does 'ptr1' get? Same for 'ptr2'. In most cases they
> > will get garbage in them, which will cause an attempt to delete[]
> > the memory, which in turn will cause undefined behaviour.[/color]
>
> There isn't such possibility. All strings in constructor are not NULL.
>[color=green]
> > The syntax errors suggest that you typed your code into the message
> > instead of copying it from your real project. That might simply not
> > show the place where the problem was. Next time post _real_ code.[/color]
>
> My real class is big and there is a lot of code.[/color]

Well. Might be. But otherwise we have no real idea of what might
have goofed up your program. The posted program (when corrected
as suggested) doesn't have any problem, so how should we know
what might be your problem?
[color=blue]
> Generally i use dynamically
> alocated char arrays.[/color]

Bad idea.
You now have seen what this might lead into.
[color=blue]
> I copy there strings or append in methods and simply
> in destructor I want to free memory.
>
> bart[/color]

Then as a first action add a copy constructor and assignment operator
to your class. You can make them private and don't need to implement
them. Then either the compiler or the linker will tell you
where your problem really is.

class A
{
....

private:
A( const A& Arg ); // not implemented by intention
A& operator=( const A& Arg ); // not implemented by intention
};

If I assume that all your string manipulations through strcat, strcpy
are correct, there are 2 possible errors left

* you copy an A object somewhere unexpectedly. Search the web for
'rule of three' to see some explanations of possible pitfalls
for this case, and why the above suggestion can help you to identify
it

* you corrupted memory somewhere else and just see the symptoms in
class A. That's why a small testprogram which just uses your class
in a standalone environment would be a good idea. It can help to
rule out this problem.

--
Karl Heinz Buchegger
kbuchegg@gascad .at

**Victor Bazarov** · Jul 22 '05, 05:15 PM

Re: problem with destructor

bArT wrote:[color=blue]
> Doesn't work.[/color]

What doesn't work? The code you posted wasn't real to start with,
so I couldn't just take it, compile, and run to see it not work.
I gave you some suggestions that could simply make the non-C++ code
you posted slightly better, at least to the extend that it would
compile. What else did you want?

Now, without seeing _real_, _complete_, _compilable_, _minimal_
code that demonstrates the behaviour you claim your program has,
there is no way to give any kind of advice. OK? We are not mind
readers here.

V

**bArT** · Jul 22 '05, 05:15 PM

Re: problem with destructor

OK, here is my class... I don't give the whole implementation of other
methods (because as I noticed it doesn't have an influence at my problem)
but only constructors and destructor.

I have problems with pointers pMessage and pServer.

Please check it and help me.
[color=blue]
> The code you posted does not compile, but with a few changes it will
> compile and run correctly.
>
> If you want help with your specific problem instead of general advice on
> how to fix the many issues with your code then I suggest you post the
> *real* code.
>
> john
>[/color]

class CSmtp
{
public:
//void Initialize();
CSmtp();
CSmtp(int Port, const char *Server, const char *User, const char *Pass,
const char *Title, const char *Message, const char *Receiver, const char
*Sender);
virtual ~CSmtp();

private:
// BOOL RecvMsg();
//void CheckServerErro r();
//BOOL SendMsg(char *pMsgToSend);
char * pReceiver;
char * pSender;
char * pTitle;
char * pMessage;
// int CheckAuth();
char * pPassword;
char * pUser;
// BOOL CheckResponse(c har *pNumber);
// BOOL AuthPlain();
//BOOL AuthLogin();

int nBufferLength;
//BOOL SetBuffer(int nLength);
//BOOL SetTimeout(int nSeconds);
//void CheckSocketErro r();
char *Buffer;
SOCKET Sock;
char *pServer;
int nPort;
// void ClearBuffer();
// void CloseConnection ();
//BOOL SetConnection() ;

};

CSmtp::CSmtp()
{
pMessage = NULL;
pServer = NULL;
pUser = NULL;
pPassword = NULL;
pReceiver = NULL;
pSender = NULL;
pTitle = NULL;
Buffer = NULL;
nBufferLength = 0;

}

CSmtp::CSmtp(in t Port,const char *Server, const char *User,const char *Pass,
const char *Title, const char *Message, const char *Receiver, const char
*Sender)
{
pMessage = NULL;
pServer = NULL;
pUser = NULL;
pPassword = NULL;
pReceiver = NULL;
pSender = NULL;
pTitle = NULL;

nPort = Port;
if (Server != NULL)
{
pServer = new char[strlen(Server+1 )];
strcpy(pServer, Server);
}
Buffer = NULL;
nBufferLength = 0;

if (Message != NULL)
{
pMessage = new char[strlen(Message) +5];
strcpy(pMessage , Message);
}
if (User != NULL)
{
pUser = new char[strlen(User)+1];
strcpy(pUser, User);
}
if (Pass != NULL)
{
pPassword = new char[strlen(Pass)+1];
strcpy(pPasswor d, Pass);
}
if (Title != NULL)
{
pTitle = new char[strlen(Title)+1];
strcpy(pTitle, Title);
}
if (Receiver != NULL)
{
pReceiver = new char[strlen(Receiver )+1];
strcpy(pReceive r, Receiver);
}
if (Sender != NULL)
{
pSender = new char[strlen(Sender)+ 1];
strcpy(pSender, Sender);
}
}

CSmtp::~CSmtp()
{
if (pMessage != NULL)
delete [] pMessage;
if (pUser != NULL)
delete [] pUser;
if (pPassword != NULL)
delete [] pPassword;
if (pTitle != NULL)
delete [] pTitle;
if (pReceiver != NULL)
delete [] pReceiver;
if (pSender != NULL)
delete [] pSender;

if (pServer != NULL)
delete [] this->pServer;
if (Buffer != NULL)
delete [] Buffer;

pMessage = NULL;
pServer = NULL;
pUser = NULL;
pPassword = NULL;
pReceiver = NULL;
pSender = NULL;
pTitle = NULL;
Buffer = NULL;
}

**John Harrison** · Jul 22 '05, 05:15 PM

Re: problem with destructor

On Thu, 29 Jul 2004 19:11:26 +0200, bArT <bart212@poczta .onet.pl> wrote:
[color=blue]
> OK, here is my class... I don't give the whole implementation of other
> methods (because as I noticed it doesn't have an influence at my problem)
> but only constructors and destructor.
>
> I have problems with pointers pMessage and pServer.
>
> Please check it and help me.
>[/color]

Well the first thing to note is that you because it crashes in the
destructor does not mean that the problem is with the destructor (or
constructor). I'd like to know how you've established that the other
methods don't have a bearing because that is the first place I would be
looking.

The only way to be sure that the problem is not with the other methods
would be to remove them entirely and still find that the program crashed.
Have you tried that?

But the first thing to do however it to take Karl's advice and add a
private copy constructor and a private assignment operator. If doing that
causes a compile or link error then you've found your problem. Its very
quick to do and to be honest should have been done already. (I'm assuming
here that the copy constructor and assignment operator aren't two of the
methods that you left out in your post.)

If that doesn't work then I think you are reduced to cutting out whole
sections of code until you can isolate the section that is causing the
problem. Remember that it could be anywhere, just because the code crashes
in the destructor of one class, does not mean that is where the problem is.

Unfortunately there is nothing fatally wrong with the code you've posted
(that I can see). So you are going to have to hunt around or post some
more code.

john

**bArT** · Jul 22 '05, 05:15 PM

Re: problem with destructor

When I deleted all methods except constructor and destructor I still have
the same error. So other methods can't cause a problem but only this
destructor or constructor.

I can't add private assigment operator, because I have such error: "function
'class CSmtp &__thiscall CSmtp::operator =(const class CSmtp &)' already has
a body".
What is Your advice?

bart

[color=blue]
>
> Well the first thing to note is that you because it crashes in the
> destructor does not mean that the problem is with the destructor (or
> constructor). I'd like to know how you've established that the other
> methods don't have a bearing because that is the first place I would be
> looking.
>
> The only way to be sure that the problem is not with the other methods
> would be to remove them entirely and still find that the program crashed.
> Have you tried that?
>
> But the first thing to do however it to take Karl's advice and add a
> private copy constructor and a private assignment operator. If doing that
> causes a compile or link error then you've found your problem. Its very
> quick to do and to be honest should have been done already. (I'm assuming
> here that the copy constructor and assignment operator aren't two of the
> methods that you left out in your post.)
>
> If that doesn't work then I think you are reduced to cutting out whole
> sections of code until you can isolate the section that is causing the
> problem. Remember that it could be anywhere, just because the code crashes
> in the destructor of one class, does not mean that is where the problem[/color]
is.[color=blue]
>
> Unfortunately there is nothing fatally wrong with the code you've posted
> (that I can see). So you are going to have to hunt around or post some
> more code.
>
> john[/color]

**Karl Heinz Buchegger** · Jul 22 '05, 05:16 PM

Re: problem with destructor

bArT wrote:[color=blue]
>
> When I deleted all methods except constructor and destructor I still have
> the same error. So other methods can't cause a problem but only this
> destructor or constructor.
>
> I can't add private assigment operator, because I have such error: "function
> 'class CSmtp &__thiscall CSmtp::operator =(const class CSmtp &)' already has
> a body".
> What is Your advice?[/color]

check that the operator= has a correct implementation.
If you are unsure, post it.

--
Karl Heinz Buchegger
kbuchegg@gascad .at

**Andre Kostur** · Jul 22 '05, 05:16 PM

Re: problem with destructor

"bArT" <bart212@poczta .onet.pl> wrote in
news:cebd9m$1pa $1@nemesis.news .tpi.pl:
[color=blue]
>
> When I deleted all methods except constructor and destructor I still
> have the same error. So other methods can't cause a problem but only
> this destructor or constructor.
>
> I can't add private assigment operator, because I have such error:
> "function 'class CSmtp &__thiscall CSmtp::operator =(const class CSmtp
> &)' already has a body".
> What is Your advice?[/color]

1) Don't top-post

2) Show us your implementation of operator=, you may have an error in
there....

problem with destructor

problem with destructor

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment