Is it bad manners to return a heap allocated pointer from a function?

Before boost::shared_ptr, was it considered a bad practice to return a heap allocated point开发者_运维问答er from a function, since the caller will be required to remember to free() that object?

Or, was it considered "normal"?

I don't consider it bad practice, so long as your API also provides an equivalent XXX_free (or XXX_close, XXX_clearup, or whatever) function, that the client code can call when finished with the pointer.

That way, you have a consistent, symmetrical API, in the sense that responsibility for the lifetime of a heap object is maintained in one place.

This approach is also amenable to more-complex resource freeing. For example, if the pointer that gets returned is to a dynamically-allocated struct that in turn has members that point to dynamically-allocated memory, the entire cleanup procedure can be hidden/abstracted from the client code.

You've tagged your question C. In C, it's very commonplace, e.g. fopen returning FILE * which must later be deallocated by calling fclose.

If you meant to tag it C++, then it's more complicated. Older codebases (mid-1990s and earlier) frequently passed around naked pointers to objects. Whole commercially supported libraries were based on that pattern (Borland's OWL, Microsoft MFC).

If you need to do this, it's common practice to provide your own "free" function that takes your allocated pointer and free it. This prevent user to use an incompatible free implementation that would corrupt memory.

You'll find examples of both methodologies: either a function allocating memory and returning a pointer, or a function accepting a pointer to already allocated space.

As long as the interface is clearly documented and followed, there are positives and negatives for both. For example, as many other people have mentioned, a library providing an allocation function typically should provide a delete function. As you, the client, don't know what method was used to allocate memory in that fancy function, you (the client) don't know what method should be used to destroy it.

On the other hand, the logic can be more complex when you need to worry about allocation of storage, passing that off to something that may or may not perform the work expected, and then determining if that storage is still pertinent, etc. Depending on the use of the memory, hiding away the allocation details could help encapsulate some optimization of it as well.

Short answer is: it's up to you. The really wrong way to go is to pick something and either be inconsistent or unclear in the interface.

It was, and is still common, as it's pretty much the only way to handle libraries built against different runtimes on certain operating systems. For example, on Windows, sharing multiple VC++ runtime libraries in a single executable typically will only work correctly if the libraries are responsible for all of their own memory management, including both the allocation of the objects as well as the disposal. shared_ptr and similar tools actually can cause issues in that case.

That being said, typically, the function name and/or documentation (when done correctly) would make it obvious this was happening. It was also common to have a corresponding DeleteXXX() function to handle the free call.

Generally you would only return a pointer from an explicit create_blah() function

More common is to pass in a pointer (to a pointer), which if it is null was allocated by the function.

Actually a better idiom (used by COM interfaces) is to require pointer creation at the caller before the point of the function call, and write a function that accepts a double pointer.

For example:

    HRESULT CreateDevice(
      [in]           UINT Adapter,
      [in]           D3DDEVTYPE DeviceType,
      [in]           HWND hFocusWindow,
      [in]           DWORD BehaviorFlags,
      [in, out]      D3DPRESENT_PARAMETERS *pPresentationParameters,
      [out, retval]  IDirect3DDevice9 **ppReturnedDeviceInterface
    );

So, the caller is responsible for creating the pointer and calling the allocation function, and so is more likely to remember to call the deallocation function (which in COM's case requires you call a ->Release() method on the COM object)

But I think adopting Creation/Destruction functions, combined with passing double pointers, is a better way to remind the recipient of a heap allocated object to clean up after using the object.

I agree with Oli that Create/Destroy functions are much more symmetrical and the existence of the Destroy function should turn the API user onto the fact that these objects he gets from the Create functions won't just disappear on their own, and they (Destroy fcns) need to be called.