Dangling pointers and double free

Question

After some painful experiences, I understand the problem of dangling pointers and double free. I am seeking proper solutions.

aStruct has a number of fields including other arrays.

aStruct *A = NULL, *B = NULL;
A = (aStruct*) calloc(1, sizeof(sStruct));
B = A;
free_aStruct(A);
...
// Bunch of other code in various places.
...
free_aStruct(B);

Is there any way to write free_aStruct(X) so that free_aStruct(B) exits gracefully?

void free_aStruct(aStruct *X) {
    if (X ! = NULL) {
        if (X->a != NULL) { free(X->a); x->a = NULL; }
        free(X); X = NULL;
    }
}

Doing the above only sets A = NULL when free_aStruct(A); is called. B is now dangling.

How can this situation be avoided / remedied? Is reference counting the only viable solution? Or, are there other "defensive" approaches to freeing memory, to prevent free_aStruct(B); from exploding?

Answer 1

In plain C, the most important solution to this problem is discipline, because the root of the problem is here:

B = A;

Making a copy of the pointer without changing anything within your struct, circumventing whatever you use without any warning from the compiler. You have to use something like this:

B = getref_aStruct(A);

The next important thing is to keep track of the allocations. Some things that help are clean modularization, information hiding and DRY -- Don't Repeat Yourself. You directly call calloc() to allocate the memory while you use a free_aStruct() function to free it. Better use a create_aStruct() to allocate it. This keeps things centralized and in one place only, instead of throwing memory allocations all over your codebase.

This is a much better base for whatever memory tracking system you build on top of this.

Answer 2

I do not think you can do this automatically as C places the onus and burden of you to manage the memory and therefore your responsibility to ensure that references and of course dangling pointers are looked after!

void free_aStruct(aStruct *X){
  if (X ! = NULL){
      if (X->a != NULL){free(X->a); x->a = NULL;}
      free(X); X = NULL;
}
}

By the way, there's a typo blip in the if check above ... use of lower case 'x' instead of 'X'...

My thinking when I was looking at the above code is that you are doing a free on a copy of a pointer variable of type aStruct *. I would modify it to be a call-by-reference instead...

void free_aStruct(aStruct **X){
  if (*X ! = NULL){
      if (*X->a != NULL){
          free(*X->a); 
          *X->a = NULL;
      }
      free(*X); 
      *X = NULL;
  }
}

And call it like this:

free_aStruct(&A);

Other than that, you are ultimately responsible for the 'dangling pointers' yourself whether its an unintentional coding or a design fault...

Answer 3

Even if you could prevent the free_aStruct(B) from blowing up, if there's any reference to B in the code behind your comment, that's going to be using memory that's been freed, and so might be overwritten with new data at any point. Just "fixing" the free call will only mask the underlying error.

Answer 4

There are techniques you can use but the bottom line is that nothing you do can be strictly enforcable in C. Instead, i recommend incorporating valgrind (or purify) in your development process. Also, some static code analyzers may be able to detect some of these problems.

Answer 5

Reference counting's really not that hard:

aStruct *astruct_getref(aStruct *m)
{
    m->refs++;
    return m;
}

aStruct *astruct_new(void)
{
    sStruct *new = calloc(1, sizeof *new);
    return astruct_getref(new);
}

void astruct_free(aStruct *m)
{
    if (--m->refs == 0)
        free(m);
}

(In a multithreaded environment you will also potentially need to add locking).

Then your code would be:

aStruct *A = NULL, *B = NULL;
A = astruct_new();
B = astruct_getref(A);
astruct_free(A);
...
//bunch of other code in various places.
...
astruct_free(B);

---

You've asked about locking. Unfortunately there's no one-size-fits-all answer when it comes to locking - it all depends on what access patterns you have in your application. There's no substitute for careful design and deep thoughts. (For example, if you can guarantee that no thread will be calling astruct_getref() or astruct_free() on another thread's aStruct, then the reference count doesn't need to be protected at all - the simple implementation above will suffice).

That said, the above primitives can easily be extended to support concurrent access to the astruct_getref() and astruct_free() functions:

aStruct *astruct_getref(aStruct *m)
{
    mutex_lock(m->reflock);
    m->refs++;
    mutex_unlock(m->reflock);
    return m;
}

aStruct *astruct_new(void)
{
    sStruct *new = calloc(1, sizeof *new);
    mutex_init(new->reflock);
    return astruct_getref(new);
}

void astruct_free(aStruct *m)
{
    int refs;

    mutex_lock(m->reflock);
    refs = --m->refs;
    mutex_unlock(m->reflock);
    if (refs == 0)
        free(m);
}

...but note that any variables containing the pointers to the structs that are subject to concurrent access will need their own locking too (for example, if you have a global aStruct *foo that is concurrently accessed, it will need an accompanying foo_lock).