-6
#include  <iostream>

using namespace std;

int *GetSquare(int x)
{
    int y = x;

    y = y * y;
    return &y;
}

int main()
{
    const int n = 4;
    int *p[n];

    for (int j = 0; j < 2; ++j)
    {
        p[2 * j + 1] = new int[2];

        for (int i = 0; i < 2; ++i)
            p[2 * j + 1][i] = 2 * j + 1;
    }
    p[0] = GetSquare(2);
    p[2] = GetSquare(4);

    for (int j = 0; j < n; ++j)
    {
        for (int i = 0; i < 1; ++i)
            cout << p[j][i] << " ";
        cout << endl;
    }
    return 0;
}

When I dry run this code I get the output "4 1 16 3". while in the compiler i get "16 1 3 3" theres no way the in which i can find the first output to be 16. What is wrong with my code?

lurker
  • 56,987
  • 9
  • 69
  • 103
  • 7
    One thing I notice: http://stackoverflow.com/questions/6441218/can-a-local-variables-memory-be-accessed-outside-its-scope – chris Mar 13 '15 at 19:08
  • 5
    Please change your title to something more relevant to the problem you're having. "Not getting the right output" pretty much applies to any software bug ever. – Borgleader Mar 13 '15 at 19:08
  • 2
    One issue is that your `GetSquare` is returning a pointer to a variable which is local to `GetSquare`. That's bad. Why don't you just return the value? And what do you mean, *When I dry run this code...*? What's it actually supposed to do? – lurker Mar 13 '15 at 19:10
  • He probably means debug, but I'm not editting it in in case he means something else. @lurker –  Mar 13 '15 at 19:12

2 Answers2

2

This:

int *GetSquare(int x)
{
    int y = x;
    y = y * y;
    return &y;
}

is a very bad thing.

You are returning the address of a value on the stack, which is invalidated once leaving the function. You cannot rely on what lives at that address once you've left the function.

You write:

p[0] = GetSquare(2);

and you might think p[0] is a pointer to an integer with the value 4. Except that pointer was only valid within GetSquare.

The problem seems engineered to be bad. Why not return an actual integer value from GetSquare, rather than a pointer to an invalid memory address?

int GetSquare(int x) { return x * x; }

EDIT: The question could be simplified to this:

int* GetSquare(int x)
{
    int y = x;
    y = y * y;
    return &y;
}

int main()
{
    int* p = GetSquare(2);
    int* q = GetSquare(4);
    cout << "p == q? " << (p == q ? "YES" : "NO") << endl;
    cout << p << " " << q << " " << endl;
    return 0;
}

Run that in debug mode, run in release, with various levels of debug info and/or optimization... You might sometimes get 4 16, but probably not. It may also sometimes say p and q are the same, sometimes not.

The link in chris's comment has a decent explanation.

Matthew Moss
  • 1,258
  • 7
  • 16
  • Some of the code in `main` will subsequently need to be refactored as well since the OP was counting on `GetSquare` returning a pointer to the value. – lurker Mar 13 '15 at 19:14
  • @lurker Very true... left as an exercise for the reader. ;) But seriously, the original question is rather... bizarre... almost like it was written to specifically do bad things at bad times to bad people. At a glance, I'm not convinced that `GetSquare` is the *only* bad pointery thing in there. `GetSquare` is an obvious problem, but perhaps not the only. – Matthew Moss Mar 13 '15 at 19:18
  • All true. :) I was just thinking if one is indicating a solution, pointing out the obvious knock-on effects is helpful but I agree: exercise for the reader. Lots of goblins in the question and code. – lurker Mar 13 '15 at 19:22
  • The rest of the code is *safe* but *ugly*. – Matthew Moss Mar 13 '15 at 19:24
0

Your ptogram has undefined behaviour because you are returning pointer to a local object of the function that will be destroyed after exiting it.

int *GetSquare(int x)
{
    int y = x;

    y = y * y;
    return &y;
}

Also it does not make sense to return pointer instead of a temporary object. The function could be written like

long long int GetSquare( int x )
{
    return ( long long int )x * x;
}
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335