3

have this code:

void set(list<Person*>* listP){
    Person timmy = Person(10);
    listP->push_back(&timmy);
}
int main()
{
    list<Person*> listP;
    set(&listP);
    Person* timmy  = listP.back();
}

If i understand correct (please correct me) timmy is allocated on the stack , so i cannot count on the values of timmy when i use them in the main. Am i correct? do i need to create timmy like this:

Person* timmy = new Person(10);

in order to create it on the heap and not on the stack ,so it will not be destroyed after method return?

Thank you

trincot
  • 317,000
  • 35
  • 244
  • 286
Avihai Marchiano
  • 3,837
  • 3
  • 38
  • 55

2 Answers2

2
void set(list<Person*>* listP){
    Person timmy = Person(10); // create timmy on automatic storage (stack)
    listP->push_back(&timmy); //push timmy's address
} //timmy is destroyed. pushed address points to deallocated memory

Yes, you need to use Person* timmy = new Person(10); to allocate on heap.

void set(list<Person*>* listP){
    Person *timmy = new Person(10); // timmy is a pointer now
    listP->push_back(timmy); //push timmy's copy (copy of pointer)
} //timmy (pointer) is destroyed, but not the memory it points to

Also prefere to use smart_pointers such as std::shared_ptr, std::unique_ptr or boost smart pointers. It will simplify memory management and writing exception-safe code

Andrew
  • 24,218
  • 13
  • 61
  • 90
  • 1
    @user1495181: Your understanding is correct but the conclusion isn't necessarily, you could use `list` instead of `list`. That would avoid these problems. – jahhaj Jul 31 '12 at 11:06
  • Thank you for correction. yes it will avoid but will copy new instance. – Avihai Marchiano Jul 31 '12 at 11:06
  • 2
    @user1495181: Sure, and the problem with that is ...? – jahhaj Jul 31 '12 at 11:07
  • @user1495181: don't make optimization until you found a bottleneck. Write clean well readable code and care about optimizations only when you must – Andrew Jul 31 '12 at 11:08
  • Performance , at least in my case. i load 100K records from DB for each of them i create RecordIntance fill values and add it to list (with list for each record you will create 2 instances. one for read and additional when you add to the list). After i create this list i return the list to the caller if it list , so now again the complete list will be copied. i am new to cpp , so please correct me. – Avihai Marchiano Jul 31 '12 at 11:11
  • Andrew, i agree with you (premature optimization is root of evil), but in this case i know the the cost. look on my comment above. – Avihai Marchiano Jul 31 '12 at 11:13
  • 3
    Return value optimization will most likely kick in here. Not sure what the exact conditions for rvo are, but I never failed to meet them so far. Making copies of 100k elements twice is likely faster than performing 100k heap allocations, unless your elements are very heavy weight. – Cubic Jul 31 '12 at 11:13
  • 1
    @user1495181: take a look at this question: http://stackoverflow.com/questions/4809120/creating-and-returning-a-big-object-from-a-function – Andrew Jul 31 '12 at 11:15
2

Your assumption is correct, but you don't have to create timmy on the "heap", if you use a list of Persons instead:

void set(list<Person>& listP){
    listP.push_back(Person(10));
}

It is extremely likely that no extra person copies will be made in the push_back (copy elision). In C++11, even if the copy wasn't elided, move semantics could kick in and expensive copying.

juanchopanza
  • 223,364
  • 34
  • 402
  • 480
  • are you talking about RVO? isnt it used only for big object? see the discussion bellow. thank you! – Avihai Marchiano Jul 31 '12 at 11:24
  • 1
    @user1495181 RVO is a subset of copy elision, and it applies to returning by value. So it doesn't apply to this example. I was speculating that the push_back would only make one construction, but it actually makes that plus a copy construction. But with C++11, the temporary `Person` is move-copied. – juanchopanza Jul 31 '12 at 11:41
  • @user1495181 I forgot to say, RVO has nothing to do with the size of the object being returned. – juanchopanza Jul 31 '12 at 11:50
  • I test to see if listP.push_back(Person(10)); dosnt create two objects (in c++11) and it does. – Avihai Marchiano Jul 31 '12 at 12:14