[Q] about free(ptr)

[kj]

How can one test if a pointer has been "freed" (i.e. with free())?
My naive assumption was that such a pointer would equal NULL, but
not so.

Thanks,

kj

 

[Gordon Burditt]

How can one test if a pointer has been "freed" (i.e. with free())?

You don't, unless you want to risk having your program crash.

My naive assumption was that such a pointer would equal NULL, but
not so.

There is nothing preventing you from setting a pointer variable to
NULL after passing its value to free(), but that doesn't get the
other copies that might be around.

Gordon L. Burditt

 

[subnet]

How can one test if a pointer has been "freed" (i.e. with free())?
My naive assumption was that such a pointer would equal NULL, but
not so.

Think about this: when you call free(ptr), ptr is passed by value, so
free() cannot modify it. I think there's no way of knowing whether a
pointer points to something valid or not, apart from writing one's own
free(), ie something like

void my_free(void **ptr) {
free(*ptr);
*ptr = NULL;
}

int *p = malloc(200);
....
my_free(&p);

but I think it's not advisable.

HTH

 

[Keith Thompson]

You can't. Once you've called free(p), the value of p is
indeterminate. Any attempt to refer to the value of p invokes
undefined behavior. (Yes, that's any attempt to refer to the value of
p, not just any attempt to dereference it.)

Think about this: when you call free(ptr), ptr is passed by value, so
free() cannot modify it. I think there's no way of knowing whether a
pointer points to something valid or not, apart from writing one's own
free(), ie something like

void my_free(void **ptr) {
free(*ptr);
*ptr = NULL;
}

int *p = malloc(200);
...
my_free(&p);

but I think it's not advisable.

It's invalid. You're passing an argument of type int** to a function
that expects a void**. There's an implicit conversion from int* to
void*, but not from int** to void**.

A note to the OP: the "[Q]" prefix on your subject is not helpful. We
can tell that it's a question.

 

[bd]

How can one test if a pointer has been "freed" (i.e. with free())?
My naive assumption was that such a pointer would equal NULL, but
not so.

You can't. You need to keep track of it yourself. It's common to set
pointers to NULL manually after freeing them, to act as an indicator.

 

[subnet]

It's invalid. You're passing an argument of type int** to a function
that expects a void**. There's an implicit conversion from int* to
void*, but not from int** to void**.

You are right of course, haste makes waste. I wrongly extended the "*"
behavior to "**".

I think that line should look

my_free((void **)&p);

(correct me if I'm wrong) but it's ugly anyway

Thanks

 

[Joe Wright]

kj wrote:




You can't. You need to keep track of it yourself. It's common to set
pointers to NULL manually after freeing them, to act as an indicator.

Common? I've never done it and I've never seen it done. The only
test-before-use I see is immediately after malloc() to determine
whether it worked or failed. Passing that first test, no code that I
have ever seen tests for NULL before using the pointer.

Not that you couldn't. But it's certainly not common.

 

[Steffen Fiksdal]

Common? I've never done it and I've never seen it done. The only
test-before-use I see is immediately after malloc() to determine
whether it worked or failed. Passing that first test, no code that I
have ever seen tests for NULL before using the pointer.

Not that you couldn't. But it's certainly not common.

I always set my pointers to NULL after freeing the memory, but not as an
indicator.

If some code contains heavy use of malloc() and free() one can end up in a
situation where a pointer freed had been freed earlier (without no
malloc() in between).

Usually this gives a segfault, but it doesn't have to (undefined
behavior).

Let's assume I have made this mistake ( 2*free() on a pointer), and
it works ok for me before a release. This means I have released a
potential segfault program, which I don't wanna do .

So, just to be sure, I always NULL pointers after a free() and always
check for NULL after a malloc(). This means that my second free (which
has nothing in there to do) does absolutely nothing....

I have no idea how common it is, but this gives me more ease at heart
when I release my code....


Best regards
Steffen

 

