0

I am running the following code:

int* range(int start, int end, int step){
    int size = (end - start)/step;
    int out[size];
    for(int i = 0, j = start; i < size; i++, j += step){
        out[i] = j;
    }
    return out;
}

int main()
{
    int *a = range(1, 5, 1);
    printf("%i\n", *a);
    printf("%i\n", *a);
    return 0;
}

I get 1 on the first line, as expected, and some garbage value on the second (6422260 for instance). What is the explanation? I would expect to get 1 on both lines.

  • 3
    You're returning a pointer to a local variable from the `range` function. That local variable will go out of scope and cease to exist once the function returns. If you dereference the pointer you will have *undefined behavior*. – Some programmer dude Aug 10 '17 at 12:19
  • 3
    There must be 100s of duplicates of this question – Jabberwocky Aug 10 '17 at 12:20
  • 1
    This one is *slightly* different. The fact this is **undefined** in any case remains, but the first call **happens** to work "as expected" because the value from the function is still there, only calling `printf()` finally overwrites it. –  Aug 10 '17 at 12:22
  • This is a `UB` and don't try to think how it's printing expected value the first time. You cannot predict the output and it may vary from compiler to compiler. As @FelixPalmen mentioned, in your case `printf()` which uses the stack might have overwritten the area where `out` exists – infinite loop Aug 10 '17 at 12:31
  • @Lundin this case is not the duplicate especially if the optimisations are on. The function `range` will be inlined and then reduced to the constant value `1`. Then two calls to `printf` with constant value `1` as the parameter. In this case it is not an UB as all local variables are being optimised out (including the pointer `a` in the `main`) – 0___________ Aug 10 '17 at 12:45
  • 1
    @PeterJ optimizations have nothing to do with UB. As you're often writing such comments, I recommend you try to understand the difference between the concept of the abstract machine and formal definition of C and properties of a concrete implementation. –  Aug 10 '17 at 12:46
  • @Felix Palmen No - it explains why it will always print one. It does not matter how later memory is used as the result is not taken from there. – 0___________ Aug 10 '17 at 12:49
  • 1
    @PeterJ [Undefined, unspecified and implementation-defined behavior](https://stackoverflow.com/q/2397984/2371524). Please stop reasoning about a specific outcome with a specific implementation, it doesn't make **undefined behavior** magically defined. The dupes are obviously valid. –  Aug 10 '17 at 12:52
  • @Felix Palmen it does not make this case less interesting, as it shows traps of the optimisations. Maybe you always compile with -O0 to keep your programs 100% conforming, but most people do not, and it good to know what happens behind it. – 0___________ Aug 10 '17 at 12:58
  • 1
    @PeterJ `-O0` will **hide** many problems caused by **undefined behavior**, not the other way around. So, ending this discussion here, as you actively refuse to learn. –  Aug 10 '17 at 13:02
  • @Felix Palmen please do not judge me! – 0___________ Aug 10 '17 at 13:04
  • 1
    @PeterJ To continue the hotel analogy from the 2nd linked duplicate, you are saying "But I know that the hotel will occasionally place the bedside table in the alley outside for cleaning. That is when I'll strike! By doing so I'll avoid all the nasty things that could happen inside the hotel." Nope. The hotel has no obligation to place the bedside table outside. Or if they do, there is no guarantee that the book will be there. Or by doing so, they encourage thieves to steal the book, so it will be nowhere to be found at all. And so on. Undefined behavior is behavior which is not defined. – Lundin Aug 10 '17 at 13:16

1 Answers1

0

If this:

int out[size];

doesn't appear in file scope (in other words, appears inside a function), it defines a variable with automatic storage duration. This means the variable only exists as long as execution is inside its scope (which is the nearest set of braces). Once the scope is left, the variable is gone.

Returning an array isn't possible in C, return out instead returns a pointer to the array's first element. So you see the problem: The pointer points to an object that doesn't exist any more.


Now with your code, the first call to printf() seems to work. This is "by accident" (it's not guaranteed at all), but can be explained: In your implementation, nothing changes the memory where your array was located, until another function (here printf()) is called and overwrites this memory with its own local variables.


How to do this correctly: There are several possibilities:

  1. Let the caller provide the array.
  2. return a pointer to an allocated array using malloc() as shown in anoher answer, caller has to free() it.
  3. Add a static to the array definition, which gives it static storage duration (for the whole execution time of the program), but be aware of a severe consequence of this: there's only one instance of this array forever, creating all sorts of problems, so, better don't.

IMHO, 1. is the most idiomatic C way of doing this, using your original code:

void range(int *out, int start, int end, int step){
    int size = (end - start)/step;
    for(int i = 0, j = start; i < size; i++, j += step){
        out[i] = j;
    }
}

int main()
{
    int a[5];
    range(a, 1, 5, 1);
    printf("%i\n", *a);
    printf("%i\n", *a);
    return 0;
}