Returning string from function produces garbage

Question

Having a little trouble with this simple program. I can solve this by making response[10] a global variable but I don't want to do that. Program tests for a proper response and works but the return string is garbage:

#include <stdio.h>
#include <string.h>

char *user_string(char *Setting_Type[]);

int main()
{
char *response;
char *test_names[2] = {"Test", "test"};

printf("Enter \"Test\" or \"test\": ");
response = user_string(test_names);
printf("\nCorrect! Your input is: %s\n", response);

return 0;
}
char *user_string(char *Setting_Type[])
{
int loop = 1;
char response[10];
char *response_string;

while(loop = 1)
    {
    scanf("%s", &response);
    response_string = response;

    if(strcmp(response_string, Setting_Type[0]) != 0 && strcmp(response_string, Setting_Type[1]) != 0)
        printf("\nWrong! Please try again: ");
    else
        break;
    }

return response_string;
}

Do you mean to use an assignment operator in a conditional statement? `while(loop = 1)`. Should it be?: `while(loop == 1)` — ryyker, Oct 26 '13 at 04:18
There were several issues to address in your code, mostly having to do with string handling, scope issues, and memory allocation and freeing. To much to address in this comment. See my answer below. — ryyker, Oct 26 '13 at 04:49
@Blastfurnace - Do not agree with specific reason sited, as OP does not include that question in his query. I agree, he is asking a question that goes directly to the root issue of scope, but as with many that are new to C, scope is not on the forefront of what he is observing. Learning of scope will eventually solve his issue, but for now, learning the reasons why the string is garbled, and the techniques of how to deal with strings working or not working from function call to return is sufficient reason to allow this question to remain active. — ryyker, Oct 26 '13 at 05:06
@ryyker Determining whether or not questions are duplicates isn't just whether or not the questions are asking the same thing; they can also be marked as duplicates if the underlying issue ends up being the same. — Dennis Meng, Oct 26 '13 at 06:27
Besides, if you wanted to get really technical, there are also questions with "why is my string garbled" that are also because of this. — Dennis Meng, Oct 26 '13 at 06:27
I have found that SO has lots of responses and quick too but its hard to find an answer that's tailor-made for your specific question. I always search the site for an appropriate response first. Besides if all the answers were already given previously then there's really no need for a discussion board that induces active reasoning and activity. — Samuel, Oct 26 '13 at 06:41

ryyker · Accepted Answer · 2013-10-26T04:52:45.847

Your scanf() function needed to be edited
from scanf("%s", &response);
to scanf("%s", response);.
That will fix part of the problem.

Since you do not want to use globals, why can you not put another argument in

char *user_string(char *Setting_Type[], char *response_string) ?

You would have to allocate memory for it, and free it in the calling function (main()), but it would work in this scenario. (really should be providing some memory for it anyway, in its current usage)

Example: [Tested, works]

#include <ansi_c.h>
#include <stdio.h>
#include <string.h>

char *user_string(char *Setting_Type[], char *s);

int main()
{
    char *response;
    char *test_names[2] = {"Test", "test"};
    char *resp;

    resp = malloc(80);//I picked 80, you can pick something more appropriate
    response = malloc(80);//I picked 80, you can pick something more appropriate
    printf("Enter \"Test\" or \"test\": ");
    //user_string() returns a char *, so just call it in printf()
    printf("\nCorrect! Your input is: %s\n", user_string(test_names, response));
    free(resp);
    free(response);

    return 0;
}

char *user_string(char *Setting_Type[], char *response_string)
{
    int loop = 1;
    char response[10];

    while(loop == 1)
        {
        scanf("%s", response); //removed &
        strcpy(response_string,response);//changed from `=` to `strcpy()`

        if(strcmp(response_string, Setting_Type[0]) != 0 && strcmp(response_string, Setting_Type[1]) != 0)
            printf("\nWrong! Please try again: ");
        else
            break;
        }

    return response_string;
}

Thank you for that detailed response. I am just learning about malloc and don't quite understand it altogether. Doesn't the user's input only take in ten characters into response[10] and the rest goes to stdin? — Samuel, Oct 26 '13 at 05:10
You're welcome, thank you for accepting! In `user_string()`, response[10] can take _up to_ ***9*** chars from stdin, there has to be one left for the nul terminator '\0' `scanf()` brings in data from stdin, expecting it to conform to the format string, "%s" in this case, and will append '\0' to that input if it has room. A non-fatal warning "Attempt to write beyond string" occurs if you enter 10 (or more) chars, and a fatal error if you `strcpy()` what you have, into a pointer (such as response_string). Try it with this code, and use a break point in debug, so you can view the variables. — ryyker, Oct 26 '13 at 16:15

score 0 · Answer 2 · answered Oct 26 '13 at 03:58

response is an array local to user_string(), it'll go out of scope the moment that function returns, you can't use it from main(), here. You'd need to either malloc() memory for it in user_string(), or pass in a buffer from main(). Many, many duplicates of this question on SO.

score 0 · Answer 3 · answered Oct 26 '13 at 03:59

0

You are returning an address of a local array, which stops existing after the return statement.

Also this line scanf("%s", &response); introduces the possibility of a buffer overflow.

answered Oct 26 '13 at 03:59

this

5,229
1
22
51

1

Not to mention it should be written `scanf("%s", response);` (no & for char *) – ryyker Oct 26 '13 at 04:31
@ryyker It works both ways since `&response == response`. – this Oct 26 '13 at 05:54

Returning string from function produces garbage

3 Answers3