Copy constructor demo (crashing...)

Question

Here is the program...

class CopyCon
{
public:
char *name;

CopyCon()
{ 
    name = new char; 
}

CopyCon(const CopyCon &objCopyCon)
{
    name = new char;
    _tcscpy(name,objCopyCon.name);
}

~CopyCon()
{
    if( name != NULL )
    {
        delete name;
        name = NULL;
    }
}
};

int main()
{
    CopyCon objCopyCon1;
    objCopyCon1.name = "Hai";
    CopyCon objCopyCon2(objCopyCon1);
    objCopyCon1.name = "Hello";
    cout<<objCopyCon2.name<<endl;
    return 0;
}

Once the code execution completes, when the destructor called, it crashes on 'delete' saying...

Debug Error!

Program: ...

HEAP CORRUPTION DETECTED: after Normal block (#124) at 0x00366990. CRT detected that the application wrote to memory after end of heap buffer.

(Press Retry to debug the application)

Don't we have to clear the heap memory in destructor. What's wrong with this program? Pls someone help! Copy constructor works perfectly as intended. But still... !?

Answer 1

The problem is you are allocating only one char in the copy constructor.

In main you are assigning a 4-byte string (remember the null), but when you copy the object, you only allocate enough room for 1 byte.

What you probably want to do is change

name = new char;

to

name = new char[tcslen(objCopyCon.name) + 1];

And in the destructor:

delete name;

to

delete [] name;

Also:

You are assigning "Hai" and "Hello" to objCopyCon1.name which is hiding the memory allocated in the constructor. This memory can never be freed!

Answer 2

You write past the allocated variable and that is undefined behavior.

When the folloing lines run

 CopyCon objCopyCon1;
 objCopyCon1.name = "Hai";
 CopyCon objCopyCon2(objCopyCon1);

_tcscpy() copies 4 characters (3 letters and the null terminator) into a buffer that can legally hold only one character. So you write past the buffer end and this leads to heap corruption.

You need to alocate the buffer of the right size:

CopyCon(const CopyCon &objCopyCon)
{
    name = new char[_tcslen(objCopyCon.name) +1];
   _tcscpy(name,objCopyCon.name);
}

also you need to change the delete in the destructor to delete[] and also change all other new calls to new[] to avoid undefined behavior.

Answer 3

The job of the copy ctor should be to create a copy of the object. So the char array pointed to by name in the both the objects should be of the same size and same content, which is not happening in your case. So change

name = new char; // allocates only one char

to

name = new char[strlen(objCopyCon.name) + 1]; // allocate array of char

Answer 4

You need to allocate enough memory to hold the information you are trying to store. "Hai" is 4 bytes or chars (including the null terminator) and you have only allocated one. You also do not copy strings from one memory location to another using "=". You need to strncpy the string across.

Use std::string it will make your life a million times easier :)

Answer 5

You are allocating one character and trying copy multiple characters into that memory location. First find out the length of the string then allocate length + 1 characters (extra char to accommodate the NULL character) using new char[length+1] syntax. You need to correspondingly change your destructor to delete[] name.

Answer 6

Besides the new char issue that everyone mentioned, the strings "Hai" and "Hello" reside in read-only memory. This means you cannot delete them (but you do so in your destructor) - this does generate crashes. Your code should not assign to name directly, but use a set function such as:

void set_name(const char *new_name)
{
    delete [] name; // delete is a no-op on a NULL pointer
    name = new char[tcslen(new_name) + 1];
    _tcscpy(name,new_name);
}

I'm surprised that assignment does not produce a compiler warning to be honest. You are assigning a const char * to a char *, which can lead to all sorts of problems like the one you're seeing.

Answer 7

Here the code that works perfect!

class CopyCon
{
public:
char *name;

CopyCon()
{ 
    name = NULL;        
}

CopyCon(const CopyCon &objCopyCon)
{
    name = new char[_tcslen(objCopyCon.name)+1];
    _tcscpy(name,objCopyCon.name);
}

~CopyCon()
{
    if( name != NULL )
    {
        delete[] name;
        name = NULL;
    }
}

void set_name(const char *new_name) 
{ 
    //delete [] name; // delete is a no-op on a NULL pointer 
    if( NULL != name)
    {
        delete[] name; name = NULL;
    }
    name = new char[_tcslen(new_name) + 1]; 
    _tcscpy(name,new_name); 
} 
};

int main()
{
     CopyCon objCopyCon1;
objCopyCon1.set_name("Hai");
CopyCon objCopyCon2(objCopyCon1);
objCopyCon1.set_name("Hello");
cout<<objCopyCon1.name<<endl;
cout<<objCopyCon2.name<<endl;
return 0;
}

Thanks to all for their view points. It really helped!