[Chris Torek]

my_free(&p);

You are right of course, haste makes waste. I wrongly extended the "*"
behavior to "**".

I think that line should look

my_free((void **)&p);

(correct me if I'm wrong) but it's ugly anyway

That shuts up the compiler, but the code remains wrong.

Suppose we replace my_free() with a similar function whose only
job is to set a "double" variable to 0.0:

void clear_it_out(double *dp) {
/* free(dp) - does not make sense */
*dp = 0.0;
}

This is very similar to what we are doing with the "void **" in the
original my_free, which, to reiterate, looks like this:

void my_free(void **vpp) {
free(*vp);
*vp = NULL;
}

But now we have an ordinary "int":

int x;

and at some point, we want to clear it out:

clear_it_out(&x); /* WRONG; draws a diagnostic, given a prototype */

We can shut up the compiler by inserting a cast:

clear_it_out((double *)&x); /* still WRONG, but no diagnostic */

but (from long use of this compiler on this machine) we already
know that an "int" is 4 bytes and a "double" is eight bytes. This
is going to write 8 bytes of 0.0 into the four-byte "int" named
"x". What happens to the other four bytes making up 0.0? The
answer is: they clobber some other, possibly very important, data.

What we *can* do, if we want to clear out "x", is copy it to a
"double", clear out the "double", and then copy the result back:

int x;
double temp;
...
temp = x; /* make a big 8-byte version */
clear_it_out(&temp);/* give clear_it_out 8 bytes to clobber */
x = temp; /* copy the cleared value back */

Likewise, if we want to use my_free() to clear out "p", which has
type "int *" and is thus quite possibly smaller than a "void *"
(perhaps "int *" is 4 bytes and "void *" is eight, just like the
int-and-double situation), we have to copy it to a temporary:

int *p;
void *temp;
...
temp = p; /* make a big 8-byte version */
my_free(&temp); /* give my_free 8 bytes to clobber */
p = temp; /* copy the cleared value back */

Now, at last, the code is completely valid and guaranteed to work
on all implementations, regardless of differences between sizeof(void *)
and sizeof(int *) (ancient PR1MEs), or any change in bit patterns
for byte pointers vs word pointers (ancient Data General Eclipse
machines), or whatever. This is *guaranteed* to work.

There is just one problem. It is also utterly stupid.

We *know* what clear_it_out() does to &temp, and using the sequence:

temp = x;
clear_it_out(&temp);
x = temp;

is just totally bonkers. All we have to do is write:

x = 0; /* well, that was easy */

which is WAY INCREDIBLY EASIER.

Likewise, we *know* what my_free() does to &temp. There is one
extra step my_free() has that clear_it_out() lacks, though, and
that is a call to free(). So we can expand my_free() in-line, by
adding its argument as a local variable and doing assignments:

int *p;
void *temp;
...
temp = p;
{
void **vpp = &temp;
free(*vpp);
*vpp = NULL;
}
p = NULL; /* well, that was slightly easier than "p = temp" */

This time, it may not be immediately obvious what all the vpp
nonsense is about -- but if we note that vpp is set to &temp, and
think about it for half a second, we should realize that *vpp is
just another name for "temp". So we can get rid of *vpp by
substituting "temp":

...
temp = p;
{
void **vpp = &temp;
free(temp);
temp = NULL;
}
p = NULL;

and now it is clear that vpp is not really any use either, so we
can get rid of it too:

...
temp = p;
free(temp);
temp = NULL;
p = NULL;

Now that just leaves one more moment of thought required. What
good is "temp"? In the call to free(), it passes a value of the
correct type ("void *" -- remember, p is "int *"). But we are
allowed to assign any data-pointer type to any variable of type
"void *"; it is this very property that lets us write "temp = p"
in the first place. And of course, any prototyped function call
passes parameters in just the same way as assignment -- so we
do not have to copy "p" to a temp variable just to call free():

temp = p;
free(p); /* instead of free(temp) */
temp = NULL;
p = NULL;

But now "temp" is obviously useless, and we should get rid of it
entirely. This gives:

free(p);
p = NULL;

which is of course what we should have written in the first place.
That dinky little my_free() function was worse than useless -- it
forced us to invent a temp variable, just so that we could copy
the cleared-out value back to the variable we *really* wanted to
clear out. It is just as bad as that stupid "clear_it_out" function
that clears out a double.

If we need to clear out an int -- or even a double -- we can just
do it ourselves. It only takes one line of code.

Likewise, if we need to clear out an int-star -- or even a void-star
-- we can just do it ourselves. It only takes one line of code.
If you dislike the fact that we now have to use two lines to squeeze
in the call to free() as well, just use a comma-expression to cram
it into one line:

free(p), p = NULL;

or even:

#define FREE_VARIANT1(x) (free(x), (void *)NULL)
p = FREE_VARIANT1(p);

or:

#define FREE_VARIANT2(x) (free(x), (x) = NULL)
FREE_VARIANT2(p);

(I dislike the macro versions, in general; just writing the two
statements, or joining them into a simple comma-expression, is
simpler. The macro versions may be simpler in special cases, where
they have names that suggest some useful invariant property of the
code, and/or provide some sort of user-defined abstract type
interface. In this case, the macro(s) probably should not be named
"FREE" anyway.)

 

[subnet]

That shuts up the compiler, but the code remains wrong.
[snip]
That dinky little my_free() function was worse than useless -- it
forced us to invent a temp variable, just so that we could copy
the cleared-out value back to the variable we *really* wanted to
clear out. It is just as bad as that stupid "clear_it_out" function
that clears out a double.

Ok, I was wrong again. But, at least, we agree on the fact that using
an extra function to do such a simple task is ugly and awkward.
Thanks again.

 

[kj]

That dinky little my_free() function was worse than useless...

Too bad! I kinda liked that dinky little my_free(). The reason
is this. Suppose I have defined the following struct as the basic
node in a linked list:

struct elem {
void *payload;
struct elem *next_one;
};

....and I also have

struct linked_list {
struct elem *first;
struct elem *last;
struct elem *next;
int length;
};

Now I want to define a generic destroy_linked_list function, like this:

void destroy_linked_list(struct linked_list **ll,
void (*destroyer_of_payload)(void **)) {
(*ll)->next = (*ll)->first;
while ((*ll)->next) {
struct elem *hold;

hold = (*ll)->next;
(*ll)->next = (*ll)->next->next_one;
_destroy_element(&hold, destroyer_of_payload);
}
free(*ll);
*ll = NULL;
return;
}

void _destroy_element(struct elem **e,
void (*destroyer_of_payload)(void **)) {
if (destroyer_of_payload)
(*destroyer_of_payload)(&((*e)->payload));
free(*e);
*e = NULL;
return;
}

OK, I'm beyond my depth here (which is probably pretty obvious by
now), so I imagine there are already quite a few problems with the
code I've written so far, but what *I* can see as the first problem
with this scheme, in light of Chris's post, is in defining specific
destroyer_of_payload functions, since they must have a void **
argument, but free memory associated with a specific type.

