Memory clean up in C: where to check pointers?

Question

You're creating a struct and you need to destroy it later in your program. However due to some error the struct might actually not have been allocated, so you need to check if the pointer is valid (EDIT: checking for NULL doesn't ensure the pointer is valid, see LucasHenrique's and Giorgi's comments). Basically you can do this in two ways.

Check at the 'top':

main.c:

struct my_struct* foo = create();
...
if (foo) {
  destroy(foo);
}

mystruct.c:

void destroy(struct my_struct* foo) {
  /* Do clean up */
}

Check at the 'bottom':

main.c:

struct my_struct* foo = create();
...
destroy(foo);

mystruct.c:

void destroy(struct my_struct* foo) {
  if (foo) {
    /* Do clean up */
  }
}

The first one makes sure that the user is cleaning up everything correctly, but the second one gives you cleaner code inside main.c (although 'destroy' might not be the best name for that function in that case). I can't really decide which principle I should follow and I'd like to know if there are any real disadvantages to one of them?

Checking if the pointer isn't null doesn't means that the pointer is valid. — , Apr 21 '15 at 18:40
Note that this is a somewhat uncommon problem to try to solve as it's not possible to check if a pointer is valid in the general case.. e.g. the standard C library doesn't solve it, and if you pass in something invalid to the standard library functions, all bets are off. You could assume/document that any caller should set the pointer to NULL after it's destroyed, and detect a NULL pointer being passed in to your functions. — nos, Apr 21 '15 at 19:34

score 1 · Answer 1 · answered Apr 21 '15 at 18:41

1

The destroy() function should definitely check whether there is anything to do.

Besides that, less code is needed to be written, compiled, etc. since presumably there are many calls to destroy() but only once instance of it.

Note that destroy() cannot NULL out the pointer when it is called:

foo = create ();
/* use foo a lot */

destroy (foo);

/* No matter what destroy() does, foo is unchanged here */
/* and can be dangerously dereferenced to inspect or modify the object, etc. */

answered Apr 21 '15 at 18:41

wallyk

56,922
16
83
148

So essentially I should do `foo = destroy(foo)` and return `NULL`? – Apr 21 '15 at 18:56
@gwj62: That is not a bad idea, but there is nothing to assure the code uses the return value. Passing the pointer by reference could do a better job. `destroy(&foo)` and then changing `destroy` accordingly. – wallyk Apr 21 '15 at 19:59

score 0 · Answer 2 · answered Apr 21 '15 at 18:40

Let's assume you are writing a public library. If you take example of the C library, in most functions you have to pass pointers to valid objects (which a null pointer is not). One exception is the free function (similar to a destroy function) that allows free(NULL) and does a no-operation with such an argument.

Memory clean up in C: where to check pointers?

2 Answers2