In c++, is passing an on-the-fly dynamically allocated object to a function (always) a bad idea?

Question

I know the title of the question looks a bit mind-damaging, but I really don't know how to ask this in one phrase. I'll just show you what I mean:

void f(T *obj)
{
    // bla bla
}
void main()
{
    f(new T());
}

As far as I know, (almost) every new requires a delete, which requires a pointer (returned by new). In this case, the pointer returned by new isn't stored anywhere. So would this be a memory leak?

Does C++ work some kind of magic (invisible to the programmer) that deletes the object after the function ends or is this practice simply always a bad idea?

Answer 1

There is no particular magic and delete will not be called automatically.

It is definitely not "always a bad idea" - if function takes ownership of an object in some form it is perfectly valid way for calling such function:

container.AddAndTakeOwnership(new MyItem(42));

Answer 2

Yes, it's always a bad idea; functions and constructors should be explicit about their ownership semantics and if they expect to take or share ownership of a passed pointer they should receive std::unique_ptr or std::shared_ptr respectively.

There are lots of legacy APIs which take raw pointers with ownership semantics, even in the standard library (e.g. the locale constructor taking a Facet * with ownership), but any new code should avoid this.

Even when constructing a unique_ptr or shared_ptr you can avoid using the new keyword, in the latter case using make_shared and in the former case writing a make_unique function template which should fairly soon be added to the language.

Answer 3

The code shown will result in a memory leak. C++ does not have garbage collection unless you explicitly use a specialized framework to provide it.

The reason for this has to do with the way memory is managed in C/C++. For a local variable, like your example, memory for the object is requested directly from the operating system (malloc) and then the pointer to the object exists on the stack. Because C/C++ can do arbitrarily complex pointer arithmetic, the compiler has no way of knowing whether there exists some other pointer somewhere to the object, so it cannot reclaim the memory when function f() ends.

In order to prevent the leak automatically, the memory would have to be allocated out of a managed heap, and every reference into this heap would have to be carefully tracked to determine when a given object no longer was being used. You would have to give up C's ability to do pointer arithmatic in order to get this capability.

For example, let's say the compiler could magically figure out that all normal references to obj were defunct and deleted the object (released the memory). What if you had some insanely complicated RUNTIME DEPENDENT expression like void* ptr = (&&&&(&&&*obj)/2++ - currenttime() - 567 + 3^2 % 52) etc; How would the compiler know whether this ptr pointed to obj or not? There is no way to know. This is why there is no garbage collection. You can either have garbage collection OR complex runtime pointer arithmetic, not both.

Answer 4

In this case, the pointer returned by new isn't stored anywhere. So would this be a memory leak?

No, it is not necessarily a memory leak. The pointer is stored as an argument to f:

void f(T *obj)
//        ^^^  here pointer is "stored"
{
    // bla bla
    delete obj; // no memory leak if delete will be called on obj
}
void main()
{
    f(new T());
  //  ^^^^^^^  this "value" will be stored as an argument to f
}

Does C++ work some kind of magic (invisible to the programmer) that deletes the object after the function ends or is this practice simply always a bad idea?

No magic in your example. As I showed - delete must be called explicitly.

Better is to use smart pointer, then C++ "magic" works, and delete is not needed.

void f(std::unique_ptr<T> obj)
{
    // bla bla
}
void main()
{
    f(new T());
}

Answer 5

This is often used when your function f() is storing the object somewhere, like an array (or any other data container) or a simple class member; in which case, deletion will (and must) take place somewhere else.

Otherwise is not a good idea cause you will have to delete it manually at the end of the function anyway. In that case, you can just declare an automatic (on the stack) object and pass it by pointer.

Answer 6

No magic. In your case after f is called main returns back to CRT's main and eventually the OS will clean up the "leak". Its not necessarily a bad idea, it could be giving f the ownership and it is up to f to do stuff and eventually delete. Some make call it bad practice but its probably out there in the wild.

Edit: Although I see that code no more dangerous than:

void f(T *obj)
{
    // bla bla
}
void main()
{
    T* test = new T ();
    f(test);
}

Principally it is the same. Its saying to f, here is a pointer to some memory, its yours you look after it now.

Answer 7

In C++ passing a pointer is discouraged as there are no associated owner semantics (ie you can not know who owns the pointer and thus who is responsible for deleting the pointer). Thus when you do have a function (that takes a pointer) you need to document very clearly if the function is responsible for cleaning up the pointer or not. Of course documentation like this is very error prone as the user has to read it.

It is more normal in C++ program to pass objects that describe the ownership semantics of the thing.

1. Pass by reference
   The function is not taking ownership of the object. It will just use the object.
2. Pass a std::auto_ptr (or std::unique_ptr)
   The function is being passed the pointer and ownership of the pointer.
3. Pass a std::shared_ptr
   The function is being passed shared ownership of the pointer.

By using these techniques you not only document the ownership semantics but the objects used will also automatically control the lifespan of the object (thus relieving your function from calling delete).

As a result it is actually very rare to see a manual call to delete in modern C++ code.

So I would have written it like this:

void f(std::unique_ptr<T> obj)
{
    // bla bla
}
int main()
{
    f(std::unique_ptr<T>(new T()));
}