What is wrong in this strcpy()?

User 5495012

What is wrong in this strcpy()? I tried this function call, no compile time errors, but results in error during runtime. What could be the problem in that function call? char *s= "TEST"; char *s1= NULL; char * s_cpy(char * s,char * s1) { char * p=(char*) malloc(strlen(s)); p=s; printf("%s\n",p); // prints TEST printf("%s\n",s1); // prints printf("%s\n",s); // prints TEST strcpy(s1,p); //error, test.exe has encountered a problem return s1; }

Rajesh R Subramanian

Member 5502879 wrote:

printf("%s\n",s1); // prints

Prints what? Junk? Zero? Null?

Member 5502879 wrote:

strcpy(s1,p); //error, test.exe has encountered a problem

You never allocated memory to s1, but are trying to write to something to it.

Many are stubborn in pursuit of the path they have chosen, few in pursuit of the goal - Friedrich Nietzsche .·´¯`·->Rajesh<-·´¯`·. [Microsoft MVP - Visual C++]

Malli_S

If you're calling s_cpy() function with s, s1 (global variables i.e.

char *s= "TEST";
char *s1= NULL;

) the formal parameter s1 will hold NULL (invalid pointer) which makes strcpy() crash. The prototype for strcpy() is

char *strcpy( char *strDestination, const char *strSource);

Passing invalid pointer at any of the parameter to strcpy (or any string function taking char *) will crash the program.

User 5495012

I am sorry for tat. Since i have already set char *s1=NULL; printf("%s\n",s1); // prints NULL

CPallini

Member 5502879 wrote:

char *s= "TEST"; char *s1= NULL; char * s_cpy(char * s,char * s1) { char * p=(char*) malloc(strlen(s)); p=s; printf("%s\n",p); // prints TEST printf("%s\n",s1); // prints printf("%s\n",s); // prints TEST strcpy(s1,p); //error, test.exe has encountered a problem return s1; }

Good C tutorial needed. :)

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong. -- Iain Clarke
[My articles]

User 5495012

Even when i change s1 to char *s1="TEMP"; strcpy(s1,p); // resulting in error.

SandipG

One suggestion

Member 5502879 wrote:

char * p=(char*) malloc(strlen(s)); p=s;

you allocating buffer and assigning some other pointer to p again. this will cause memory leak.

Regards, Sandip.

Malli_S

char *s1 = "TEMP" defines the char pointer to constant "TEMP", of which you cann't change the value. which makes strcpy() fail.

User 5495012

Even if do not create 'p' and directly copy s to s1, strcpy(s1,s); im still getting the same error.

Rajesh R Subramanian

Try allocating memory to s1 before you copy stuff into it and then tell if it crashes. If it doesn't, well, guess why. :)

Many are stubborn in pursuit of the path they have chosen, few in pursuit of the goal - Friedrich Nietzsche .·´¯`·->Rajesh<-·´¯`·. [Microsoft MVP - Visual C++]

SandipG

I did not answer you why it crashes.. others have already told you the reason. I just pointed the mistake.

Regards, Sandip.

auralius manurung

try allocate memory like this: char * s_cpy(char * s,char * s1) { char * p =(char*) malloc(strlen(s)+1); s1 = (char*) malloc(10); //use this!!!! p=s; printf("%s\n",p); // prints TEST printf("%s\n",s1); // prints printf("%s\n",s); // prints TEST strcpy(s1,p); //error, test.exe has encountered a problem return s1; } don forget to free them up later or you will encounter memory leak...

CPallini

auralius wrote:

char * p =(char*) malloc(strlen(s)+1); s1 = (char*) malloc(10); //use this!!!! p=s;

And what's the point of doing the above? :)

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong. -- Iain Clarke
[My articles]

David Crow

I see two issues.

Member 5502879 wrote:

char * p=(char*) malloc(strlen(s)); p=s;

You've reassigned p to some other address. This will cause problems if you go to free the address returned by malloc().

Member 5502879 wrote:

strcpy(s1,p); //error, test.exe has encountered a problem

Because s1 has not been allocated any memory. strcpy() is therefore dereferencing a null pointer.

"Love people and use things, not love things and use people." - Unknown

"The brick walls are there for a reason...to stop the people who don't want it badly enough." - Randy Pausch

Member 4194593

Several things are wrong. As pointed out, s1 is allocated as just a pointer to a string so you cannot copy data to it. In addition, you allocated only strlen bytes for the p copy, you need to allocate strlen+1 to allow the terminating null for string s. You may get away with this if a paragraph (16 bytes) is probably allocated for the malloc, but it is still bad code.

auralius manurung

allocating memory so i won't be a NULL pointer anymore... :)

Mahesh Kulkarni

I observed following issues 1) Issue of Memory allocation 2) this is the case where we can understand the Global an Local variable concept. Try to name ur sencond argument in list with different name. May be it will work.

The secret of life is not enjoyment but education through experience. - Swami Vivekananda.

CPallini

char * p =(char*) malloc(strlen(s)+1);
s1 = (char*) malloc(10); //use this!!!!
p=s;

allocating memory for p and then setting p=s; make no sense to me. :)

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong. -- Iain Clarke
[My articles]

auralius manurung

yup, i know...i meant allocating memory for s1.. my concern wasn't on part where p=s. i thought application crashed due to NULL pointer on s1...that's why we must allocate memory for s1... :-D