Whether following code is prone to memory leak?

Question

I am new to C++ and I want to know whether following code is prone to memory leak. Here I am using a std::ostream pointer to redirect output to either console or to a file. For this I am calling new operator for std::ofstream.

#include <iostream>
#include <fstream>

int main() {
    bool bDump;

    std::cout << "bDump bool" << std::endl;
    std::cin >> bDump;

    std::ostream *osPtr;

    if (bDump) {
        osPtr = new std::ofstream("dump.txt");
    } else {
        osPtr = &std::cout;
    }

    *osPtr << "hello";
    return 0;
}

And one more thing, I have not closed the file which I opened while calling constructor for ofstream. Do we have any potential data loss situation here. as file is not closed.

Answer 1

Yes. Definitely. Any time you call new without delete, there's a memory leak.

After your code has executed, you need to add this:

        if(bDump)
        {
            delete osPtr;
        }

Answer 2

As @Mahmoud Al-Qudsi mentioned anything you new must also be deleted otherwise it will be leaked.

In most situation you do not want to use delete but rather you want to use a smart pointer to auto-delete the object. This is because in situations with exceptions you could again leak memory (while RAII) the smart pointer will guarantee that the object is deleted and thus the destructor is called.

It is important the at the destructor is called (especially in this case). If you do not call the destructor there is a potential that the not everything in the stream will be flushed to the underlying file.

#include <iostream>
#include <fstream>

int doStuff()
{
    try
    {
        bool bDump;

        std::cout<<"bDump bool"<<std::endl;
        std::cin>>bDump;

        // Smart pointer to store any dynamic object.
        std::auto_ptr<std::ofstream>   osPtr;

        if(bDump)
        {
            // If needed create a file stream
            osPtr.reset(new std::ofstream("dump.txt"));
        } 

        // Create a reference to the correct stream.
        std::ostream&  log = bDump ? *osPtr : std::cout;

        log << "hello";
     }
     catch(...) {throw;}

 } // Smart pointer will correctly delete the fstream if it exists.
   // This makes sure the destructor is called.
   // This is guaranteed even if exceptions are used. 

Answer 3

Yes, anything that is newed but never deleteed leaks.

In some cases, it is perfectly reasonable to allocate left right and center, and then just exit, particularly for short-lived batch-style programs, but as a general rule you should delete everything you new and delete[] everything you new[].

Especially in the case above, leaking the object is unsafe, since the object being leaked is an ostream that will never write unflushed content.

Answer 4

There is no leak in the code shown. At all times during the execution, all allocated objects are referenceable. There is only a memory leak if an object has been allocated and cannot be referenced in any way.

If the pointer goes out of scope, or its value is changed, without the object being deallocated, that is a memory leak. But so long as the pointer is in the outermost scope and nothing else changes its value, there is no leak.

"In object-oriented programming, a memory leak happens when an object is stored in memory but cannot be accessed by the running code." Wikipedia -- 'Memory leak'

The other answers suggest that any program that uses typical singleton patterns or doesn't free all allocated objects prior to termination has a memory leak. This is, IMO, quite silly. In any event, if you accept that definition, almost every real world program or library has a memory leak, and memory leaks are certainly not all bad.

In a sense, this kind of coding is prone to a memory leak, because it's easy to change the value of the pointer or let it go out of scope. In that case, there is an actual leak. But as shown, there is no leak.

You may have a different problem though: If the destructor has side-effects, not calling it can result in incorrect operation. For example, if you never call the destructor on a buffered output stream writing to a file, the last writes may never actually happen because the buffer didn't get flushed to the file.