CODEWITHSUNDEEP
c++memory-managementnew-operator
Daniel
Daniel

Reputation: 22395

C++ whats going on here

Lets say i have a class TwoWayList that holds Records , and GetRec actually creates a new list on the heap, here is the method 

void GetRec(TwoWayList<Record> &rec)
{
   TwoWayList<Record>* list= new TwoWayList<Record>();
   Record r;
   list->Insert(&r);
}

Now i have the follow two scenarios, the first one dies when i call delete, and the other one i just get a null reference to record, so when i call MoveToStart() i get a segfault, however if i just delete it works...

int main () {
    TwoWayList<Record> record;
    GetRec(record);
    record.MoveToStart();
    delete &record;//crash
   return 0;
}

int main () {
    TwoWayList<Record> *record;
    GetRec(*record);
    record->MoveToStart(); //segfault
    delete record;
   return 0;
}

So whats going on here? Im creating a TwoWayList in the heap in the method, therefore shouldnt i be able to delete (in fact wont it be a leak if i dont delete it?) Whats the correct way to get the TwoWayList from the method here in order for me to be able to delete it later?

Thanks

Daniel

Upvotes: 1

Views: 225

Answers (3)

Murka
Murka

Reputation: 361

void GetRec(TwoWayList<Record> &rec)
{
   TwoWayList<Record> *list= new TwoWayList<Record>();
   Record r;
   list->Insert(&r);
}

Well, you allocate some memory on the heap and let its pointer go out of scope here. You sure you didn't mean to do:

rec = new TwoWayList<Record>(); //rec should really be a reference to a pointer

?

TwoWayList<Record> record;
delete &record;//crash

You cannot delete objects on the stack.

TwoWayList<Record> *record;
GetRec(*record);
record->MoveToStart(); //segfault
delete record;

Here your GetRec isn't changing record, which i assume you wanted. As record points to garbage, a call on that object will obviously break.

Upvotes: 1

Tim
Tim

Reputation: 9172

Your first main creates record on the stack -- not the heap. So your attempt to delete the address of a stack variable crashes.

Your second main never allocates record at all. So when you try to use the pointer, it segfaults.

Also, your function allocates memory, but then forgets about it and leaks it. By that I mean that you create a new list, but never hold onto the pointer. Once the function exits, you no longer have a pointer to the list you created -- probably not what you wanted to do.

GetRec also ignores the input parameter -- also probably not what you wanted.

Guessing at what you're attempting...

void GetRec(TwoWayList<Record> &rec)
{
    Record r;
    rec.Insert(r);
}

int main () {
    TwoWayList<Record> record;
    GetRec(record);
    record.MoveToStart();
    return 0;
}

This creates a TwoWayList (named record), passes a reference to record to the function GetRec. GetRec creates a Record and Inserts it into the TwoWayList. Back in main, it calls MoveToStart on record (which now has one Record inserted into it).

This avoids any issues with new/delete by using the stack, at the cost copying Record when you insert it into the TwoWayList. It's doubtful the performance cost of that copy will matter much to you. But if it does, just say so.

Upvotes: 5

James
James

Reputation: 5425

In the first example, you are deleting a local variable, which is bad. Not needed; the variable is 'deleted' when it goes out of scope at the end of the function.

Your second example is attempting to use an uninitialized pointer. You need to allocate an instance of TwoWayList<Record> using new.

Also, your GetRec function is allocating a list on the heap and never calling delete, which is causing a memory leak.

Upvotes: 0

Related Questions