3

I have the following code:

 char* gen()
 {
     char out[256];
     sprintf(out, ...); // you don't need to know what's in here I don't think

     return out;
 }

And I am getting this error when I try to compile:

ERROR: function returns address of local variable

I've tried having this return char[] and char with no luck. Am I missing something?

stefan
  • 10,215
  • 4
  • 49
  • 90
Cassidy
  • 3,328
  • 5
  • 39
  • 76
  • 2
    Yes, when the function returns the contents of `out` will be destroyed. – Brian Bi Mar 09 '14 at 22:13
  • 2
    The proper answer depends on the used language. Are you programming in `C` or `C++`? Decide, tag accordingly, and _please_ never use both tags again, it's almost always wrong. – stefan Mar 09 '14 at 22:15
  • 1
    If `c++` is allowed here, maybe use `std::string`. – gongzhitaao Mar 09 '14 at 22:16
  • @stefan, in both languages local variables are destroyed when the function returns. – vonbrand Mar 09 '14 at 22:18
  • 2
    @vonbrand yes, but that's not the point. The question obviously leads to "how to avoid this? / how to return strings?". The answer to this differs quite a lot in these two languages. – stefan Mar 09 '14 at 22:19
  • possible duplicate of [Can a local variable's memory be accessed outside its scope?](http://stackoverflow.com/questions/6441218/can-a-local-variables-memory-be-accessed-outside-its-scope) – chris Mar 09 '14 at 22:21

3 Answers3

5

Your char array variable out exists only inside the function's body.
When you return from the function, the content of out buffer can't be accessed anymore, it's just local to the function.

If you want to return some string from your function to the caller, you can dynamically allocate that string inside the function (e.g. using malloc()) and return a pointer to that string to the caller, e.g.

char* gen(void)
{   
    char out[256];
    sprintf(out, ...);

/* 
 *   This does NOT work, since "out" is local to the function.
 *
 *   return out;
 */

    /* Dynamically allocate the string */
    char* result = malloc(strlen(out) + 1) /* +1 for terminating NUL */

    /* Deep-copy the string from temporary buffer to return value buffer */
    strcpy(result, out);

    /* Return the pointer to the dynamically allocated buffer */
    return result;
    /* NOTE: The caller must FREE this memory using free(). */
}

Another simpler option would be to pass the out buffer pointer as a char* parameter, along with a buffer size (to avoid buffer overruns).

In this case, your function can directly format the string into the destination buffer passed as parameter:

/* Pass destination buffer pointer and buffer size */
void gen(char* out, size_t out_size)
{   
    /* Directly write into caller supplied buffer. 
     * Note: Use a "safe" function like snprintf(), to avoid buffer overruns.
     */
    snprintf(out, out_size, ...);
    ...
}

Note that you explicitly stated "C" in your question title, but you added a [c++] tag. If you can use C++, the simplest thing to do is to use a string class like std::string (and let it manage all the string buffer memory allocation/cleanup).

Stargateur
  • 24,473
  • 8
  • 65
  • 91
Mr.C64
  • 41,637
  • 14
  • 86
  • 162
  • Rather than using the non-portable, Microsoft-specific function, you could instead use the function defined in the C99 standard, `snprintf`. – dreamlax Mar 09 '14 at 22:38
  • @dreamlax: Thanks for the suggestion. I adjusted the code accordingly. – Mr.C64 Mar 09 '14 at 22:40
  • How would you free the memory after that? Would it be in the method itself, or afterwards? – Cassidy Mar 10 '14 at 00:55
  • @CassidyWilliams: The function returns a pointer to a dynamically allocated string (using `malloc()`), you use this string at the call site and wherever you need it (outside the function body), and when you don't need it anymore, you can just release the memory calling `free()` on the string pointer (to avoid the so called _dangling references_, you may also want to set the pointer to `NULL` after the call to `free()`). – Mr.C64 Mar 10 '14 at 12:26
3

When you use the following declaration in the function char out[256]; the space allocated will be freed once the function returns, so it doesn't make sense to return a pointer to the out char array.

If you want to return a pointer to a string you create in the function you should use malloc() as in

char* out = (char*)malloc(256*sizeof(char));

which allocates space for 256 chars but should be freed manually at some point with the free() function.

Or as suggested in the comment by Brian Bi pass a char * that points to the string you want to use as an argument to your function.

Community
  • 1
  • 1
juan.facorro
  • 9,791
  • 2
  • 33
  • 41
  • I think you mean `sizeof(char)` (or better yet `sizeof *out`) instead of `sizeof(256)`, since you only need size for 256 `char`s, not 256 `int`s. – Kninnug Mar 09 '14 at 22:22
  • @Kninnug Yeah, that's what I actually wanted to write. Thanks! – juan.facorro Mar 09 '14 at 22:23
  • 1
    the problem with the caller passing in a pointer to a buffer is that you dont know how big it is - an accident waiting to happen – pm100 Mar 09 '14 at 22:37
  • @pm100 I agree, although you have the same problem by allocating the memory for the buffer in the function, the caller doesn't the size of the returned buffer. You can always provide the size of the buffer as an argument along with the buffer itself or have a `#define BUFSIZE 256` to determine the size, it always depends on how you structure your code and the conventions you want to establish. – juan.facorro Mar 09 '14 at 22:43
  • Where would you free the memory after that? Would it be locally in the method itself, or afterwards? – Cassidy Mar 10 '14 at 00:56
  • @CassidyWilliams Once you no longer need to use the contents of `out` or the allocated memory. Since you are returning the pointer `out` I guess you want to use the values it holds, so the freeing would need to happen afterwards, in the code that calls the `gen()` function. – juan.facorro Mar 10 '14 at 02:00
1

The problem is that when the function gen returns (exits) its local variables (such as out) run out of scope and are no longer accessible to the caller. So, when you return out you return a pointer to memory that is no longer allocated.

There are two options to 'return' a pointer/buffer from a function:

  1. Allocate the buffer in the calling function and pass it to gen:

    char out[256];
    gen(out, sizeof out);
    

    it is common to also provide the size of the buffer you pass in, since the called function cannot know this. This means that you have to change the declaration of gen to:

    void gen(char * out, size_t size){
    

    You could also hard-code the size of the incoming buffer to 256 (since you have it hard-coded in your gen function right now):

    void gen(char out[256]){
    

    That means you must provide a variable of type char[256] to gen (and no other pointers or arrays). But it does allow you to do sizeof out inside gen.

  2. Allocate the buffer dynamically inside the function:

    char * out = malloc(256 * sizeof *out);
    // ...
    return out;
    

    this has as advantage that the declaration of gen does not change. But it does mean that the calling function has to free the returned buffer when its done with it.

Kninnug
  • 7,992
  • 1
  • 30
  • 42