Copy constructor demo (crashing... case 2)

Question

Please have a glance at this program:

class CopyCon
{
public:
char *name;

CopyCon()
{ 
    name = new char[20];        
    name = "Hai";//_tcscpy(name,"Hai");
}

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

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

int main()
{
    CopyCon obj1;
    CopyCon obj2(obj1);
    cout<<obj1.name<<endl;
    cout<<obj2.name<<endl;
}

This program crashes on execution. Error: "Expression: _BLOCK_TYPE_IS_VALID(pHead->nBlockUse)"

If I assign "Hai" to name using aasignment operator, its crashing. Where as when I use string func _tcscpy to assign "Hai" to name, its working perfectly. Can some one explain why so?

It's kind of funny how only 10k reputation users post an answer to your problem. And basically they all say the same. Does that mean that one gets access to higher level knowledge when reaching 10k reputation? — DaClown, Jun 07 '10 at 08:30
You don't need to check for null before delete. passing NULL to delete is OK. — Pete Kirkham, Jun 07 '10 at 08:36
Why are you using `_tcslen` and `_tcscpy` with a `char` array? — JoeG, Jun 07 '10 at 09:00
Same problem as here: http://stackoverflow.com/questions/255612/c-dynamically-allocating-an-array-of-objects/255744#255744 Basically use a std::string rather than do memory management yourself. — Martin York, Jun 07 '10 at 09:05
Joe Gauterin: _tcslen() and _tcscpy() are not functions, but macros aliases for the versions of these functions that match your program's default character set, as determined by whether or not _UNICODE and/or _MBCS are #defined. — Akaanthan Ccoder, Jun 07 '10 at 11:20

score 4 · Answer 1 · answered Jun 07 '10 at 08:20

4

 name = "Hai";//_tcscpy(name,"Hai");

You are not copying contents of "Hai" into name instead name will point to a read only memory ( whose contents are "Hai") if you try to delete name later then it might crash.

answered Jun 07 '10 at 08:20

aJ.

34,624
22
86
128

There is no more to say here. – fmuecke Jun 07 '10 at 08:24
Except that C++ makes no guarantee that "Hai" is stored in read-only memory. – Jun 07 '10 at 08:26
1

It makes a guarantee that calling delete[] on a string literal results uin undefined behaviour though. – JoeG Jun 07 '10 at 08:52

score 3 · Accepted Answer · answered Jun 07 '10 at 08:21

In the default constructor

CopyCon()
{ 
    name = new char[20];        
    name = "Hai";//_tcscpy(name,"Hai");
}

you assign the address of a string literal to the pointer and in the destructor you call delete[] on it, that's undefined behavior. delete[] should only be called on addresses returned by new[].

When you instead use _tcscpy() you copy the literal content to the buffer allocated by new[] and then the destructor runs fine.

score 2 · Answer 3 · answered Jun 07 '10 at 08:20

When you use assignment, you make the pointer name point at the string literal "Hai". This later gets deleted in the destructor. However, the string literal was not allocated with new, and cannot be deleted like this, so you get undefined behaviour. You can only deallocate with delete things you allocated with new. This has nothing to do with the copy constructor.

score 2 · Answer 4 · answered Jun 07 '10 at 08:24

name = new char[20];        
name = "Hai";//_tcscpy(name,"Hai");

Here you are not copying the data into the memory allocated by new. Instead you are assigning a new value to pointer name which points at read-only location (in most cases). Since this memory was not allocated using new you can not do delete on it. Also, note that you have a memory leak here as the memory allocated using new char[20]; is never deleted.

score 2 · Answer 5 · answered Jun 07 '10 at 08:49

The very same program, but in C++:

struct CopyCon
{
  CopyCon(): name("HAI") {}
  std::string name;
};

int main(int argc, char* argv[])
{
  CopyCon obj1;
  CopyCon obj2(obj1);
  cout<<obj1.name<<endl;
  cout<<obj2.name<<endl;
}

Mine works, is clear, and I typed less than you did ;)

score 0 · Answer 6 · answered Jun 07 '10 at 08:22

What you do in that code is that you allocate a memory block for the name (assign an address to name pointer). Then you actually overwrite this address by the address of the string literal "Hai" (which ceases to exist after the constructor finishes). Thats why you get the error, since the destructor tries to free memory which does not belong to you. ( You did not allocate it ).

Copy constructor demo (crashing... case 2)

6 Answers6