0

The following function "increment" add 1 to a number represented as an array.

int*  increment(int array[], int size, int *sizeLen)
{
    int temp[size+1];

    int carry = 0;
    carry = (array[size-1]+1)/10;
    temp[size] = (array[size-1]+1)%10;

    for(int i=size-2;i>=0;i--)
    {
        temp[i+1] = (array[i] + carry)%10;
        carry = (array[i]+carry)/10;
    }
    if(carry)
    {
        temp[0] = 1;
        *sizeLen = size+1;
        return temp;
    }
    else
    {
        *sizeLen = size;
        return (temp+1);
    }
}

int main()
{
    int array[] = {9,9,9};
    int length;
    int *res = increment(array, sizeof(array)/sizeof(int), &length);
    for(int i=0;i<length;i++)
    {
        cout << res[i] << " ";
    }
}

I know gcc supports variable length arrays and they are stored on stack. I expect temp to go out of scope once this function ends and the attempt to print the array in main should print garbage values. But in my case the actual values are printed. When does the variable length array declared in the function goes out of scope?

eagle
  • 35
  • 1
  • 8
  • 6
    What you are seeing is undefined behavior. That's a pitfall of C. –  Jan 24 '17 at 08:34
  • 1
    C++ doesn't even support VLAs and your code is basically C. Use containers and iterators. – cadaniluk Jan 24 '17 at 08:40
  • I didn't notice the tag. Why are you using VLA's in C++ code (an extension, non-standard)? Is your question really about C? Clarify, please. Those are different languages. – StoryTeller - Unslander Monica Jan 24 '17 at 08:44
  • Thanks all of you for your help. Yes i understand i wrote a mix of c and c++ code. I was curious about the strange behavior of the program. – eagle Jan 24 '17 at 09:12
  • @eagle -- *I was curious about the strange behavior of the program* -- Trying to figure out why your program behaves strangely may become a big waste of time. Why? If you compiled that very same program, but used different compiler options, such as optimizations, the program may yet again behave differently than what you're seeing. It gets to the point where it becomes pointless to attempt to figure out undefined behavior. – PaulMcKenzie Jan 24 '17 at 09:57
  • @PaulMcKenzie -- True. I replaced cout with printf and it began to show garbage values. When the program was giving me the correct result, I actually began to wonder about the scope of the variable length arrays. – eagle Jan 24 '17 at 10:06

4 Answers4

2

The key here is indeed the storage duration of the array. It has automatic storage duration. The lifetime of variables with automatic storage duration ends the moment the scope they're are in is exited.

It does exactly as you expect. But nothing in C stops you from taking the address of a local object and returning it from the function.

Using that pointer is undefined behavior. It may appear to work, but the array is still for all intents and purposes "dead". Such pointers are known colloquially as "dangling pointers".


Well, the above is true for C. Since this is about the GCC extension, the same applies mostly, but may need to be taken with a grain of salt.

StoryTeller - Unslander Monica
  • 165,132
  • 21
  • 377
  • 458
1

This won't work. You are returning a pointer to a local array, which will be deleted (more likely over-written/re-used) as soon as the function returns.

The C++ way to do things like that is to use std::vector<>. For example (assuming that in your original size and sizelen referred to the number of elements in array and the returned, respectively)

std::vector<int> increment(std::vector<int> const&array)
{
    std::vector<int> temp(array.size());
    auto a = array.rbegin();
    auto b = temp.rbegin();
    auto carry = (*a+1)/10;
    *b = (*a+1)%10;
    for(++a,++b; a!=array.rend(); ++a,++b)
    {
        *b = (*a+carry)%10;
        carry = (*a+carry)/10;
    }
    if(carry) { // we must add another element into temp at the start
                // that cannot be done with vector, hence we must create
                // another vector. Note that this should rarely happen.
       std::vector<int> result(temp.size()+1);
       auto r = result.begin();
       *r = 1;
       std::copy(temp.begin(),temp.end(),++r);
       return result;
    } else
       return temp;
}

Note that your code would be much simpler if you had defined the order of digits the other way around, i.e. the least significant digit as the first element (index 0). Moreover, as your digits seem to be decimal, you could use a smaller type than int. In this case, the code would look like

std::vector<std::uint8_t> increment(std::vector<std::uint8_t> const&input)
{
    std::vector<std::uint8_t> result;
    result.reserve(input.size()+1);
    auto a = input.begin();
    auto carry = (*a+1)/10;
    result.push_back((*a+1)%10);
    for(++a; a!=input.end(); ++a)
    {
        result.push_back((*a+carry)%10);
        carry = (*a+carry)/10;
    }
    if(carry)
        result.push_back(1);
    return result;
}
Walter
  • 44,150
  • 20
  • 113
  • 196
  • What's wrong with `if (carry) temp.insert(1, temp.begin());` ? It involves shuffling up all the elements, but that's no worse than the copy you implement. – Martin Bonner supports Monica Jan 24 '17 at 10:05
  • Also if you initialize `carry` to 1, and then make the loop a `do-while` the whole thing becomes `std::vector result(array);` (or better, take array by value - saves a copy if the input is a temporary). `int carry = 1; auto it = result.rbegin(); do { auto v = *it + carry; *it++ = v %10; carry = v / 10; } while (carry && it != result.rend()) if (carry) { result.insert(carry, result.begin(); } return result;` – Martin Bonner supports Monica Jan 24 '17 at 10:14
  • Ah no! Doesn't work with an empty input. `int carry = 1; for (auto it = result.rbegin(); carry && it != result.rend(); ++it) { auto v = *it + carry; *it = v %10; carry = v / 10; } if (carry) { result.insert(carry, result.begin(); } return result;` – Martin Bonner supports Monica Jan 24 '17 at 10:17
0

This is undefined behavior. The next time a program pushes to the stack, the data will be lost.

Don't rely on this for professional programs.

0

You are right. temp does go out of scope, and it should print garbage values.

But sometimes it prints correct values. Because the local stack memory is not cleared/overrided immediately when current function ends.

zhm
  • 3,513
  • 3
  • 34
  • 55