Failing C Function to Accept User String Array Input

Question

I am trying to create a C function I can call in small programs I write, to accept user input:

char GetStringMine()
{
    int i = 0;
    char ch;
    char * tmpstring = (char *) malloc(2048 * sizeof(char));
    while(ch != '\n')
    {
        ch = getchar();
        tmpstring[i++] = ch;
    }
    tmpstring[i] = '\0';
    return * tmpstring;
    free(tmpstring);
}

But it does not compile. Please can you tell me what I am doing wrong, and what I can do better?

[Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). — Sourav Ghosh, Dec 21 '15 at 11:22
What do you think these two lines `return * tmpstring; free(tmpstring);` will do? — haccks, Dec 21 '15 at 11:23
Every thing happens after `return` statement wont meter anymore. — Michi, Dec 21 '15 at 11:57

score 2 · Answer 1 · edited May 23 '17 at 12:23

2

First thing:

In your code,
```
 while(ch != '\n')
```
invokes undefined behavior, as ch is an automatic local variable and unless initialized explicitly, it contains an indeterminate value. Trying to read an indeterminate value is UB.
Second thing,
```
free(tmpstring);
```
after the return statement has no effect, at all. Just remove it.
Third thing:

Please see this discussion on why not to cast the return value of malloc() and family in C..
Fourth thing:

As per your return statement, the function return type should be of char *, instead of a char.

Finally

But it does not compile

cannot be answered in current form. You need to provide more information in your question to clarify "what" and "how".

edited May 23 '17 at 12:23

Community

1
1

answered Dec 21 '15 at 11:24

Sourav Ghosh

133,132
16
183
261

2

True, but none of these are gonna stop compilation of program. – haccks Dec 21 '15 at 11:26
Sorry. The actual content of comment was changed. May be I pressed some other keys while posting. – haccks Dec 21 '15 at 11:28
@haccks Right sir, that's why I added the last statement in the _answer_. :) – Sourav Ghosh Dec 21 '15 at 11:34

MikeJ · Answer 2 · 2015-12-21T12:00:45.077

0

Try this...

static char tmpstring[2048] = {0};

char* GetStringMine()
{
    int i = 0;
    char ch = 0;

    while(ch != '\n')
    {
        ch = getchar();
        tmpstring[i++] = ch;
    }

    return tmpstring;  
}

You cannot free the allocated memory then return a pointer to it, the data will be gone
No need to use malloc, as you are not dynamically allocating the array(1)
Return a char*, not a char

(1) In your example you effectively are fixing the size of memory to 2048. Think about protecting against a buffer overrun - what will happen if a user enters more than 2048 characters, and how will you protect your code against this. In a real world application you would need to reallocate if going over the allotted size, or restrict the amount of input for the memory allocated.

edited Dec 21 '15 at 12:00

answered Dec 21 '15 at 11:34

MikeJ

2,367
3
18
23

4

Your program is returning a pointer to a local variable (`tmpstring`) which will also be gone once the function has been called. Correct your answer or you'll certainly get downvotes. – Jabberwocky Dec 21 '15 at 11:35
Thanks, how can I dynamically allocate the array? I would like not to set it to fixed length. – Unpossible Dec 21 '15 at 12:00

seleciii44 · Answer 3 · 2015-12-21T11:44:54.360

0

You are trying to return a char pointer(char*) but the return type is a char. Also you should consider other comments too.

char* GetStringMine()
{
    int i = 0;
    char ch;
    char * tmpstring = (char *) malloc(2048 * sizeof(char));
    while(ch != '\n')
    {
        ch = getchar();
        tmpstring[i++] = ch;
    }
    tmpstring[i] = '\0';
    return  tmpstring;
}

edited Dec 21 '15 at 11:44

answered Dec 21 '15 at 11:35

seleciii44

1,529
3
13
26

1

No. He is not trying to return `char *`. – haccks Dec 21 '15 at 11:36
he is, but he does not know that. – seleciii44 Dec 21 '15 at 11:38
He isn't, he's dereferencing the char pointer which evaluates to a char. – user2697817 Dec 21 '15 at 11:43

score 0 · Answer 4 · answered Dec 21 '15 at 11:50

The worst thing about this code is that you're trying return a pointer to a local variable. The variable tmpstring is destroyed after the execution of your function is complete (i.e. on your return statement).

To correct this, you should ask a char* as a parameter, and store the read characters in it (careful with the overflows if you're going with this solution).

Or, you could declare tmpstring as a static char tmpstring[2048] = {0};. static means it won't be destroyed after the function is over. Although I've seen that kind of things in the standard library sometimes, I wouldn't recommend it since the contents will be erased when the function is called again.

For the other problems, see the previous answers.

Failing C Function to Accept User String Array Input

4 Answers4