Repeatedly freeing the same memory in C, preventing errors-CodePudding

This is probably very simple question, but i cant spacify it in proper way, to find the anwser.

So, i have learned, that it is good to free memory in that way:

void fun(int** arr)
{
    //some operations
    free(*arr);
    *arr = NULL; 
}

int main(int argc, char** argv)
{
    int* arr = calloc(10, sizeof(int));

    fun(&arr);
    free(arr); //this is a mistake made by someone

    return 0;
}

Firstly, is it really a good practice? (it was said, that it prevents errors if someone is freeing memory repeatedly)

Secondly, if this works, why this does not?

void fun(int* arr)
{
    //some operations
    free(arr);
    arr = NULL;
}

int main(int argc, char** argv)
{
    int* arr = calloc(10, sizeof(int));

    fun(arr);
    free(arr); //this is a mistake made by someone

    return 0;
}

CodePudding user response：

One of the main reasons why you'd set the pointer to NULL after calling free() is rather that it allows you to re-use the same pointer variable later, for allocating memory once again.

Also free(NULL) is required to be a safe no-op. Therefore code like if(ptr != NULL) free(ptr); is always nonsense.

So yeah setting the pointer to NULL after free() would also prevent someone from accidentally calling free() twice on the same pointer, but that's a rather weak argument since calling free() twice might be an application bug. In which case you'd rather like to have that bug surface early on by a heap corruption crash, instead of unknowingly hiding it under the carpet as a dormant bug.

Regarding your code, writing a function like that is obfuscation. Just write:

free(ptr); 
ptr = NULL;

This code is simple, readable and efficient and thus it's the best practice. Someone will make the "do not repeat yourself" argument, but it's also a weak argument since keeping code simple, readable and fast are all far more import things than avoiding code repetition. In particular, all C programmers can read and understand my code above at a glance, but they can't know what my_magic_free(&ptr) does without looking up that function, thus it is worse practice (and potentially also slower).

Not to be confused with writing an abstraction layer on top of all memory allocation, which is of course fine practice where it is appropriate.

You second example doesn't work because arr = NULL; just sets the local pointer variable inside that function and not the pointer on the caller side.

CodePudding user response：

Note that you need to write many functions if you do this to deallocate int, long, double, struct pointers, void pointers, and so on.

I would first check what your standard library implementations thinks about this. Some will tell you when you try to do a double free. Sometimes there are special debugging versions that will tell you. That's much easier.

If that's not enough, I'd write a function void* my_free(void* p) { free(p); return NULL; } and then you can write ppp = my_free(ppp); in your code. This also works if you now the pointer is stored in multiple locations: array[3] = a = b = c = my_free(c);

But make sure that you understand that you'd have to set all places where this pointer is stored to NULL.