tags:

views:

122

answers:

8

Suppose I have a class

class person
{
char* name;
public:
void setname(const char*);
};
void person::setname(const char* p)
{
name=new char[strlen(p)];
strcpy(name,p);
name[strlen(p)]='\0';
}

My question is about the line name=new char[strlen(p)]; suppose the p pointer is pointing to string i.e “zia” , now strlen(p) will return 3 it means we have an array of 4 characters i.e char[3] now I copy the string into the name and at the 4th location , I put the null character , what is wrong with this?????

+2  A: 

The problems are

  1. You never delete[] name;, thus every time the user calls setname(), you leak an array.
  2. To accomodate the extra '\0' you need to allocate for strlen(p)+1 elements.
KennyTM
+5  A: 

You say:

we have an array of 4 characters i.e char[3]

Surprisingly enough, char[3] is an array of THREE characters, not FOUR!

anon
I am so sorry, I don't know what the hell I was thinking!My apologies, I've been fasting and am at the end of a long day. :S
Computer Guru
+2  A: 

You need to allocate one more memory position for the \0 character otherwise when you do this name[strlen(p)]='\0'; you get a Segmentation Fault. Basically do new char[strlen(p) + 1].

ruibm
+2  A: 

You should allocate strlen(p)+1 characters:

name = new char[strlen(p)+1];
Philippe Leybaert
+3  A: 

You MUST allocate one more char for zero terminator:

name = new char[strlen(p) + 1];
Dewfy
+2  A: 

The point where you're going wrong is here:

now strlen(p) will return 3 it means we have an array of 4 characters i.e char[3]

An array of 4 characters is a char[4]; if you allocate dynamically, you will have to pass 4 to operator new or, in general:

name=new char[strlen(p)+1];
Martin B
+1  A: 
void person::setname(const char* p)
{

name=new char[strlen(p) + 1]; // make a room to have null character 
strcpy(name,p);
name[strlen(p)]='\0';

}

array index starts from 0 so max index for array of size = 5 is arr[4].

Ashish
strcpy puts the 0 terminator in - no need to do it again
James Hopkin
+1  A: 

A lot of people have mentioned a cure for the immediate problem you've encountered. In doing so, they've almost done you a disservice though. Unless you have a really good reason to do otherwise, what you should probably do is define the name as and std::string, and use its assignment operator to handle the job correctly.

If you do have a spectacularly good reason to avoid using std::string, then you should design a string class of your own, and use it instead. At least in my opinion, writing code the way you have, with dynamic allocations and strcpys all over the place is just a poor idea. Even at very best, it's difficult to read, prone to lots of silly off-by-one errors, and essentially impossible to make anywhere close to exception safe.

Jerry Coffin