How do I make sure it's safe to return a pointer from a function

Question

Here's my problem: I read here on StackOverflow that it is unsafe sometimes to return pointers to local variables from a function. For example:

#include<iostream>

using namespace std;

int *foo(void) {
    int x[] = {1,2,3};
    return x;
}

int main() {
    int *numbers;
    numbers = foo();
    return 0;
}

I'd like to know if this is unsafe, considering that x being a local array, the memory could be unallocated, what's the better way to achieve the same result?

Answer 1

You can either:

1. Declare x as static
2. Declare x as a pointer

Answer 2

Yes it is unsafe because the array is allocated on the stack, as such it will be deallocated when the function returns.

Instead of allocating the array inside the function, create it outside and pass a pointer to it to the function. This is just an example:

#include<iostream>
using namespace std;

void foo(int numbers[]){
    numbers[0] = 1;
    numbers[1] = 2;
    numbers[2] = 3;
}

int main(int args, char**argv) {
    int numbers[3];
    foo(numbers);
    cout << numbers[0] << numbers[1] << numbers[2];
    return 0;
}

This will print "123".

Answer 3

First of all int x = {1,2,3} is syntax error. It should be int x[] = {1,2,3};

This is undefined behaviour. Because the automatic array has a lifetime inside its block where it is defined, that is inside the foo() function. So whenever you return from foo() the storage for x is no more guaranteed to be reserved for you. Therefore if you access that location through pointers then the behaviour is undefined.

To achieve the same result dynamically allocate memory.

int *foo(void){
int x[] = {1,2,3}, *x_to_return;
x_to_return = new int [sizeof (x)/sizeof(x[0])];
memcpy (x_to_return, x, sizeof (x));
return x_to_return;
}

Basically, what you need to do is to dynamically allocate the storage using new, copy your data to the block of memory (base of which is) allocated by new and return that memory address to the caller.

Don't forget to free the allocate the memory once you have finished using it, else your code would have memory leakage.

Also a point to be noted, if you have a declaration like static int x[] = {1,2,3}; then you can return the address of x, because in this case the lifetime of x is the entire program runtime.

As your question is tagged c++ you should use vector, check moooeeeep's answer.

Answer 4

I read here on StackOverflow that it is unsafe sometimes to return pointers to local variables from a function.

It is always unsafe to return pointers to local variables. Indeed it is wrong to do so, and using this pointer will cause undefined behavior. See also this awesome post.

If you want to return data to the calling scope, you could use a std::vector as a copy:

std::vector<int> foo(void){
  std::vector<int> x = {1,2,3}; // using C++11 initializer list
  return x;
}

If it's a fixed length array (always of size 3), you could use std::array instead.

---

Depending on your requirements you may also use a static variable. That is, the variable will never go out of scope, s.t. you can return it safely by reference (or by pointer). Note that you have only one copy. If you modify it, it will remain modified. (Make it const & if it's read only.)

std::vector<int>& foo(void) {
  // this is only instantiated once when the function is first called
  static std::vector<int> x = {1,2,3}; 
  return x;
}

Answer 5

It is never safe to to return a pointer to a local variable (it's undefined behaviour in fact). For most implementations those live on the stack, so it can be overwritten when the pointer is used.

You can return an dynamically allocated array:

int* foo() { int* x = new int[3]; ..}

Of course you would need to manually delete the pointer, which makes it hard to write robust, excpetion safe code. Therefore it's typically preferable to use a vector:

std::vector<int> foo() { 
   std::vector<int> x;
   x.push_back(1); 
   x.push_back(2); 
   x.push_back(3);
   return x;
}

If you use c++11, you can use an initialization list to fill the vector, making the code much nicer:

std::vector<int> foo() { std::vector<int> x = {1,2,3};  return x; }

C++11 has move semantics, which means that in this case returning the vector by value costs almost no performance. For C++03 if performance is essential you could give the function a reference/pointer to vector as a parameter and fill that:

void foo(std::vector<int>& x) {x.clear(); x.push_back(1); ...}

Answer 6

It is unsafe (or rather wrong) to return a pointer to that array, because it does not exist any more when the function returns. The memory could not just be deallocated, but it will be.
Note that it might still accidentially work, but honestly that would be worse than if it didn't work (because it's unpredictable and impossible to debug). Never attempt "but it seems to work" things, even if they seem to work.

This is the same for returning a pointer or a reference, with a const reference being an exception. A const reference keeps the referenced object alive for its own lifetime.

Dynamically allocating or making the object static would be options if you want to return a pointer. Or, just return a temp object by value, relying on the compiler to RVO it out.