Is ptr = free(ptr), NULL safe?

Question

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.

`free` does not return a value, thus the line is bad style. The code is not even less typing, use the standard ideom. — too honest for this site, Oct 02 '16 at 01:49
AFAIK, comma operator discards `free` value, which is `void`, and uses `NULL` is instead. And it **is** less typing if your style demands consistently assign pointers to NULL after freeing them. — , Oct 02 '16 at 01:50
There is no value to discard! Did you try to compile this before asking? Did you find proof in the standard this is legal or illegal? Please cite the section. (oh, and I did try with gcc). — too honest for this site, Oct 02 '16 at 01:52
`void` is **not** a value! This is about C! There is no `void` type. — too honest for this site, Oct 02 '16 at 01:53
Well, yeah, but since it's discarded... I don't know if it matters. — , Oct 02 '16 at 01:54
Btw. the citation is wrong. Check the standard! Don't use non-autoritative resouces for language questions. — too honest for this site, Oct 02 '16 at 01:55
http://stackoverflow.com/questions/5401062/comma-operator-and-void-expression — tkausl, Oct 02 '16 at 01:56
if you try to compile it this will be the result `error: void value not ignored as it ought to be ` — rfreytag, Oct 02 '16 at 01:57
@pfannkuchen_gesicht I just tried OP to compile himself to see the result. Just telling someone the result is a bad idea. — too honest for this site, Oct 02 '16 at 01:58
@tkausl: Did you notice the tags? (The WP citation is wrong anyway) — too honest for this site, Oct 02 '16 at 01:59
Yes, and now it invokes undefined behaviour without you being notified. — too honest for this site, Oct 02 '16 at 02:00
@Olaf see the answer dbush. That works fine and is actually quite neat, considering you safe some writing and a line and get your pointer nulled. — rfreytag, Oct 02 '16 at 02:20
@pfannkuchen_gesicht: That was not my point! Yet it is uncommen in professional code, as it is less clear than a simple `free(p); p = NULL;`. But fee free to use it, I hardly see your code anyway. — too honest for this site, Oct 02 '16 at 02:24

dbush · Accepted Answer · 2016-10-02T03:39:10.527

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.

Is ptr = free(ptr), NULL safe?

1 Answers1