Pointer returns and scope

Question

Is returning a pointer declared inside the function considered bad practice? Example:

int* foo(void)
{
    int * ret_val = 1234;

    return ret_val;
}

I believe you're suppose to use:

static int * ret_val = 1234;

But then again, wouldn't it still be considered bad practice to return memory outside of your scope?

Answer 1

Returning a pointer isn't a problem as long as it points to memory that is still in scope, such as dynamically allocated memory, global data, or local variables that still exist further up the call stack.

The problem with the above code is that you're dereferencing a pointer without assigning anything to it. This: *ret_val = 1234; assigns the value 1234 to the address that ret_val points to, but ret_val wasn't assigned anything.

If on the other hand you did this:

int* foo(void)
{
    int * ret_val;

    ret_val = malloc(sizeof(int));
    *ret_val = 1234;

    return ret_val;
}

That is fine since you're returning a pointer to dynamically allocated memory.

Answer 2

The primary culprit is

*ret_val = 1234;

is applied with an uninitialized pointer.

I believe you're suppose to use:

static int* ret_val;

No, to fix that you could use

int* foo() {
     static int_val = 1234;
     return &int_val;
}

But it's arguable if that's what you really want. All callers of that function will share the same instance of int_val.

Answer 3

It's a really bad practice if your return a pointer to a local variable, because you will have a dangling pointer.

int* foo(void)
{
    int a = 1337;
    return &a; //Don't do this!
}

But sometimes it could be useful:

int* copy_array(int *array, int size)
{
    int *v = (int*) malloc (size * sizeof (int));
    //..copy the array..
    return v;
}

Anyway, you should avoid returning pointers, so there is no risk of memory leaks.

Answer 4

Aside from your use of ret_val without it being allocated, returning a pointer from a function is fair practice, though it takes special care:

• Don't return a pointer to a local variable, as it will be destroyed when the function exits and cause undefined behavior if dereferenced
• If the function allocates memory, it is the responsibility of the caller function to ensure that the pointer is deallocated when it is no longer needed, to avoid memory leaks
• If the function is processing an array in the form of a pointer, it needs to be aware of the size of the array to avoid undefined behavior, typically as an extra size parameter or a global constant

Answer 5

You should consider:

int *foo() {
        int *ret_val = new int(1234);
        return ret_val;
}

instead of (bad code):

int* foo(void)
{
    int * ret_val;

    *ret_val = 1234;

    return ret_val; // dangling pointer!
}

otherwise you will have dangling pointer (the second example). Obviously don't forget to release the memory. You should also consider using smart pointers: https://en.wikipedia.org/wiki/Smart_pointer

As you specified both C++ and C languages the above code is in C++ (operator new), please refer to (e.g.) @Mattia F. answer for ANSI C.

Answer 6

A pointer is like a shortcut or a hyperlink. Creating just a shortcut does not install a program. Creating a hyperlink does not magically sets up the whole website.

Same with a pointer: a int* is a pointer to an integer. It is not the integer itself. As such you must make sure that the pointer points to something meaningful before you try to follow the pointer.

One way of doing so is to create an integer and the sets a pointer to point to it:

int* ret_val = new int;

This will create an integer (the new operator), and then sets the ret_val pointer to point to it.

Objects created through new exist until you explicitly destroy them through delete or when your application ends. For that reason, if you have new int in a function, it is safe to access it even if the function ends.

What would be dangereous, and probably caused your concern, is if you would set the pointer ret_val not to a new integer, but to an existing local variable, i.e.

int myInteger;
int* ret_val = &myInteger;

In such scenario, ret_val points to a local variable myInteger which gets destroyed when the scope containing the declaration ends. You end up with a dangling pointer, referencing stuff that does not exist (think: broken hyperlink). Using such a pointer can lead to undefined behavior. It may crash your program, but it may as well silently accept it, modifying some random space in memory.

---

So, a function of the shape:

int* foo(void)
{
    int* ret_val = new int;
    *ret_val = 1234;
    return ret_val;
}

is safe to use. It is not necessarily a good practice: the user of such function must know that it creates a new integer, that later - at some point - someone should delete. Functions that do that typically highlight such behavior in some way, e.g. through its name. For example, allocateInt() makes it clear that it created something that later should be delete. In contrast, getInt() suggests that the integer already exists and nothing new is created.