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?

Answer 1

• 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".

Answer 2

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.

Answer 3

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;
}

Answer 4

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.