Is there any way to get something like this to work (without
switching to C++ that is)?

TIA,

kj

 

[Joe Wright]

I always set my pointers to NULL after freeing the memory, but not as an
indicator.

If some code contains heavy use of malloc() and free() one can end up in a
situation where a pointer freed had been freed earlier (without no
malloc() in between).

Usually this gives a segfault, but it doesn't have to (undefined
behavior).

Let's assume I have made this mistake ( 2*free() on a pointer), and
it works ok for me before a release. This means I have released a
potential segfault program, which I don't wanna do .

So, just to be sure, I always NULL pointers after a free() and always
check for NULL after a malloc(). This means that my second free (which
has nothing in there to do) does absolutely nothing....

I have no idea how common it is, but this gives me more ease at heart
when I release my code....

If it makes you feel warm and fuzzy, go for it. But I don't think it
will protect you. If your programming technique has you occasionally
freeing a pointer two or more times, it might have you mallocating a
pointer multiple times as well. Little tricks won't fix this.

We (you and I) should develop a technique that ensures a match
between a malloc() and a free().

 

[Andrey Tarasevich]

How can one test if a pointer has been "freed" (i.e. with free())?
My naive assumption was that such a pointer would equal NULL, but
not so.
...

A pointer cannot be "freed". The only thing that can be freed is the
memory the pointer is pointing to. And there's no portable way to test
whether the memory the pointer is pointing to was "freed" or not.

