Pointer of class local variable?

Question

Possible Duplicate:
Can a local variable's memory be accessed outside its scope?

Is there worrying thing to do a code such (getIDs() returns a pointer):

class Worker{
private:
    int workerID;
    int departID;
    int supervisorID;
public:
    Worker() 
    {
        workerID=0;
        departID=0;
        supervisorID=0;
        name="anonymous";
        workerAddress="none";
    }

    void setIDs(int worker, int depart, int supervisor)
    {
        workerID=worker;
        departID=depart;
        supervisorID=supervisor;
    }

    int* getIDs()
    {
        int id[3];
        id[0]=workerID;
        id[1]=departID;
        id[2]=supervisorID;
        return id;
    }
};

And then, use it such:

Worker obj;
obj.setIDs(11,22,33);
cout<<(*obj.getIDs())<<endl;
cout<<++(*obj.getIDs())<<endl;
cout<<++(++(*obj.getIDs()))<<endl;

I am wondering about that because the compiler shows:

Warning 1 warning C4172: returning address of local variable or temporary

This isn't really a duplicate - in that question the poster understands that what they are doing shouldn't work and is confused as to why it does work. In this question the poster wants to know if this will work or not. — Justin, Dec 01 '11 at 15:25

score 3 · Answer 1 · answered Dec 01 '11 at 13:01

3

Your int id[3] is allocated on a stack and gets destroyed when your int* getIDs() returns.

answered Dec 01 '11 at 13:01

arrowd

33,231
8
79
110

score 2 · Answer 2 · answered Dec 01 '11 at 13:06

2

Even if this would work, it wouldn't be good practice to do it this way in c++ unless there is some profound reason why you want pointers to int. Raw c-syle arrays are more difficult to handle than, for instance, std::vectors, so use those, like

std::vector<int> getIDs(){
  std::vector<int> id(3);
  id[0]=workerID; id[1]=departID; id[2]=supervisorID;
  return id;
}

If you're worried about the overhead: this is likely to be optimized away completely by modern compilers.

answered Dec 01 '11 at 13:06

leftaroundabout

117,950
5
174
319

@: Use of static int id[3]; can solve it? – Aan Dec 01 '11 at 13:11
@Adban since nobody answered you yet, I will. Yes, static will help. But the question is if static is really what you want. *Normally* static is used to save some internal data of a fuction. Here, you just want to compute and return something. – Roman Byshko Dec 01 '11 at 13:31
likely? How can we rely on likely? I wouldn't *copy* the vector like this... – Roman Byshko Dec 01 '11 at 13:32
The copy will "almost certainly" be optimized away by any recently modern standard compliant compiler due to copy elision. However, the correct approach is (in my opinion) to simply provide accessors/mutators for all of your class member variables that require them. – Chad Dec 01 '11 at 15:30
@Adban: as Roman B. said it would work with static, but that's almost certainly not what you want. You would then return a pointer to an array shared by all instances of `Worker`, so if you call, from the outside, e.g. for `Worker w1, w2;` first `int* w1IDs=w1.getIDs()` and then `int* w2IDs = w2.getIDs()`, then both pointers refer to the same array in which now only the IDs of `w2` are stored while those of `w1` were overwritten again. – leftaroundabout Dec 01 '11 at 17:07

score 2 · Accepted Answer · answered Dec 01 '11 at 15:02

You're return a pointer to a variable that gets destroyed immediately after getIDs() returns. The pointer then becomes dangling and is practically useless as doing anyting with it is undefined behaviour.

Suppose you defined your class like this:

class Worker{
private:
    int IDs[3];
public
    // ...
    int* getIDs() { return IDs; }
};

This partially solves your problem, as the pointer remains valid as long the Worker object is in scope, but it's still bad practice. Example:

int* ptr;
while (true) {
    Worker obj;
    obj.setIDs(11,22,33);
    ptr = obj.getIDs();
    cout << *ptr;   // ok, obj is still alive.
    break;
}  // obj gets destroyed here
cout << *ptr;   // NOT ok, dereferencing a dangling pointer

A better way of solving this is to implement your custom operator << for your class. Something like this:

class Worker {
private:
    int workerID;
    int departID;
    int supervisorID;
public:
    // ...
    friend ostream& operator<<(ostream& out, Worker w);
};


ostream& operator<<(ostream& out, const Worker& w)
{
    out << w.workerID << "\n" << w.departID << "\n" << w.supervisorID;
    return out;
}

Roman Byshko · Answer 4 · 2011-12-01T13:09:02.687

1

A local (also caled automatic) variable is destroyed once you leave the function where it is defined. So your pointer will point to this destroyed location, and of course referencing such a location outside the function is incorect and will cause undefined behaviour.

edited Dec 01 '11 at 13:09

answered Dec 01 '11 at 13:01

Roman Byshko

8,591
7
35
57

Ross · Answer 5 · 2011-12-01T14:00:36.800

Edit: My apologies, I completely misread the question. Shouldn't be answering StackOverflow before my coffee.

When you want to return an array, or a pointer rather, there are two routes.

One route: new

int* n = new int[3];
n[0] = 0;
// etc..
return n;

Since n is now a heap object, it is up to YOU to delete it later, if you don't delete it, eventually it will cause memory leaks.

Now, route two is a somewhat easier method I find, but it's kind of riskier. It is where you pass an array in and copy the values in.

void copyIDs(int arr[3] /* or int* arr */)
{
    arr[0] = workerID;
    /* etc */
}

Now your array is populated, and there was no heap allocation, so no problem.

Edit: Returning a local variable as an address is bad. Why?

Given the function:

int* foo() { 
    int x = 5;
    return &x; // Returns the address (in memory) of x
} // At this point, however, x is popped off the stack, so its address is undefined
  // (Garbage)  

// So here's our code calling it
int *x = foo(); // points to the garbage memory, might still contain the values we need
// But what if I go ahead and do this?
int bar[100];        // Pushed onto the stack
bool flag = true;    // Pushed onto the stack
std::cout << *x << '\n'; // Is this guaranteed to be the value we expect?

Overall, it is too risky. Don't do it.

score 1 · Answer 6 · answered Dec 01 '11 at 13:31

The basic problem here is that when you enter a function call, you get a new frame on your stack (where all your local variables will be kept). Anything that is not dynamically allocated (using new/malloc) in your function will exist in that stack frame, and it gets destroyed when your function returns.

Your function returns a pointer to the start of your 3-element-array which you declared in that stack frame that will go away. So, this is undefined behavior.

While you may get "lucky/unlucky" and still have your data around where the pointer points when you use it, you may also have the opposite happen with this code. Since the space is given up when the stack frame is destroyed, it can be reused - so another part of your code could likely use the memory location where your three elements in that array is stored, which would mean they would have completely different values by the time you dereferenced that pointer.

If you're lucky, your program would just seg-fault/crash so you knew you made a mistake.

Redesign your function to return a structure of 3 ints, a vector, or at the very least (and I don't recommend this), dynamically allocate the array contents with new so it persists after the function call (but you better delete it later or the gremlins will come and get you...).

Pointer of class local variable?

6 Answers6