1

I've seen in a few places objects constructed as such:

Foo foo;
foo = *(new Foo());

In my opinion this is horribly wrong. The new operator allocates memory for Foo, then the pointer is dereferenced and used to copy the contents to the original and already constructed foo object. The memory allocated is leaked, as the pointer is lost.

Is this evil and should never ever be done or am I missing something?

Hennio
  • 433
  • 2
  • 18
  • 1
    Possible duplicate of [C++ functions returning a reference and memory](http://stackoverflow.com/questions/38687049/c-functions-returning-a-reference-and-memory) – LogicStuff Oct 05 '16 at 08:10
  • 3
    This is true. . – KeatsPeeks Oct 05 '16 at 08:12
  • 8
    You are right, it is horrible! The unnecesary allocation of `Foo *foo = new Foo();` when no real dynamic object is needed (Java style) is bad enough, but this is evil. – rodrigo Oct 05 '16 at 08:12
  • Rhetorical question I guess... – 101010 Oct 05 '16 at 08:13
  • 1
    I suppose this kind of initialization comes from Java/C# programmers trying to create new objects, noticing the compiler does not allow them to do so as foo is an object not a pointer, and adding the * so the compiler is happy.. – Hennio Oct 05 '16 at 08:20
  • 1
    if a java/c# programmer do a new in modern C++ you could assume it's wrong. They're used to do new everywhere and in C++ it's almost always the worst thing to do. – ColdCat Oct 05 '16 at 08:57

1 Answers1

3

It's very very horrible, yes, but there's not a guaranteed memory leak.

That depends on the type Foo.

In practice there will never be a type where it's not a memory leak, but in principle one can define e.g.

struct Foo
{
    std::unique_ptr<Foo> p;
    void operator=( Foo& o ){ p.reset( &o ); }
};

I added the horrible incompatible-with-standard-containers void result type, just for good measure. :)

So, regarding

the memory allocated is leaked, as the pointer is lost. Is this true or am I missing something?

… you're ¹only missing the case of the trainee who copies the above code directly from this SO answer and inserts it in the company's Big Application™, for some inexplicable reason.

Notes:
¹ Martin Matilla writes in a comment on the question: “I suppose this kind of initialization comes from Java/C# programmers trying to create new objects, noticing the compiler does not allow them to do so as foo is an object not a pointer, and adding the * so the compiler is happy.”, which is another possibility.

Community
  • 1
  • 1
Cheers and hth. - Alf
  • 142,714
  • 15
  • 209
  • 331
  • how does it depend? – Hennio Oct 05 '16 at 08:13
  • You would have to work pretty damn hard to make this not a leak :-) – Kerrek SB Oct 05 '16 at 08:14
  • To make it not a leak, you can override the new/delete operator. – UKMonkey Oct 05 '16 at 08:32
  • @UKMonkey: Yes, that's another possibility. The allocation function could simply throw an exception, or terminate the program. But I think that's not quite in the spirit of the question, with code that somehow "works". – Cheers and hth. - Alf Oct 05 '16 at 08:37
  • Ohh - completely agree this is about as evil as lines of C++ get, but making this not leak isn't that hard, just takes a little thought... – UKMonkey Oct 05 '16 at 08:40
  • To be clear: The set of people who know enough about C++ to be able to think of ways to make this not leak, and the set of people who would use this in production code is pretty nearly disjoint. – Martin Bonner supports Monica Oct 05 '16 at 09:01
  • I still do not get how this is not a memory leak. You still have a `std::unique_ptr` out there that you can never get rid of until the program terminates. That IMHO is still a memory leak. – NathanOliver Oct 05 '16 at 12:14
  • @NathanOliver: The `std::unique_ptr` is in the instance called `foo` in the question's example code. At the end of the scope of `foo` it's destroyed. And as part of that, it destroys and deallocates the dynamically allocated instance. – Cheers and hth. - Alf Oct 05 '16 at 13:10
  • @Cheersandhth.-Alf But the `std::unique_ptr` from `*(new Foo())` is never destroyed. That is a memory leak. – NathanOliver Oct 05 '16 at 13:12
  • @NathanOliver: Try to read what I wrote again? Or, if that doesn't help, just test this. Add some trace output. Ask an SO question about it if that doesn't help. ;-) – Cheers and hth. - Alf Oct 05 '16 at 13:14