Setting variable to NULL after free

Viewed 117618

In my company there is a coding rule that says, after freeing any memory, reset the variable to NULL. For example ...

void some_func () 
{
    int *nPtr;

    nPtr = malloc (100);

    free (nPtr);
    nPtr = NULL;

    return;
}

I feel that, in cases like the code shown above, setting to NULL does not have any meaning. Or am I missing something?

If there is no meaning in such cases, I am going to take it up with the "quality team" to remove this coding rule. Please advice.

23 Answers

Setting unused pointers to NULL is a defensive style, protecting against dangling pointer bugs. If a dangling pointer is accessed after it is freed, you may read or overwrite random memory. If a null pointer is accessed, you get an immediate crash on most systems, telling you right away what the error is.

For local variables, it may be a little bit pointless if it is "obvious" that the pointer isn't accessed anymore after being freed, so this style is more appropriate for member data and global variables. Even for local variables, it may be a good approach if the function continues after the memory is released.

To complete the style, you should also initialize pointers to NULL before they get assigned a true pointer value.

This is considered good practice to avoid overwriting memory. In the above function, it is unnecessary, but oftentimes when it is done it can find application errors.

Try something like this instead:

#if DEBUG_VERSION
void myfree(void **ptr)
{
    free(*ptr);
    *ptr = NULL;
}
#else
#define myfree(p) do { void ** p_tmp = (p); free(*(p_tmp)); *(p_tmp) = NULL; } while (0)
#endif

The DEBUG_VERSION lets you profile frees in debugging code, but both are functionally the same.

Edit: Added do ... while as suggested below, thanks.

Setting the pointer to the free'd memory means that any attempt to access that memory through the pointer will immediately crash, instead of causing undefined behavior. It makes it much easier to determine where things went wrong.

I can see your argument: since nPtr is going out of scope right after nPtr = NULL, there doesn't seem to be a reason to set it to NULL. However, in the case of a struct member or somewhere else where the pointer is not immediately going out of scope, it makes more sense. It's not immediately apparent whether or not that pointer will be used again by code that shouldn't be using it.

It's likely the rule is stated without making a distinction between these two cases, because it's much more difficult to automatically enforce the rule, let alone for the developers to follow it. It doesn't hurt to set pointers to NULL after every free, but it has the potential of pointing out big problems.

If you reach pointer that has been free()d, it might break or not. That memory might be reallocated to another part of your program and then you get memory corruption,

If you set the pointer to NULL, then if you access it, the program always crashes with a segfault. No more ,,sometimes it works'', no more ,,crashes in unpredictible way''. It's way easier to debug.

The idea behind this, is to stop accidental reuse of the freed pointer.

This (can) actually be important. Although you free the memory, a later part of the program could allocate something new that happens to land in the space. Your old pointer would now point to a valid chunk of memory. It is then possible that someone would use the pointer, resulting in invalid program state.

If you NULL out the pointer, then any attempt to use it is going to dereference 0x0 and crash right there, which is easy to debug. Random pointers pointing to random memory is hard to debug. It's obviously not necessary but then that's why it's in a best practices document.

This rule is useful when you're trying to avoid the following scenarios:

1) You have a really long function with complicated logic and memory management and you don't want to accidentally reuse the pointer to deleted memory later in the function.

2) The pointer is a member variable of a class that has fairly complex behavior and you don't want to accidentally reuse the pointer to deleted memory in other functions.

In your scenario, it doesn't make a whole lot of sense, but if the function were to get longer, it might matter.

You may argue that setting it to NULL may actually mask logic errors later on, or in the case where you assume it is valid, you still crash on NULL, so it doesn't matter.

In general, I would advise you to set it to NULL when you think it is a good idea, and not bother when you think it isn't worth it. Focus instead on writing short functions and well designed classes.

This might be more an argument to initialize all pointers to NULL, but something like this can be a very sneaky bug:

void other_func() {
  int *p; // forgot to initialize
  // some unrelated mallocs and stuff
  // ...
  if (p) {
    *p = 1; // hm...
  }
}

void caller() {
  some_func();
  other_func();
}

p ends up in the same place on the stack as the former nPtr, so it might still contain a seemingly valid pointer. Assigning to *p might overwrite all kinds of unrelated things and lead to ugly bugs. Especially if the compiler initializes local variables with zero in debug mode but doesn't once optimizations are turned on. So the debug builds don't show any signs of the bug while release builds blow up randomly...

To add to what other have said, one good method of pointer usage is to always check whether it is a valid pointer or not. Something like:


if(ptr)
   ptr->CallSomeMethod();

Explicitly marking the pointer as NULL after freeing it allows for this kind of usage in C/C++.

Related