4

A function (Say "fun()") allocates memory and returns the pointer to allocated memory. How should I make sure I that this memory is released. I can't release it immediately in function "fun()" as it is returned to caller. And what if fun() is part of library? Whose responsibility it is to free memory. In case of fopen(), the memory is released by fclose(). But in my case, "fun()" is called repeatedly. So I can not wait till end to release the memory.

John Yang
  • 1,300
  • 10
  • 23
Rahul
  • 257
  • 1
  • 4
  • 11
  • 6
    Is this C or C++? – Steven Oct 12 '11 at 11:28
  • Who makes the mess should clear it up! Apply that logic to memory management. – Ed Heal Oct 12 '11 at 11:30
  • 6
    @EdHeal: nice catchphrase, but it can be interpreted both ways. Who makes the mess, the library or the program that decided to call into the library? – Fred Foo Oct 12 '11 at 11:37
  • Thank you. Why not just pass a pointer into the function (aka the person that has made the mess) and then that function could populate the memory. That memory could be either on the heap or on the stack. – Ed Heal Oct 12 '11 at 17:55

6 Answers6

5

The following is the answer for C, posted before the OP confessed to using C++. In that language, use RAII and smart pointers, as recommended by others.

If a function returns allocated memory, then the caller is responsible for deallocation and this must be stated in the function's documentation.

If more cleanup is needed then is offered by free, or such cleanup may to be needed in future versions of the library, then you should offer a cleanup function (like stdio does with fclose) that does the deallocation. If you can't predict whether extra cleanup may be necessary in the future, then it's a good idea to assume it will be at some point. Wrapping free is cheap.

Think of it as a form of symmetry: if the client gets a resource (object) from the library, then it is eventually responsible for handing it back to the library for disposal:

void use_the_foo_library()
{
    Foo *f = make_foo();
    if (f == NULL)
        ERROR();

    foo_do_bar(f);
    foo_do_baz(f);

    foo_destroy(f);
}

where in foolib 1.0, foo_destroy is just

void foo_destroy(Foo *p)
{
    free(p);
}

but in version 2.0, it may have grown to

void foo_destroy(Foo *p)
{
    fclose(p->logfile);
    free(p);
}

etc. This style is consistent with the opaque pointer design pattern. It also gives you the freedom of replacing malloc and free at any point with a special purpose memory allocator, such as a pool allocator, without having to change any client code.

Fred Foo
  • 355,277
  • 75
  • 744
  • 836
5

If it's C++, don't return a raw pointer to memory, return a smart pointer instead.

eg:

std::shared_ptr<TypePointedTo> data = fun();

That way, when the shared_ptr destructs it will automatically free the memory for you.

Or, if it's an array you want to return use a vector, again this will automatically free the memory for you:

std::vector<BYTE> data = fun();

Read the excellent comments, std::unique_ptr could be a better than std::shared_ptr in a lot of scenarios.

If it's C ... see other answers!

Scott Langham
  • 58,735
  • 39
  • 131
  • 204
  • 3
    It's very bad form for a library to return a `shared_ptr`, as this imposes a perhaps inappropriate strategy on the client. Once an object is managed by `shared_ptr`, it must always be managed by `shared_ptr`. A far better solution is to return an `std::auto_ptr`; by calling `release()`, the client code can recover full ownership and use the most appropriate strategy. And if the most appropriate strategy is `shared_ptr`... `shared_ptr` has a constructor which takes an `std::auto_ptr`. – James Kanze Oct 12 '11 at 12:05
  • 3
    It's quite good form to return a `shared_ptr` **if** the object pointed to is shared by the library and the client code. Otherwise, `std::unique_ptr` would be the most appropriate (or indeed `auto_ptr` for backwards compat). – Fred Foo Oct 12 '11 at 12:09
  • I am with James here, if you don't know your client, do not impose your requirements on them. If you are going to share the object between the library and user code, or you know for a fact (a different overload?) that the user wants a `shared_ptr`, and you can use `make_shared`, allocating it internally might have a slight performance boost. That is, if unsure, offer `auto_ptr`/`unique_ptr`, and consider an overload with `shared_ptr` *only* if you can internally use `make_shared`. – David Rodríguez - dribeas Oct 12 '11 at 14:18
2

C Solution:

And what if fun() is part of library?

Your library api should then doccument the fact memory needs to freed by the caller.
You also should provide a free function for the same and mandate that user of the library should call it to free the allocation.

EDIT:

C++ Solution:

Since you edited to say that you are using C++, perhaps it is best to use smart pointers (std::tr1::shared_ptr)to automatically handle the memory for you.

If you cannot use smart pointers for some reason using std::vector is also a good choice.

Alok Save
  • 202,538
  • 53
  • 430
  • 533
  • Thanks!! Just curious, Do we have anything that says like that in standard c/c++ libraries ? – Rahul Oct 12 '11 at 11:32
  • @Rahul: Think of it as `fopen` and `fclose`, `fopen` causes allocation of some resources(handles etc), while the user has to explicitly call `fclose` to deallocate those resources after done with usage of the file, only difference in your case the resource is a dynamic memory. – Alok Save Oct 12 '11 at 11:38
  • @No, not really but look at Als answer. – Prof. Falken Oct 12 '11 at 11:39
  • @AmigableClarkKant: This is my answer, Sorry.I didnt understand your comment. – Alok Save Oct 12 '11 at 11:45
  • Now that I read it again maybe I was confused myself. :-) I just meant that there is nothing in any standard that says that 3rd party libraries have to do one way or another. Your answer is good advice. – Prof. Falken Oct 12 '11 at 11:49
  • On the use of smart pointers... before blindly using `shared_ptr` consider if the resource is to be *shared* or not. `unique_ptr` offers more flexibility to the end user (they can decide to hold *unique* ownership or *share* it by extracting the pointer and adding it to a `shared_ptr`) Once a pointer is held by a `shared_ptr` you can no longer release ownership of the resource. – David Rodríguez - dribeas Oct 12 '11 at 12:08
2

In C++, you would return a smart pointer, which clearly states (and enforces) that ownership has been transferred to the caller, and allows the caller to choose what to do with it. It also provides exception safety, preventing a memory leak if an exception (or just an early function return) causes the only pointer to the resource to go out of scope.

In C++03, std::auto_ptr is the best choice here; in C++11, this is deprecated in favour of std::unique_ptr.

Mike Seymour
  • 249,747
  • 28
  • 448
  • 644
0

If you need the memory outside of the callee, it's pretty obvious that it is responsible to free up the memory, since you can't really free it in the callee. It's the same as new and delete - you simply must remember to free that memory. Make sure you document the fact that the caller is responsible for memory management.

Luchian Grigore
  • 253,575
  • 64
  • 457
  • 625
0

You have to document that memory is allocated and that the caller is responsible for freeing it. You can't free it yourself as you can't know the caller's intention. you'd be freeing it before use.

You could provide a cleanUp() method to be called and flush the memory but that relies on the caller still and you add complications around when it should be called.

The only real alternative is to set up a clever "reference counting" mechanism such as in Objective-C (see release and autorelease).

wmorrison365
  • 5,995
  • 2
  • 27
  • 40