views:

313

answers:

7

I was reviewing a friend's code and got into an interesting debate on how C/C++ allocates memory on the stack and manages its release. If I were to create an array of 10 objects in a function, but return said array, does it release when the function pops (hence making the given data invalid) or is it placed into the heap (which raises the question of how do we release it?).

Sample code as follows:

Gene* GetTopTen()
{
    // Create 10 genes (or 10 objects, doesn't matter)
    Gene Ten[10];

    // Sort out external pool data
    Sort();

    // Copy over data to the array of 10 objects
    for(int i = 0; i < 10; Ten[i++] = pool[i]);

    // Here is the core of my question:
    return Ten;
}

Any help is greatly appreciated, this is turning into a very interesting question my friends and I can't answer.

+22  A: 

That's a stack-allocated array, so the returned pointer is invalid.

smorgan
And, a good compiler will warn about this situation.
Greg Hewgill
+3  A: 

That results in undefined behaviour. ten array is stored in the stack. Once the GetTopGene function ends, that stack is destroyed, so you shouldn´t use pointers to that part of memory.

What you want to do is allocate space in the heap.

Gene* GetTopTen(){
        Gene* genes = (Gene*) malloc (10*sizeof(Gene));
        //do stuff.
         return genes;
}

You have to remember to free that memory once you are done with it.

   Gene* genes = GetTopTen();
   //do stuff with it.
   free(genes);
Tom
"What you want to do is allocate space in the heap." I'd say what you want to do is take the destination array as a parameter ;-)
Steve Jessop
+1  A: 

If you want to return an array from a function, you'll have to put it on the heap yourself:

In C

Gene* GetTopTen()
{
    // Create 10 genes (or 10 objects, doesn't matter)
    Gene *Ten = malloc(sizeof(Gene)*10);
    ....    
    // Now it's ok to return
    return Ten;
}

int main()
{
    Gene *genes = GetTopTen();
    free (genes);
}

And in C++:

Gene* GetTopTen()
{
    // Create 10 genes (or 10 objects, doesn't matter)
    Gene *Ten = new Gene[10];
    ....    
    // Now it's ok to return
    return Ten;
}

int main()
{
    Gene *genes = GetTopTen();
    delete [] genes;
}

Of course, it's also up to you to keep track of how long that array is, either by returning a length from GetTopTen in a pointer/reference parameter, or by some sort of constant.

Eclipse
If you are going to provide a C++ alternative, I suggest returning a std::vector is a better idea.
anon
That's not on the stack, that's on the heap!
dreamlax
should be: "...you'll have to put it on the HEAP yourself." and the malloc line is broken in two places.
Dan
Guys, SO is a wiki. Don't just comment on mistakes, fix 'em!
Graeme Perrow
SO is a wiki for people with 2k+ reputation; the rest of us have to point out errors the old-fashioned way ;)
smorgan
@smorgan: Heh - good point. My bad. +1 for the comment
Graeme Perrow
+8  A: 

Once the function returns, the data is not destroyed, but will likely be overwritten by the next function call. You may get lucky and it may still be there, but it is undefined behavior. You shouldn't rely on it.

Here is what may happen: during the function the stack looks like this:

"---------------------------
| caller function's data   |
----------------------------
| Ten[9]                   |
| Ten[8]                   |
| ...                      |
| Ten[0]                   |
---------------------------"

Immediately after the function exits, it will probably look the same. But if the caller calls another function like this,

void some_func() {
    Gene g;
    ...
}

the stack will now look like this:

"---------------------------
| caller function's data   |
----------------------------
| g                        |
----------------------------
| Ten[8]                   |
| ...                      |
| Ten[0]                   |
---------------------------"

Some of the data may be silently overwritten (in this case it is Ten[9]), and your code will not know it. You should allocate the data on the heap with malloc() and explicitly free it with free().

Zifre
Just to be explicit, the memory may be overwritten before the next function call. Lots of things push onto the stack.
Kennet Belenky
That's why I said 'probably.' In my experience it is usually true.
Zifre
A: 

Your function says it returns a pointer, so when your function is called, only space for a pointer is allocated on the stack for the return value. Therefore, your return value is a pointer that points to a place on the stack that will be invalid when your function exits.

Wrap your return value in a struct.

typedef struct
{
    int Array[10];
} ArrayOfTen;

ArrayOfTen SomeFunction ()
{
    ArrayOfTen array = {{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}};
    return array;
}

int main (int argc, char *argv[])
{
    ArrayOfTen array = SomeFunction ();
    int i;

    for (i = 0; i < 0; i++)
        fprintf (stdout, "%d\n", array.Array[i]);

    return 0;
}
dreamlax
+1  A: 

This is the cause of the most hideous bugs! Sometimes it will work, other times it won't.

This is a bug, not a "clever piece of code".

Robert
A: 

Another way to do this correctly, without having to allocate heap memory, is to alter the function's prototype to take a pointer to the array to write the result into:

void GetTopTen( Gene Ten[] ) { ... }

then just delete the declaration of Ten in the function body since it's now a parameter.

The caller now needs to declare its own ten-element array and pass it as a parameter:

... Gene top[10]; GetTopTen(top); ...

Just be careful that the caller declares a big enough array! Unfortunately C doesn't have a good way for the function to specify the size of the array that should be passed in, so the compiler won't warn you if the caller declares too small an array; it'll just overwrite the stack at runtime.

Jens Alfke
Be sure to explicitly pass the size of that array, otherwise you risk another bug when someone accidentally passes a different sized array without thinking about it.
johnny