C question: single dereference on a void** double indirection pointer

Question

I got this message:

expected 'void **' but argument is of type 'char **'

when I tried to compile something similar to this:

void myfree( void **v )
{
    if( !v || !*v )
        return;

    free( *v );
    *v = NULL;

    return;
}

I found what I think is a solution after reading this question on stack overflow:
Avoid incompatible pointer warning when dealing with double-indirection - Stack Overflow

So I adjusted to something like this:

#include <stdio.h>
#include <stdlib.h>

void myfree( void *x )
{
    void **v = x;

    if( !v || !*v )
        return;

    free( *v );
    *v = NULL;

    return;
}

int main( int argc, char *argv[] )
{
    char *test;

    if( ( test = malloc( 1 ) ) )
    {
        printf( "before: %p\n", test );
        myfree( &test );
        printf( "after: %p\n", test );
    }

    return 0;
}

Is this legal C? I am dereferencing a void pointer aren't I?

Thanks guys


EDIT 12/10/2010 4:45PM EST:
As it has been pointed out free(NULL) is safe and covered by the C standard. Also, as discussed below my example above is not legal C. See caf's answer, Zack's answer, and my own answer.

Therefore it's going to be easier for me to initalize any to-be-malloc'd pointers as NULL and then later on to just free() and NULL out directly in the code:

free( pointer );
pointer = NULL;

The reason I was checking for NULL in myfree() like I did was because of my experiences with fclose(). fclose(NULL) can segfault depending on platform (eg xpsp3 msvcrt.dll 7.0.2600.5512) and so I had assumed (mistakenly) the same thing could happen with free(). I had figured rather than clutter up my code with if statements I could better implement in a function.

Thanks everyone for all the good discussion

Answer 1

No, this is not legal C, unless you pass the address of a void * object to myfree() (so you might as well just keep your original definition).

The reason is that in your example, an object of type char * (the object declared as test in main()) is modified through an lvalue of type void * (the lvalue *v in myfree()). §6.5 of the C standard states:

7 An object shall have its stored value accessed only by an lvalue expression that has one of the following types:

— a type compatible with the effective type of the object,
— a qualified version of a type compatible with the effective type of 
the object,
— a type that is the signed or unsigned type corresponding to the effective
type of the object,
— a type that is the signed or unsigned type corresponding to a qualified
version of the effective type of the object,
— an aggregate or union type that includes one of the aforementioned
types among its members (including, recursively, a member of a subaggregate
or contained union), or
— a character type.

Since void * and char * are not compatible types, this constraint has been broken. The condition for two pointer types to be compatible is described in §6.7.5.1:

For two pointer types to be compatible, both shall be identically qualified and both shall be pointers to compatible types.

To achieve the effect you want, you must use a macro:

#define MYFREE(p) (free(p), (p) = NULL)

(There is no need to check for NULL, since free(NULL) is legal. Note that this macro evaluates p twice).

Answer 2

This is perfectly legal but can get confusing for other people who read your code.

You could also use casting to eliminate the warning:

myfree((void **)&rest);

This is more readable and understandable.

Answer 3

In C you have no choice but to introduce a cast somewhere in here. I would use a macro to ensure that things were done correctly at the call site:

void
myfree_(void **ptr)
{
    if (!ptr || !*ptr) return;
    free(*ptr);
    *ptr = 0;
}
#define myfree(ptr) myfree_((void **)&(ptr))

[You could actually name both the function and the macro "myfree", thanks to C's no-infinite-macro-recursion rules! But it would be confusing for human readers. Per the long discussion below caf's answer, I will also stipulate that the statement *ptr = 0 here modifies an object of unknown type through a void** alias, which is runtime-undefined behavior -- however, my informed opinion is, it will not cause problems in practice, and it's the least bad option available in plain C; caf's macro that evaluates its argument twice seems far more likely (to me) to cause real problems.]

In C++ you could use a template function, which is better on three counts: it avoids needing to take the address of anything at the call site, it doesn't break type correctness, and you will get a compile-time error instead of a run-time crash if you accidentally pass a non-pointer to myfree.

template <typename T>
void
myfree(T*& ptr)
{
    free((void *)ptr);
    ptr = 0;
}

But of course in C++ you have even better options available, such as smart pointer and container classes.

It should, finally, be mentioned that skilled C programmers eschew this kind of wrapper, because it does not help you when there's another copy of the pointer to the memory you just freed hanging around somewhere -- and that's exactly when you need help.

Answer 4

caf's answer is correct: No, it's not legal. And as Zack points out breaking the law in this way is apparently least likely to cause problems.

I found what appears to be another solution in the comp.lang.c FAQ list · Question 4.9, which notes that an intermediate void value has to be used.

#include <stdio.h>
#include <stdlib.h>

void myfree( void **v )
{
    if( !v )
        return;

    free( *v );
    *v = NULL;

    return;
}

int main( int argc, char *argv[] )
{
    double *num;

    if( ( num = malloc( sizeof( double ) ) ) )
    {
        printf( "before: %p\n", num );

        {
            void *temp = num;
            myfree( &temp );
            num = temp;
        }

        printf( "after: %p\n", num );
    }

    return 0;
}