Changing the value of the pointer after using 'free()' won't make any
difference in general case simply because there could be many pointers
pointing to the same memory block.

 

[Andrey Tarasevich]

...
I always set my pointers to NULL after freeing the memory, but not as an
indicator.

If some code contains heavy use of malloc() and free() one can end up in a
situation where a pointer freed had been freed earlier (without no
malloc() in between).

Usually this gives a segfault, but it doesn't have to (undefined
behavior).

For my experience, in 99% of the cases multiple deallocation happens
through different pointers. Setting one of them to NULL won't make any
difference.

Let's assume I have made this mistake ( 2*free() on a pointer), and
it works ok for me before a release. This means I have released a
potential segfault program, which I don't wanna do .

So, just to be sure, I always NULL pointers after a free() and always
check for NULL after a malloc(). This means that my second free (which
has nothing in there to do) does absolutely nothing....

I have no idea how common it is, but this gives me more ease at heart
when I release my code....

In most cases by doing this you'll actually _hide_ a potential segfault
problem, not really prevent it. This means that the problem is still
there, but you just made it more unpredictable and harder to detect.

 

[Gregory Shearman]

If it makes you feel warm and fuzzy, go for it. But I don't think it will
protect you. If your programming technique has you occasionally freeing a
pointer two or more times, it might have you mallocating a pointer
multiple times as well. Little tricks won't fix this.

We (you and I) should develop a technique that ensures a match between a
malloc() and a free().

So, in the event of an error, each error branch must contain a matched
free for every malloc since the program started running? How tiresome and
how impossible to read...

Isn't it better to use a tidy-up routine and free a collection of
pointers if they are indicating they are in use

<snip>

#include <stdlib.h>
#define PCOUNT 15
#define DUMPSIZE 1000

/* An array of generic malloc pointers that your program can use */

void *x[PCOUNT];

/* Routine to free memory from a pointer if it */
/* matches a pointer in the malloc pointer array */
/* else do nothing */

void *gregs_free(void *fptr)
{
int i;

if (fptr == NULL) return;
for (i = 0; i < PCOUNT; i++)
{
if (x == fptr)
{
free(x);
x = NULL;
return;
}
}
}

/* Allocate memory to a free (NULL) pointer slot */
/* in the generic pointer array - returns a void */
/* pointer to the allocated memory. If there are */
/* no free pointers return NULL (increase PCOUNT) */

void *gregs_malloc(size_t size)
{
void *ptr = x[0];
int i;

for (i = 0; i < PCOUNT; i++)
{
if (x == NULL) return malloc(size);
}
return NULL;
}

/* Free any memory in the pointer array if it isn't NULL */

void tidyup()
{
int i;

for (i = 0; i < PCOUNT; i++)
{
if ( x != NULL )
{
free(x);
x = NULL;
}
}
}

int main() {
int i;
int *integer_dump;

/* Init the malloc pointers */

for (i = 0; i < PCOUNT; i++)
{
x = NULL;
}

/* Malloc away to your heart's content using these generic pointers */

integer_dump = (int *)gregs_malloc(DUMPSIZE);
if (integer_dump == NULL)
{
tidyup();
return 1;
}

/* You can free this pointer or just call tidyup() whenever */
/* you want to exit */

gregs_free((void *)integer_dump);

/* ....... */
/* ....... */
/* ....... */



tidyup();
return 0;
}

