How do I delete an object that was dereferenced?

Question

I have the following code

MyObject * func1() {
    MyObject * obj = new MyObject();
    // lots of stuff here
    return obj;
}

MyObject func2() {
    MyObject * obj = func1();
    // even more stuff here
    return *obj;
}

void main() {
    MyObject obj = func2()
}

As I got it from here this code is leaking. Will this:

MyObject * func1() {
    MyObject * obj = new MyObject();
    // lots of stuff here
    return obj;
}

MyObject func2() {
    MyObject * obj = func1();
    // even more stuff here
    MyObject obj_r(*obj);
    delete obj;
    return obj_r;
}

void main() {
    MyObject obj = func2()
}

resolve the issue? Or is there some other nice solutions?

in b4: no, I can't make it reference from the beginning, as func1() returns NULL in some cases.

upd: added some comments so that people didn't think I'm royally stupid

"func1() returns NULL in some cases", and yet you dereference it without testing? — R. Martinho Fernandes, Aug 19 '11 at 11:15
Well of course I do a check. I do lots of stuff there actually, I just didn't want to put it all here as it's unrelated. — Morse, Aug 19 '11 at 11:18
Exactly _how_ is func1 returning NULL? Operator `new` throws, it does not return NULL. — Damon, Aug 19 '11 at 11:18
@More Then please provide some more information. Why are you dynamically allocating something that you immediately copy to stack and discard? — Šimon Tóth, Aug 19 '11 at 11:20

score 4 · Accepted Answer · answered Aug 19 '11 at 11:25

4

A more elegant solution (and more "correct") would be to use a smart pointer:

MyObject func2()
{
    return *std::auto_ptr<MyObject>(func1());
}

(With a more modern compiler, use std::unique_ptr. Or if you're using Boost, you can also use boost::scoped_ptr.)

I say more "correct", because if the copy constructor of MyObject throws an exception, this solution will still delete the object, where as yours would leak.

answered Aug 19 '11 at 11:25

James Kanze

150,581
18
184
329

Thanx. By the way, which compiler is "modern" enough regarding GCC? – Morse Aug 19 '11 at 13:07
@Morse I don't really know. Since I have to target a number of different compilers, I don't really keep abreast as to where the compilers are with the newer features. I generally can't use them anyway. I'd use `auto_ptr` for the moment, unless I were dealing with non portable code and the one compiler I was using supported `unique_ptr`. If you want to see if you've got support, just include ``, and try to declare one. – James Kanze Aug 19 '11 at 15:59

score 1 · Answer 2 · answered Aug 19 '11 at 11:15

1

Yes, that will resolve the memory leak.

This is not a very nice pattern, in general. But then I'm not sure what you're trying to achieve here!

answered Aug 19 '11 at 11:15

Oliver Charlesworth

267,707
33
569
680

(not my downvote) It will solve it as long as the copy constructor doesn't throw. – R. Martinho Fernandes Aug 19 '11 at 11:23

score 1 · Answer 3 · answered Aug 19 '11 at 11:29

The best way to avoid a memory leak is to use a smart pointer:

#include <memory>

MyObject func2() {
    std::unique_ptr<MyObject> obj(func1());
    // stuff here
    return *obj;
}

int main() {
    MyObject obj = func2();
}

This is almost the same as your solution, but fixes the memory leak that yours has if an exception is thrown while copying the object or doing the "stuff". If you're not using C++11, then use auto_ptr rather than unique_ptr.

score 0 · Answer 4 · answered Aug 19 '11 at 11:16

0

Just change func2() into this:

MyObject func2() { return MyObject(); }

The dynamic allocation is completely useless in your case.

answered Aug 19 '11 at 11:16

Šimon Tóth

35,456
20
106
151

score 0 · Answer 5 · answered Aug 19 '11 at 11:16

0

Yes, your second solution will solve the issue, at the expense of more copies being made.

My question is: why do you want to allocate an object on the heap (with new) just to copy it to a stack allocated object, and delete it right after. Why don't you create it simply on the stack at the beginning?

You could write it as follows:

void main() {
    MyObject obj;
}

You'd get exactly the same result at the end. And it'd be much simpler!

answered Aug 19 '11 at 11:16

Didier Trosset

36,376
13
83
122

It will solve it as long as the copy constructor doesn't throw. – R. Martinho Fernandes Aug 19 '11 at 11:23

serenheit · Answer 6 · 2011-08-19T12:03:17.647

0

IMHO, the best way for this would be to delete an object in func1, if it has to remove NULL. In func2, in case we've null pointer received, we can't dereference it in any way.

edited Aug 19 '11 at 12:03

answered Aug 19 '11 at 11:54

serenheit

123
1
7

score 0 · Answer 7 · answered Aug 19 '11 at 12:10

If you're not sure whether func1 will return a valid object or not, instead of returning a pointer, boost::optional may be appropriate -- this will let func2 check whether func1 has returned something valid, without making you have to allocate anything dynamically.

How do I delete an object that was dereferenced?

7 Answers7