2

I'm trying to write a program that reads values from cin using a pointer, and then outputs the values alongside their position in the array. I can't figure out why printNumbers1 works but printNumbers2 doesn't. Here is the program (relevant code near the bottom):

#include <iostream>

using namespace std;

int *readNumbers(int);
void printNumbers1(int*);
void printNumbers2(int*);

int main()
{
    int *numbers = readNumbers(5);
    printNumbers1(numbers);
    printNumbers2(numbers);
    return 0;
}

int *readNumbers(int n)
{
    int a[n];
    int *numbers;
    numbers = &a[0];
    for (int i=0; i<n; i++)
    {
        cin >> *(numbers+i);
    }

    return numbers;
}

void printNumbers1(int *numbers)
{
    cout << 0 << ' ' << *(numbers) << endl
         << 1 << ' ' << *(numbers+1) << endl
         << 2 << ' ' << *(numbers+2) << endl
         << 3 << ' ' << *(numbers+3) << endl
         << 4 << ' ' << *(numbers+4) << endl;
}

void printNumbers2(int *numbers)
{
    for (int i=0; i<5; i++)
    {
        cout << i << ' ' << *(numbers+i) << endl;
    }
}

When I run the program, it works as intended for printNumbers1 but outputs a combination of seemingly random numbers and 0s for printNumbers2. I feel like the two printNumbers functions should should function identically, but they don't. What am I missing?

  • `a` is a local variable. It will go out of scope and become invalid at the end of the function. Returning a pointer to it is a bad idea because the pointer will be pointing to bad memory. Your are likely going to have to use `new` and `delete`. – user4581301 Apr 02 '17 at 08:11

3 Answers3

3

This happens because of a combination of two things:

  • C++ does not allow variable-length arrays - this is a popular extension, but the declaration int a[n] is not standard.
  • You cannot return a pointer to local variable from a function - pointer numbers inside readNumbers points to a, a local variable. You can use this pointer inside the function, but outside of the function it becomes invalid, because a goes out of scope.

Using an out-of-scope variable causes undefined behavior. This Q&A provides a very good explanation of what is happening, and why it may look like the program is working fine.

If you want to use built-in pointers, remove int a[n], and change the declaration of numbers as follows:

int *numbers = new int[n];

You also need to add

delete[] numbers;

before return 0 line to avoid memory leaks.

I am assuming that you wrote this code as part of a learning exercise. In general, though, a better approach in C++ is to use std::vector<int>, which hides pointer operations from your code, and deals with resource management for you.

Community
  • 1
  • 1
Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
  • Thanks! The only thing I'm still confused about is why `printNumbers1` works if `a` is out of scope for functions other than `readNumbers`. – Memelord Supreme Apr 02 '17 at 08:21
  • @MemelordSupreme If an array is out of scope, it does not mean it has random garbage: it could have the stuff that you put there before, so, unfortunately, incorrect code may give an appearance of working. However, it's not really working, it's just a situation when random garbage happens to match what you expected to see. – Sergey Kalinichenko Apr 02 '17 at 08:29
  • @dasblinkenlight Please add a note about undefined behavior; that's what's causing seemingly-correct behavior. – Nic Apr 02 '17 at 08:41
  • @QPaysTaxes Done. – Sergey Kalinichenko Apr 02 '17 at 08:52
1

readNumbers returns a pointer to a variable with automatic storage duration.

The behaviour on dereferencing that pointer is undefined.

Use a std::vector instead. Rely on return value optimisation to obviate any superfluous value copies being taken.

Bathsheba
  • 231,907
  • 34
  • 361
  • 483
0

Here you have created the array a[n] inside the function, so it is a local variable, Hence this array may or may not be deleted after the scope of the function ends.Never use the address of a local variable outside the funcrtion. This code is working:

#include <iostream>

using namespace std;

int *readNumbers(int);
void printNumbers1(int*);
void printNumbers2(int*);

int main()
{
    int *numbers = readNumbers(5);
    printNumbers1(numbers);
    printNumbers2(numbers);
    return 0;
}

int *numbers;

int *readNumbers(int n)
{

    numbers = new int[n];
    for (int i=0; i<n; i++)
    {
         cin >> *(numbers+i);
    }

    return numbers;
}

void printNumbers1(int *numbers)
{ 
cout << 0 << ' ' << *(numbers) << endl
     << 1 << ' ' << *(numbers+1) << endl
     << 2 << ' ' << *(numbers+2) << endl
     << 3 << ' ' << *(numbers+3) << endl
     << 4 << ' ' << *(numbers+4) << endl;
}

void printNumbers2(int *numbers)
{
for (int i=0; i<5; i++)
{
    cout << i << ' ' << *(numbers+i) << endl;
}
}
Arvindsinc2
  • 716
  • 7
  • 18