Where should I free memory inside functions?

Question

I write a shell in C. Although I do not expect many others to use it, I would like to practice writing supported and well-organized code. I noticed the following pattern in a number of my functions, so before it hardens, I want it to be fully tested.

As an example, consider the following function:

int foo(int param...) { // declare variables struct bar *a, *b, *c; // do some work a = bar_creator(); b = bar_modifier(a); c = bar_modifier(b); // cleanup free(a); free(b); free(c); return 1; } 

Notes:

• three stages: declaration, initiation / modification, cleaning

• newly allocated structures are often returned from functions as modified copies of other objects.

• a huge number of objects are not required, so memory usage is not a problem.

In its own way, the three sections were relatively different. This allows me to compare the first and last sections and ensure that everything is taken into account. Now I wonder if a better style can free something as soon as it is not needed. The motivation for this may be to minimize the context in which the code section makes sense.

What is your approach to freeing resources? What are the benefits of this strategy?

change

To eliminate confusion regarding the behavior of functions:

 /** * returns a newly created bar */ struct bar *bar_creator(); /** * takes a bar, and returns a _new_ copy of it that may have been modified. * the original is not modified. */ struct bar *bar_modifier(struct bar *param); 

Answer 1

Personally, I prefer to free objects immediately after I use them, and select them just before I need them. It makes me realize what kind of memory my program uses. Another advantage of this method is that it reduces the overall memory consumption if you allocate additional memory after freeing memory in the method.

Answer 2

Two different situations can be considered:

(1) an object is created in a local area, and it is not needed outside this local area. in this case you can allocate storage with calloc alloca () or using RAII . Using calloc alloca () has the big advantage of not having to worry about calling free () because the allocated memory is automatically freed when the local area remains.

(2) an object is created in a local area, and it is needed outside of this local area. In this case, there is no general advice. I will free the memory when the object is no longer needed.

EDITED: use alloca () instead of calloc ()

Answer 3

I tend to group frees at the end if I don't reuse the variable and you need to free it first. Thus, it is clear what exactly needs to be destroyed, which is useful if you are considering an early return , or if the function is a bit more complicated. Often your function will have several different control flows, and you want to be sure that they all hit the end of the cleanup, which is easier to see when the cleanup code is at the end.

Answer 4

I usually prefer the smallest amount possible, so I create the object as late as possible and I release them as soon as possible.

I will tend to have:

 char * foo; /* some work */ { foo = create(); /* use foo */ destroy(foo); } /* some other work */ { foo = create(); /* use foo */ destroy(foo); } 

Even if I could reuse the memory, I prefer to allocate it twice and release it twice. In most cases, the performance of this method is very small, since in most cases the two objects are different in any case, and if this is a problem, I try to optimize this recently in the dev process.

Now, if you have 2 objects with the same scope (or three as an example), this is the same:

 { foo1 = create(); foo2 = create(); foo3 = create(); /* do something */ destroy(foo1); destroy(foo4); destroy(foo3); } 

But this particular layout is applicable only when three objects have the same scope.

I try to avoid such a layout:

 { foo1 = create(); { foo2 = create(); /* use foo2 */ } destroy(foo1); /* use foo2 again */ destroy(foo2); } 

I think it's broken.

Of course, {} is given here just for an example, but you can also use them in actual code, or vim folds or anything that denotes a region.

When I need a wider scope (for example, global or general), I use a reference counter and a save mechanism (replace create with save and delete with release), and this always provides me with a nice and easy memory management.

Answer 5

• Usually dynamically allocated memory has a long service life (longer than a function call), so it makes no sense to talk about where within the function it frees.

• If the memory is needed only within the function, depending on the language it should be statically distributed, if necessary on the stack (declared as a local variable in the function, it will be allocated when the function is called and freed when the function exits, as shown in the example by another poster).

• As for naming, only functions that allocate memory and return it should be specifically named. Everything else is not worth saying “modfiier” - use this letter space to describe what the function does. That is, by default, we assume that it does not allocate memory, unless specifically indicated so (i.e. createX, allocX, etc.).

• In languages ​​or situations (i.e., to ensure consistency with code elsewhere in the program) where static alllocation is not suitable, then simulate a stack distribution pattern, highlighting the function at the beginning of the call and freeing it at the end.

• For clarity, if your function simply modifies an object, do not use the function at all. Use the procedure. It is absolutely clear that no new memory is allocated. In other words, eliminate your b and c pointers - they are not needed. They can change what they point to without returning a value.

• From the appearance of your code, you free up already freed memory, or bar_modifier is misleading, because it does not just modify the memory pointed to by a, but creates a new dynamically allocated memory. In this case, they should not be called bar_modifier, but create_SomethingElse.

Answer 6

Why do you free him 3 times?

If bar_creator() is the only function that dynamically allocates memory, you only need to free one of the pointers pointing to this memory area.

Answer 7

When you finish with it!

Don't let cheap memory prices promote lazy programming.

Answer 8

You need to be careful what happens when the memory fails. Since C does not support exceptions, I use goto to control the dynamic state of a reversal on error. Here's a trivial manipulation of your original function, demonstrating the technique:

 int foo(int param...) { // declare variables struct bar *a, *b, *c; // do some work a = bar_creator(); if(a == (struct bar *) 0) goto err0; b = bar_modifier(a); if(b == (struct bar *) 0) goto err1; c = bar_modifier(b); if(c == (struct bar *) 0) goto err2; // cleanup free(a); free(b); free(c); return 1; err2: free(b); err1: free(a); err0: return -1; } 

When using this method, I always want to have a return preceding the error labels to visually distinguish between the normal return case from the error case. Now this assumes that you are using the wind / unwind paradigm for your dynamically allocated memory ... What you are doing is looking more consistent, so I will probably have something closer to the following:

  a = bar_creator(); if(a == (struct bar *) 0) goto err0; /* work with a */ b = bar_modifier(a); free(a); if(b == (struct bar *) 0) goto err0; /* work with b */ c = bar_modifier(b); free(b); if(c == (struct bar *) 0) goto err0; /* work with c */ free(c); return 1; err0: return -1; 

Answer 9

Allows the compiler to clear the stack for you?

 int foo(int param...) { // declare variables struct bar a, b, c; // do some work bar_creator(/*retvalue*/&a); bar_modifier(a,/*retvalue*/&b); bar_modifier(b,/*retvalue*/&c); return 1; } 

Answer 10

For complex code, I would use structural diagrams to show how routines work, and then to distribute / release. I am trying to make them about the same level in the diagrams for this object.

In your case, I may be tempted to define a new function bar_destroyer, call it 3 times at the end of the function foo and make free () there.

Answer 11

Consider using a different template. Allocate variables on the stack if it is wise to do this (using declarations, not alloca). Consider making bar_creator bar_initialiser, which accepts a string bar *.

Then you can make your bar_modifier look like

 void bar_modifier(const struct bar * source, struct bar *dest); 

Then you do not need to worry so much about memory allocation.

In general, in C, it is better if the caller allocates memory rather than the memory being called - therefore, for some reason, strcpy is a "more pleasant" function, in my opinion, than strdup.