<snip>

You don't need to worry about matching "frees" with "mallocs" as long as
you don't use malloc() or free() within your program. just call tidyup()
anytime you need to terminate and it will always be graceful.

 

[Michael Wojcik]

/* Allocate memory to a free (NULL) pointer slot */
/* in the generic pointer array - returns a void */
/* pointer to the allocated memory. If there are */
/* no free pointers return NULL (increase PCOUNT) */

void *gregs_malloc(size_t size)
{
void *ptr = x[0];
int i;

for (i = 0; i < PCOUNT; i++)
{
if (x == NULL) return malloc(size);
}
return NULL;
}

This would probably be better if it assigned some value to x
when malloc succeeds. (And "ptr" is extraneous.) On the other
hand, a program that uses gregs_malloc and gregs_free exclusively
will indeed avoid duplicate free.

While I wouldn't implement it this way, I agree that tracking
allocations can be a useful technique for avoiding a significant
class of errors, for programs that can tolerate the performance
and storage cost - which is often the case.

--
Michael Wojcik (e-mail address removed)

She felt increasingly (vision or nightmare?) that, though people are
important, the relations between them are not, and that in particular
too much fuss has been made over marriage; centuries of carnal
embracement, yet man is no nearer to understanding man. -- E M Forster

 

[Chris Torek]

Too bad! I kinda liked that dinky little my_free().

Some people do, yes. (I think, for the most part, they are
better off using some other language -- for instance, despite all
C++'s faults, this kind of thing works well there, using templates
to expand the code in-line.)

[Long quote, because we need all this code:]

The reason is this. Suppose I have defined the following struct
as the basic node in a linked list:

struct elem {
void *payload;
struct elem *next_one;
};

...and I also have

struct linked_list {
struct elem *first;
struct elem *last;
struct elem *next;
int length;
};

[I have no idea what the "next" field is for, here.]

Now I want to define a generic destroy_linked_list function, like this:

void destroy_linked_list(struct linked_list **ll,
void (*destroyer_of_payload)(void **)) {
(*ll)->next = (*ll)->first;
while ((*ll)->next) {
struct elem *hold;

hold = (*ll)->next;
(*ll)->next = (*ll)->next->next_one;
_destroy_element(&hold, destroyer_of_payload);
}
free(*ll);
*ll = NULL;
return;
}

void _destroy_element(struct elem **e,
void (*destroyer_of_payload)(void **)) {
if (destroyer_of_payload)
(*destroyer_of_payload)(&((*e)->payload));
free(*e);
*e = NULL;
return;
}

OK, I'm beyond my depth here (which is probably pretty obvious by
now), so I imagine there are already quite a few problems with the
code I've written so far, but what *I* can see as the first problem
with this scheme, in light of Chris's post, is in defining specific
destroyer_of_payload functions, since they must have a void **
argument, but free memory associated with a specific type.

Assuming you really want to do it this way, you can simply write
your freeing function as, e.g.:

void free_xyzzy(void **vpp) {
void *p = *vpp; /* save the "void *" */

*vpp = NULL; /* mark it as freed */
free(p); /* and release the payload */
}

or, equivalently in this case:

void free_plugh(void **vpp) {
free(*vpp);
*vpp = NULL;
}

But if the only kind of payload-destroyer you will ever use always
calls free(), why not simplify destroy_linked_list() thus:

void destroy_linked_list(struct linked_list **lpp) {
struct linked_list *lp;
struct elem *ep, *next;

lp = *lpp;
for (ep = lp->first; ep != NULL; ep = next) {
next = ep->next_one;
free(ep->payload);
free(ep);
}
free(lp);
*lpp = NULL;
}

