Exception Error in Destructor

Question

I know this is very basic but somehow working on different technologies I have mashed up my C++ concepts

I have created a simple program but it is giving exception when the destructor is called.

Below is the code:

#include "stdafx.h" #include<iostream> using namespace std; class Employee { public: Employee(char *name){ cout<<"Employee::ctor\n"; myName_p = new char(sizeof(strlen(name))); myName_p = name; } void disp(){cout<<myName_p;} ~Employee() { cout<<"Employee:dtor\n\n"; delete myName_p; } private: char *myName_p; }; int main() { Employee emp("check"); emp.disp(); return(0); }

Requesting all to clear this basic concept. As per my understanding we can't use delete[] because we are not using new[] in this case. Though I have tried using delete[] , but still it was giving error

I'd like a new "close vote" reason: "C++ code doesn't use std::string" — Kerrek SB
– Kerrek SB, Commented Aug 12, 2013 at 15:10
The last two lines of your constructor would be hard-pressed to leak memory faster. This isn't Java. And sizeof(strlen(name)) is the size of a size_t, not the size of the block of memory needed for the parameter. Consider trying it like this — WhozCraig
– WhozCraig, Commented Aug 12, 2013 at 15:19
You need [] to create the array. Otherwise you are creating a single char! — Neil Kirk
– Neil Kirk, Commented Aug 12, 2013 at 16:08

Mats Petersson · Accepted Answer · 2013-08-12 15:16:30Z

You REALLY should use std::string here.

It is so much easier, especially for a beginner. The list of errors is:

you are calculating the wrong size of the name, it should be strlen(name)+1, not using sizeof anything.
You also should use new char[strlen(name)+1].
You are copying the data from the string supplied as the argument to the constructor, use strcpy rather than name_p = name - the latter leaks the memory you just allocated, and then you have a pointer to a const char * which you should not delete.
If you fix the allocation so that it is correct, you should then use delete [] name_p;.

However, it you instead use std::string, all of the above problems go away completely, you can just do:

Employee(char *name) name_p(name) { ... }

and get rid of all the problematic new, delete and copying. Of course, name_p is probably no longer a suitable name for the variable, but you get the idea.

+1 and beyond that, const std::string& for the parameter (or at the very least const char *).
+1, you've addressed everything. Also name_p was never a suitable name :)

Yang · Accepted Answer · 2013-08-12 16:37:02Z

Change

myName_p = new char(sizeof(strlen(name))); myName_p = name;

to

myName_p = strdup(name);

and #include <cstring>. This creates new space and copies the parameter string. In this way, you'll have to call free instead of delete in your destructor.

Otherwise, after the second assignment, you have assigned the string literal "check" to myName_p, and the newly created space is discarded. Then your destructor tries to delete "check" rather than the allocated space, which results in crash.

Also, it is better practice to use std::string rather than old char* strings:

class Employee { public: Employee(char *name): myName_p(name) { cout<<"Employee::ctor\n"; } void disp(){ cout << myName_p; } private: std::string myName_p; };

The string class will manage memory for you.

Going with this as an answer as this is a solution to the specific query and Yang was early to answer. Though Mats explanation is complete
Note that using strdup, you should call free rather than delete in the destructor.

Collectives™ on Stack Overflow

Exception Error in Destructor

2 Answers 2

2 Comments

2 Comments

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

2 Comments

Related