35

I created a function designed to get user input. It requires that memory be allocated to the variable holding the user input; however, that variable is returned at the end of the function. What is the proper method to free the allocated memory/return the value of the variable?

Here is the code:

char *input = malloc(MAX_SIZE*sizeof(char*));
int i = 0;
char c;

while((c = getchar()) != '\n' && c != EOF) {
    input[i++] = c;
}

return input;

Should I return the address of input and free it after it is used?

Curious as to the most proper method to free the input variable.

w m
  • 493
  • 1
  • 5
  • 7
  • If you `malloc`, eventually someone is going to need to `free`, be it you in another api or the caller. Related, if supported on your target platform(s), [`getline`](http://pubs.opengroup.org/stage7tc1/functions/getdelim.html), a library function added to POSIX.1-2008, would seem to *nearly* do what you're trying to implement. It still requires the caller to eventually `free` any returned allocations, but also provides reuse semantics, where your code does *not*. Depending on intended usage and target platform(s), it may be worth considering. – WhozCraig Apr 15 '15 at 16:14

3 Answers3

29

It's quite simple, as long as you pass to free() the same pointer returned by malloc() it's fine.

For example

char *readInput(size_t size)
 {
    char *input;
    int   chr;
    input = malloc(size + 1);
    if (input == NULL)
        return NULL;
    while ((i < size) && ((chr = getchar()) != '\n') && (chr != EOF))
        input[i++] = chr;
    input[size] = '\0'; /* nul terminate the array, so it can be a string */
    return input;
 }

 int main(void)
  {
     char *input;
     input = readInput(100);
     if (input == NULL)
         return -1;
     printf("input: %s\n", input);
     /* now you can free it */
     free(input);
     return 0;
  }

What you should never do is something like

free(input + n);

because input + n is not the pointer return by malloc().

But your code, has other issues you should take care of

  1. You are allocating space for MAX_SIZE chars so you should multiply by sizeof(char) which is 1, instead of sizeof(char *) which would allocate MAX_SIZE pointers, and also you could make MAX_SIZE a function parameter instead, because if you are allocating a fixed buffer, you could define an array in main() with size MAX_SIZE like char input[MAX_SIZE], and pass it to readInput() as a parameter, thus avoiding malloc() and free().

  2. You are allocating that much space but you don't prevent overflow in your while loop, you should verify that i < MAX_SIZE.

orlp
  • 112,504
  • 36
  • 218
  • 315
Iharob Al Asimi
  • 52,653
  • 6
  • 59
  • 97
  • He can't use local arrays, since when it returns and the function's stack frame gets destroyed, there will be no way of guaranteeing the array will be intact. The compiler will give you a warning: "function returns address of local variable [-Wreturn-local-addr]". Other than that, good answer. – Enzo Ferber Apr 15 '15 at 16:37
  • 1
    @EnzoFerber I meant pass the array to the function and fill it there, I know that you must not return a local array. I think I need to clarify that. – Iharob Al Asimi Apr 15 '15 at 17:19
  • 1
    Good example. Thank you. – George Jun 18 '21 at 15:08
6

You could write a function with return type char*, return input, and ask the user to call free once their done with the data.

You could also ask the user to pass in a properly sized buffer themselves, together with a buffer size limit, and return how many characters were written to the buffer.

orlp
  • 112,504
  • 36
  • 218
  • 315
6

This is a classic c case. A function mallocs memory for its result, the caller must free the returned value. You are now walking onto the thin ice of c memory leaks. 2 reasons

First ; there is no way for you to communicate the free requirement in an enforceable way (ie the compiler or runtime can't help you - contrast with specifying what the argument types are ). You just have to document it somewhere and hope that the caller has read your docs

Second: even if the caller knows to free the result he might make a mistake, some error path gets taken that doesnt free the memory. This doesnt cause an immediate error, things seem to work, but after running for 3 weeks your app crashes after running out of memory

This is why so many 'modern' languages focus on this topic, c++ smart pointers, Java, C#, etc garbage collection,...

pm100
  • 48,078
  • 23
  • 82
  • 145