memory lick

[alsade]

can some one plz tell me why theres a memory lick here??

#include <iostream>
#include <string>

using namespace std;

class employee
{
	char* name;
	unsigned int salary;
    public:
	employee();
	employee(char*,unsigned int);
	void Init (char*,unsigned int);
	void print();
	~employee();
};

employee::employee()
{
	name=new char[20];
	salary=1000;
};

employee::employee(char* name,unsigned int salary)
{
    this->name=name;
    this->salary=salary;
}

void employee::Init(char* name, unsigned int salary)
{
    this->name=name;
    this->salary=salary;
}

void employee::print()
{
	cout<<"name: "<<name<<endl;
	cout<<"salary: "<<salary<<endl;
}
employee::~employee()
{
	delete [] name;
}


int main()
{
	employee emp1("John",45000);
	emp1.print();

}

[Zhuge]

I'm not sure what a "memory lick" is, but in your example you are passing the C-string "John" to the constructor (illegal, by the way, as you accept only a mutable char* when the string is const char*), then try to delete it in the destructor.

[alsade]

but i delete the it in the distructor...can u plz explain how can i initialize the name with char*??

[LB]

String literals should never be deleted.

Just change "char*" to "char const *"

[Disch]

To add on to what LB is saying... you should only delete[] what you allocated with new[].

In this case... 'name' is being initialized on line 26 by merely assigning another pointer to it. Note that name is not being assigned to something that has been new[]'d. Therefore you must not delete[] it.

However, you are deleting it in the destructor. This is a problem (likely will cause a program crash).


Your 'Init' function has the same problem. If you default construct the employee class... then you will allocate the buffer with new... but when you call init... you effectively throw the new'd buffer out and replace it with a direct assignment (ie: something that must not be deleted). So you'll still crash because you're deleting something your not supposed to... AND you'll be leaking memory because you never deleted the buffer you new'd.


There are 2 things to remember here:
1) pointers are not strings, they're pointers.
2) when you delete[] a pointer, you're not actually deleting the pointer... you're deleting the data that the pointer points to.



If you are confused by this, that's understandable. And in fact... doing manual memory management is often confusing and error prone, which is why you should avoid it.

In this case... I would recommend you get rid of char pointers entirely and just use strings:

#include <iostream>
#include <string>

using namespace std;

class employee
{
	string name;  // <- use a string, not a char*
	unsigned int salary;
    public:
	employee();
	employee(string,unsigned int);  // <- strings
	void Init (string,unsigned int);  // <- strings
	void print();
	~employee();
};

employee::employee()
{
	//name=new char[20];  // <- don't need to do anything here
	salary=1000;
};

employee::employee(string name,unsigned int salary) // <- string
{
    this->name=name;  // <- since these are not strings, this will do what you expect
    this->salary=salary;
}

void employee::Init(string name, unsigned int salary)  // <- string
{
    this->name=name;  // <- same as above.  This is OK now.
    this->salary=salary;
}

void employee::print()
{
	cout<<"name: "<<name<<endl;
	cout<<"salary: "<<salary<<endl;
}
employee::~employee()
{
	//delete [] name;  <- don't need to do this.  strings clean themselves up.
}


int main()
{
	employee emp1("John",45000);
	emp1.print();
}