returning local variables

Question

I came across an issue today regarding local variables. I learned that...

 int * somefunc()
 {
     int x = 5;
     return &x;
 }

 int * y = somefunc();
 //do something

is bad, unsafe, etc. I'd imagine that the case is the same for...

int * somefunc()
{
    int * x = new int;
    x = 5;
    return x;
}

int * y = somefunc();
//do something
delete y;

I've been under the impression for the longest time that this would be safe as the address of x stays in scope when it's returned. However, I'm having second thoughts now and I'm thinking this would lead to memory leaks and other problems, just as the fist example would. Can someone confirm this for me?

Answer 1

You might be making int * as just an example, but really, in the case you noted, there is not a reason to return int *, just return int, the actual value is more than good enough. I see these situations all the time, getting overly complicated, when, what is actually needed, is just to simplify.

In the case of 'int *', I can only really think of a realistic case of returning an array of ints, if so, then you need to allocate that, return that, and hopefully, in your documentation, note that it has to be released.

Answer 2

As it stands, the second example is wrong. You probably meant this:

int * somefunc()
{
    int * x = new int;
    *x = 5; // note the dereferencing of x here
    return x;
}

Now this is technically fine, but it is prone to errors. First, if after the allocation of x an exception happens, you have to catch it, delete x and then rethrow, or you get a memory-leak. Second, if you return a pointer, the caller has to delete it - callers forget.

The recommended way would be to return a smart pointer, like boost::shared_ptr. This would solve the problems mentioned above. To understand why, read about RAII.

Answer 3

Ok, I would analyze this by answering these questions:

1. What does x contain ? - A memory location(since it is a pointer variable)
2. What is the scope of x? - Since it a a auto variable it's scope is limited to the function somefunc()
3. What happens to auto variables once they exit the local scope ? - They are deleted from the stack space.
4. So what happens to x now after return from somefunc()? - Since it is an auto variable declared on the stack , it's scope(lifetime) is limited to somefunc() and hence will be deleted.
5. Ok so now, what happens to the value pointed to by x? We have a memory leak as the value is allocated on the heap and we have just lost the address when x is deleted.
6. What does y get? - No idea.
7. What happens when y is deleted? - No idea.

Answer 4

It's both memory leak and a crash (because of the delete).

Answer 5

The first approach certainly leads to problems, as you are now well aware.

The second is kind of OK, but demands attention from the programmer because he needs to explicitly delete the returned pointer (as you did). This is harder when your application grows larger, using this method will probably cause problems (memory leaks) as the programmer will find it difficult to keep track of every single variable he needs to deallocate.

A 3rd approach for this scenario, is to pass a variable by reference to be used inside the function, which is way safer.

void somefunc(int& value)
{
    value = 5;
}

// some code that calls somefunc()
int a_value = 0;
somefunc(a_value);
// printing a_value will display 5 

Answer 6

The point is not to return a pointer or reference to a local variable, because once the function returns, locals don't exist.

However, the return value still exists, and dynamically allocated memory certainly exists as well.

In C++, we prefer to avoid raw pointers whenever possible. To "return a value that already exists" (i.e. the function does not create a new value), use a reference. To "return a value that didn't already exist" (i.e. the function creates a new value, in the idiomatic sense, not the new keyword sense) use a value, or if necessary, some kind of smart pointer wrapper.