? Note that there is no need to set ep->payload to NULL here
because the only use of "*ep" is -- or at least, had better be --
the one in the list, which we are about to destroy by free(ep).
That is, the object to which ep points, which contains both the
"next_one" pointer and the "payload" pointer, is inherently a
single-use entity, and *we* have the single use, and we are about
to discard it. We know for sure that no one else can (correctly)
be using either ep->next_one or ep->payload.

Now, if you want to allow certain lists not to call free() on their
payload parameter, just add that "free-it function" argument back:

/* this line did not fit, so I broke it up "logically" */
void destroy_linked_list(
struct linked_list **lpp,
void (*release)(void *)
) {
struct linked_list *lp;
struct elem *ep, *next;

if (release == NULL)
release = free;
lp = *lpp;
for (ep = lp->first; ep != NULL; ep = next) {
next = ep->next_one;
release(ep->payload); /* aka (*release)(ep->payload) */
free(ep);
}
free(lp);
*lpp = NULL;
}

There is *still* no need to set ep->payload to NULL, because the
"struct elem" object at *ep is still "owned" by this linked-list
module, and is still about to be destroyed via free(ep).

Of course, if the release() function might follow the list at *lpp,
*then* you have some reason for concern. But this list is in the
process of being destroyed; this sort of code-organization is not
something I consider particularly sound. We could arrange for *lpp
to get set to NULL much earlier, after copying it to "lp":

lp = *lpp;
*lpp = NULL;

so that the entire list is now "off limits". Whether or not this
is acceptable is another question entirely.

Is there any way to get something like this to work (without
switching to C++ that is)?

The various BSDs have a header file, <sys/queue.h>, that provides
type-safe list and queue macros that avoid using extra space for
the lists and queues. (The C++ equivalent with templates is at
least less klunky to use, though.)

 

[CBFalconer]

.... snip ...

[Long quote, because we need all this code:]

The reason is this. Suppose I have defined the following struct
as the basic node in a linked list:

struct elem {
void *payload;
struct elem *next_one;
};

...and I also have

struct linked_list {
struct elem *first;
struct elem *last;
struct elem *next;
int length;
};

[I have no idea what the "next" field is for, here.]

Now I want to define a generic destroy_linked_list function, like this:

void destroy_linked_list(struct linked_list **ll,
void (*destroyer_of_payload)(void **)) {
(*ll)->next = (*ll)->first;
while ((*ll)->next) {
struct elem *hold;

hold = (*ll)->next;
(*ll)->next = (*ll)->next->next_one;
_destroy_element(&hold, destroyer_of_payload);
}
free(*ll);
*ll = NULL;
return;
}

void _destroy_element(struct elem **e,
void (*destroyer_of_payload)(void **)) {
if (destroyer_of_payload)
(*destroyer_of_payload)(&((*e)->payload));
free(*e);
*e = NULL;
return;
}

OK, I'm beyond my depth here (which is probably pretty obvious by
now), so I imagine there are already quite a few problems with the
code I've written so far, but what *I* can see as the first problem
with this scheme, in light of Chris's post, is in defining specific
destroyer_of_payload functions, since they must have a void **
argument, but free memory associated with a specific type.

The originals never showed up here, so I am only addressing the use
of a void* in the generic destroyer_of_payload.

The point is that the generic list handling code only needs to know
that a payload needs destroying at some time, and what to call to
do it. Apart from that it never needs to know anything about the
payload proper (assuming the calling code handles it properly).
Thus is uses a void* pointer.

When destroyer_of_payload is actually designed, it needs to know
what a payload is. Thus it needs to convert the void* (not a
void**) to the appropriate type. Until then the list system can
pass around the void* freely, not caring one smelly hoot what it
really points to. So something like:

void destroyer_of_payload(void *payload)
{
payload_t *pptr = payload;

/* code that uses pptr to do its thing */
}

allows the generic list handling code to do list things without
caring what is in the list.

You can see examples of this sort of thing in my hashlib system,
available on the download section of my page, below.