Design Pattern to free an object in C-CodePudding

When programming in C, we usually create data structures that we initialize, then free when it is no longer needed. For instance, if we want to create a dynamic array of double, it is common to declare

struct vector {
    double *data;
    int size;
    int capacity;
}
typedef struct vector vector;

vector *v_new(int n) {
    vector *v = malloc(sizeof(vector));
    v->data = malloc(n * sizeof(double));
    v->size = n;
    v->capacity = n;
    return v;
}

The question is about the common patterns for a free function. In C, the function free accepts the NULL pointer and does nothing. Is it a common pattern to design v_free functions in such a way, or are they usually expecting a non-NULL pointer? To make it clear, would you expect this implementation

void v_free(vector *v) {
    if (v != NULL) {
        free(v->data);
    }
    free(v);
}

or this one ?

void v_free(vector *v) {
    free(v->data);
    free(v);
}

This question is asked because we began to teach C to undergraduate students in prep school in France, and we don't have that much experience in "C Design Patterns".

Thanks for your advice.

CodePudding user response：

You can't access v->data if v is NULL. So if there is a chance of that, you must do the version which checks for that, which is better written as

void v_free(vector *v) {
    if (v != NULL) {
        free(v->data);
        free(v);
    }
}

If v should never be NULL here, it's perhaps better to add an assert to make the assumption explicit:

void v_free(vector *v) {
    assert(v != NULL);
    free(v->data);
    free(v);
}

That way the programmer will notice they are doing something wrong.

Note that neither version detects a dangling pointer, ie. pointer which points to already destroyed object. This includes pointing to memory already freed (ie. you'd have double free here) or by pointer having pointed to local variable which is not in scope any more.

CodePudding user response：

The question is about the common patterns for a free function. In C, the function free accepts the NULL pointer and does nothing. Is it a common pattern to design v_free functions in such a way, or are they usually expecting a non-NULL pointer?

This is going to be a matter of opinion.

My opinion is, unless you have a good reason otherwise, program defensively. Do the thing that will make debugging a mistake easier. Make v_free error on a null pointer. Something as simple as an assert.

void v_free(vector *v) {
    assert(v != NULL);

    free(v->data);
    free(v);
}

Consider if we quietly ignore the null case. Did the caller intend to pass a null pointer, or was it a mistake? We don't know. If it was a mistake, the program continues merrily along and probably mysteriously crashes elsewhere. This makes debugging more difficult.

Consider if we assume v_free will always receive a non-null pointer. If it does free(v->data) is undefined behavior. At best a messy error, at worst the program continues merrily along and probably mysteriously crashes elsewhere. This makes debugging more difficult.

But if we provide an error, the mistake is stopped and revealed. Do the same thing for all your vector functions.

"But what if I want to pass a null pointer?" That should be infrequent, don't optimize for it. Make the caller do the check. If they really need to do it frequently, they can write a little wrapper function.

void v_free_null(vector *v) {
    if( v == NULL ) {
        return;
    }
    v_free(v);
}