Oh yes. Destructor Troubles

Hi all,

i'm using VC++ .NET 7.0 and have the following probem with the following code.

Code:

---

// Dummy.h
//
#pragma once

class Dummy
{
public:
        Dummy(void);
        ~Dummy(void);
       
        void CpyStr(char *test);

private:
        char *clone_str;

};



// Dummy.cpp
//
#include "StdAfx.h"
#include ".\dummy.h"
#include <fstream>

using namespace std;

Dummy::Dummy(void)
{
}

Dummy::~Dummy(void)
{
        if (clone_str)
                delete clone_str;
}

void Dummy::CpyStr(char *test)
{
        clone_str = new char[strlen(test)];
        strcpy(clone_str, test);
        cout << "Clone: " << clone_str << "\n";
}


// destructorTest.cpp : Definiert den Einstiegspunkt für die Konsolenanwendung.
//

#include "stdafx.h"
#include "Dummy.h"

Dummy MyDummy;

int _tmain(int argc, _TCHAR* argv[])
{

        MyDummy.CpyStr ("TestString");
        return 0;
}

---

the code in the destructor crashes the programm. I get the dialog a seen in the image below.

Can somebody please tell me why it is so ?

You need to allocate memory for the null terminator ('\0') and must use delete[] when you use new[].

Code:

---

Dummy::~Dummy(void)
{
        if (clone_str)
                delete[] clone_str;
}

void Dummy::CpyStr(char *test)
{
        clone_str = new char[strlen(test)+1];
        strcpy(clone_str, test);
        cout << "Clone: " << clone_str << "\n";
}

---

You could avoid these memory allocation/deallocation troubles completely by using std::string in your c++ code instead.

[edit]This post is partially redundant.[/edit]

Code:

---

        clone_str = new char[strlen(test)];
        strcpy(clone_str, test);

---

You forgot to allocate enough space for the nul terminator. This causes a buffer overrun when you call strcpy. Once you have overrun a buffer the behaviour is undefined.

On another matter, you need to make sure that your objects are stable at all times. Consider this code.

Code:

---

{
  Dummy MyDummy;
} /* Destructor runs here. */

---

With this code, your program will probably crash. This is because the clone_str member is not set to NULL in the constructor and so contains a random value. Your destructor will then try to delete this random value. To fix this, you need to initialize clone_str to NULL

Code:

---

Dummy::Dummy(void) : clone_str(NULL)
{
}

---

If this syntax looks odd, you can read up on constructor initializer lists.

thanks guys for your explanation, now it works correctly with the suggested changes ....

Thanks!
:)

For future reference, "damage after normal block" usually means you've done an array index out of bounds :) It's a helpful feature of MSVC: It will allocate more memory than you ask for, and add some padding memory before and after your array, and if you modify it then it will often catch you, knowing that you've overstepped your array bounds, and it will crash with a suitable error instead of doing something horribly sinister without you ever noticing. :)

It stops catching you, I'm pretty sure, when you compile on Release mode. So yeah, just don't do that until you're sure you've got all the bugs out.

Quote:

---

Originally Posted by Hunter2

For future reference, "damage after normal block" usually means you've done an array index out of bounds :) It's a helpful feature of MSVC: It will allocate more memory than you ask for, and add some padding memory before and after your array, and if you modify it then it will often catch you, knowing that you've overstepped your array bounds, and it will crash with a suitable error instead of doing something horribly sinister without you ever noticing. :)

---

you are right! it's quite useful. I didn't know it is so.
Better to see a dialog than reboot from time to time. :)

Quote:

---

Originally Posted by Hunter2

It stops catching you, I'm pretty sure, when you compile on Release mode. So yeah, just don't do that until you're sure you've got all the bugs out.

---

ok, the aim is always to produce crashfree and stabil software ;)

Thanks!