Returned string value becomes garbage

Question

Im trying to pass a character array value to a character pointer. then this value gets returned to a method that is calling it, but after it gets returned, the value becomes garbage. can anyone help me?

#include <stdio.h>

const char * getname(){
     char nam[10];
     char * name;
     gets(nam);
     name = nam;
     return name;
}
main(){
       printf("%s",getname());
       getch();
}

everything is fine, until the string gets returned

Stop returning the address of a local variable in a called function. What you're doing is undefined behavior. And use `fgets()` rather than `gets()`. That function is so bad its been outright eliminated as of C11, deprecated in C99. — WhozCraig, Feb 22 '13 at 08:50
I wish it were easy to search for this issue, it's so common for beginners to run into. Also, OP, you don't need the pointer variable. Arrays decay into pointers to the first element when necessary, you could have just (still incorrectly) written: `return nam`; — Ed S., Feb 22 '13 at 08:53
possible duplicate of [Return char\[\]/string from a function](http://stackoverflow.com/questions/14416759/return-char-string-from-a-function) — Mahmoud Al-Qudsi, Feb 22 '13 at 09:07
Let's also remember that `gets()` is one of those dangerous functions with no buffer overflow protection. You're better off using `fgets()` such as with http://stackoverflow.com/questions/4023895/how-to-read-string-entered-by-user-in-c/4023921#4023921. — paxdiablo, Feb 22 '13 at 11:02
Remember to [upvote or accept](http://meta.stackexchange.com/a/168143/206447) an answer you found helpful. — Bernhard Barker, Feb 27 '13 at 12:24

Bernhard Barker · Answer 1 · 2013-02-22T09:02:13.077

3

The nam variable has function scope. This means that once the function ends, the memory for that variable is freed. So the pointer pointing to that memory that you return will not be valid any more.

You could pass in the pointer: (a bit pointless in this case, as you can see)

void getname(char *name)
{
  gets(name);
}

You could malloc (bad, because then you need to free it again at some point):

const char * getname(){
     char * name = malloc(10);
     gets(name);
     return name;
}

edited Feb 22 '13 at 09:02

answered Feb 22 '13 at 08:51

Bernhard Barker

54,589
14
104
138

2

+1 for mentioning also 'how this common problem is usually tackled' design-wise – Roman Saveljev Feb 22 '13 at 08:53
Yes, in C you have to watch what was allocated and has to be freed where all the time. C++ can do most of that for you, but in C there is no escape from it. – Jan Hudec Feb 22 '13 at 08:55
1

You have small error in your code correct is as `gets(name);` – Grijesh Chauhan Feb 22 '13 at 09:01

Grijesh Chauhan · Accepted Answer · 2013-02-22T10:49:31.027

1

Scope of nam is local to function getname(), you are returning nam address through name pointer

const char * getname(){
     char nam[10];
     :
     name = nam;
     return name;
}

allocate memory for nam; dynamically. like:

nam = malloc(sizeof(char)*10);

additionally there may be bufferoverun don't use gets(), do like:

nam = malloc(sizeof(char)*10);
fgets( nam, 10, stdin );

You also not need to use name an extra variable simple return nam is good.

const char * getname(){
     char * nam = malloc(sizeof(char)*10);
     fgets( nam, 10, stdin ); 
     return nam;
}

edited Feb 22 '13 at 10:49

answered Feb 22 '13 at 08:51

Grijesh Chauhan

57,103
20
141
208

2

It's not necessary to write `sizeof(char)` as it's 1 by definition. – Jan Hudec Feb 22 '13 at 08:57
1

@JanHudec yes not necessary :) – Grijesh Chauhan Feb 22 '13 at 08:58
1

Agreed. Extra `sizeof(char)` doesn't hurt, forgetting `sizeof(int)` sure would. – Jan Hudec Feb 22 '13 at 10:32
@JanHudec Yes that's why I believe it good practice to use `sizeof` Thanks! – Grijesh Chauhan Feb 22 '13 at 10:48
Actually it clogs your code unnecessarily (IMO). And if you want to safeguard yourself from type changes, you should use something like: `char *nam = malloc (10 * sizeof(*nam));`. Still it's not my style to downvote "competing" answers, especially when the infraction is minor :-) – paxdiablo Feb 22 '13 at 11:00
1

@paxdiablo Thanks for the suggestion. yes! `char *nam = malloc (10 * sizeof(*nam));` is correct choice. Thanks. And we are not "competing" we are together helping. :) – Grijesh Chauhan Feb 22 '13 at 11:06

paxdiablo · Answer 3 · 2013-02-22T08:57:01.977

Your problem is that return name is returning the address of a stack variable, one that's gone out of scope after the function returns.

There are a couple of ways to fix this (at least).

The first is to have the address allocated outside of the function and then passed in:

char *getname (char *buff) {
    strcpy (buff, "pax");
    return buff;
}

char name[20];
printf ("Name is '%s'\n", getname (name));

The second is to use allocation functions that don't go out of scope on function exit (the pointers may but, as long as you pass them back, you can still get to the allocated memory):

char *getname (void) {
    char *buff = malloc (21);        // should really check this for failure.
    strcpy (buff, "pax");
    return buff;
}

buff = getname();
printf ("Name is '%s'\n", buff);
free (buff);                         // This is important, caller is responsible
                                     //   for freeing the memory.

score 0 · Answer 4 · answered Feb 22 '13 at 12:11

0

Declaring nam static would do also:

const char * getname() {
  static char nam[10];
  ...

Beware: This code isn't thread-safe, as nam now is treated as if it were declared globally.

answered Feb 22 '13 at 12:11

alk

69,737
10
105
255

Returned string value becomes garbage

4 Answers4

Linked