cvariable-assignmentcomma-operatorsymmetry

Is ptr = free(ptr), NULL safe?


I'm writing code like this:

#include <stdlib.h>

int main(void)
{
    void *kilobyte;
    kilobyte = malloc(1024);
    kilobyte = NULL, free(kilobyte);
    return 0;
}

for symmetry, which is nice. But I've never seen anyone else using this idiom before, so I wonder if this might actually be unportable/unsafe, despite this Wikipedia quote:

In the C and C++ programming languages, the comma operator (represented by the token ,) is a binary operator that evaluates its first operand and discards the result, and then evaluates the second operand and returns this value (and type).


Edit: mixed up the order. Now it compiles on gcc without any warnings.


Solution

  • By doing this:

    kilobyte = NULL, free(kilobyte);
    

    You have a memory leak.

    You set kilobyte to NULL, so whatever memory it was pointing to is no longer referenced anywhere. Then when you do free(kilobyte), you're effectively doing free(NULL) which performs no operation.

    Regarding free(NULL), from the C standard.

    7.22.3.3 The free function

    1.

    #include <stdlib.h>
    void free(void *ptr);
    

    2. The free function causes the space pointed to by ptr to be deallocated, that is, made available for further allocation. If ptr is a null pointer, no action occurs. Otherwise, if the argument does not match a pointer earlier returned by a memory management function, or if the space has been deallocated by a call to free or realloc, the behavior is undefined.

    As for your original code before the edit:

    kilobyte = free(kilobyte), NULL;
    

    The problem with this is that the = operator has higher precedence than the , operator, so this statement is effectively:

    (kilobyte = free(kilobyte)), NULL;
    

    This tries to set a variable to void which is not allowed.

    What you probably indented to do is this:

    kilobyte = (free(kilobyte), NULL);
    

    This frees the pointer, then sets the pointer to NULL.

    As mentioned by Olaf in the comments, rather than doing everything in one line, it would be preferable to do this instead:

    free(kilobyte);
    kilobyte = NULL;
    

    Doing this is more clear to the reader than condensing the code into something others might not understand, and (as you've now seen) is less error prone.