c - Should one really set pointers to `NULL` after freeing them?

Question

There seem to be two arguments why one should set a pointer to NULL after freeing them.

Avoid crashing when double-freeing pointers.

Short: Calling free() a second time, by accident, doesn't crash when it's set to NULL.

• Almost always this masks a logical bug because there is no reason to call free() a second time. It's safer to let the application crash and be able to fix it.

• It's not guaranteed to crash because sometimes new memory is allocated at the same address.

• Double free occurs mostly when there are two pointers pointing to the same address.

Logical errors can lead to data corruption too.

Avoid reusing freed pointers

Short: Accessing freed pointers can cause data corruption if malloc() allocates memory in the same spot unless the freed pointer is set to NULL

• There's no guarantee that the program crashes when accessing the NULL pointer, if the offset is big enough (someStruct->lastMember, theArray[someBigNumber]). Instead of crashing there will be data corruption.

• Setting the pointer to NULL cannot solve the problem of having a different pointer with the same pointer value.

The questions

Here's a post against blindly setting a pointer to NULL after freeing.

• Which one is harder to debug?
• Is there a possibility to catch both?
• How likely is it, that such bugs lead to data corruption instead of crashing?

Feel free to expand this question.

Answer 1

The second one is way more important: re-using a freed pointer can be a subtle error. Your code keeps right on working, and then crashes for no clear reason because some seemingly unrelated code wrote in the memory that the re-used pointer happens to be pointing at.

I once had to work on a really buggy program someone else wrote. My instincts told me that many of the bugs were related to sloppy attempts to keep using pointers after freeing the memory; I modified the code to set the pointers to NULL after freeing the memory, and bam, the null pointer exceptions started coming. After I fixed all the null pointer exceptions, suddenly the code was much more stable.

In my own code, I only call my own function that is a wrapper around free(). It takes a pointer-to-a-pointer, and nulls the pointer after freeing the memory. And before it calls free, it calls Assert(p != NULL); so it still catches attempts to double-free the same pointer.

My code does other things too, such as (in the DEBUG build only) filling memory with an obvious value immediately after allocating it, doing the same right before calling free() in case there is a copy of the pointer, etc. Details here.

EDIT: per a request, here is example code.

void
FreeAnything(void **pp)
{
    void *p;

    AssertWithMessage(pp != NULL, "need pointer-to-pointer, got null value");
    if (!pp)
        return;

    p = *pp;
    AssertWithMessage(p != NULL, "attempt to free a null pointer");
    if (!p)
        return;

    free(p);
    *pp = NULL;
}


// FOO is a typedef for a struct type
void
FreeInstanceOfFoo(FOO **pp)
{
    FOO *p;

    AssertWithMessage(pp != NULL, "need pointer-to-pointer, got null value");
    if (!pp)
        return;

    p = *pp;
    AssertWithMessage(p != NULL, "attempt to free a null FOO pointer");
    if (!p)
        return;

    AssertWithMessage(p->signature == FOO_SIG, "bad signature... is this really a FOO instance?");

    // free resources held by FOO instance
    if (p->storage_buffer)
        FreeAnything(&p->storage_buffer);
    if (p->other_resource)
        FreeAnything(&p->other_resource);

    // free FOO instance itself
    free(p);
    *pp = NULL;
}

Comments:

You can see in the second function that I need to check the two resource pointers to see if they are not null, and then call FreeAnything(). This is because of the assert() that will complain about a null pointer. I have that assert in order to detect an attempt to double-free, but I don't think it has actually caught many bugs for me; if you want to leave out the asserts, then you can leave out the check and just always call FreeAnything(). Other than the assert, nothing bad happens when you try to free a null pointer with FreeAnything() because it checks the pointer and just returns if it was already null.

My actual function names are rather more terse, but I tried to pick self-documenting names for this example. Also, in my actual code, I have debug-only code that fills buffers with the value 0xDC before calling free() so that if I have an extra pointer to that same memory (one that doesn't get nulled out) it becomes really obvious that the data it's pointing to is bogus data. I have a macro, DEBUG_ONLY(), which compiles to nothing on a non-debug build; and a macro FILL() that does a sizeof() on a struct. These two work equally well: sizeof(FOO) or sizeof(*pfoo). So here is the FILL() macro:

#define FILL(p, b) 
    (memset((p), b, sizeof(*(p)))

Here's an example of using FILL() to put the 0xDC values in before calling:

if (p->storage_buffer)
{
    DEBUG_ONLY(FILL(pfoo->storage_buffer, 0xDC);)
    FreeAnything(&p->storage_buffer);
}

An example of using this:

PFOO pfoo = ConstructNewInstanceOfFoo(arg0, arg1, arg2);
DoSomethingWithFooInstance(pfoo);
FreeInstanceOfFoo(&pfoo);
assert(pfoo == NULL); // FreeInstanceOfFoo() nulled the pointer so this never fires