free()

[Christopher Benson-Manica]

Richard Bos wrote:

Can you cite an example where setting a pointer to NULL could mask a
bug? The way I see it is there are three things you can do with a
pointer, test it, dereference it and free it. Once set to NULL, the
first to will fail and the third is harmless.

You missed Richard's point. The problem lies not in setting the
pointer to NULL, but in *believing* one has when in fact one has not.

 

[Richard Heathfield]

Christopher Benson-Manica said:

You missed Richard's point. The problem lies not in setting the
pointer to NULL, but in *believing* one has when in fact one has not.

Lots of problems can arise from believing one has done something when in
fact one has not. That doesn't mean that the something is a bad thing
to do.

Short of writing an entire pointer management subsystem, there isn't
much you can do with a pointer to check it for validity, but you *can*
test it against NULL. If you know it's NULL, you know it's invalid. If
it isn't NULL, you can't be sure either way. So setting it to NULL when
it would otherwise be indeterminate is a Good Thing, because it
increases the amount of information available to you at a trivial cost.

 

[CBFalconer]

.... snip ...

I argue that attempting to ensure that pointers are either null or
have legally deferencible values is better. A null pointer is by
definition an unusable pointer. A pointer with an indeterminate
value, on the other hand, may or may not exhibit observable error,
when misused. I prefer to take the more deterministic route.

The point is that the attempt will involve some sort of operation
on the pointer, comparison, masking, whatever. For an invalid
pointer that operation itself will involve undefined behaviour.
Remember, we are talking portable standard C, not system dependent
kluges. In fact:

p = malloc(10);
free(p);
if (p) .... /* involves undefined behaviour */

--
<http://www.cs.auckland.ac.nz/~pgut001/pubs/vista_cost.txt>
<http://www.securityfocus.com/columnists/423>

"A man who is right every time is not likely to do very much."
-- Francis Crick, co-discover of DNA
"There is nothing more amazing than stupidity in action."
-- Thomas Matthews

 

[Richard Heathfield]

CBFalconer said:

The point is that the attempt will involve some sort of operation
on the pointer, comparison, masking, whatever. For an invalid
pointer that operation itself will involve undefined behaviour.
Remember, we are talking portable standard C, not system dependent
kluges. In fact:

p = malloc(10);
free(p);
if (p) .... /* involves undefined behaviour */

Yes, but santosh knows that already. He is saying he prefers:

p = malloc(10);
if(p != NULL)
{
use(p);
free(p);
p = NULL;
}
/* [1] */
if(p != NULL) /* doesn't involve undefined behaviour */

Of course, this check is rather pointless as it stands in this trivial
example, but if you add code at [1] which may change p's value, it
becomes less silly.

 

[Racaille]

Racaille said:


It is not sophistry, I am being honest, I don't see what my use of
English has to do with this, and I'm not pushing anything - so I'm
afraid you're 100% wrong there.

You're pushing another 'rule of thumb' (every time set a pointer to
NULL
after free()ing it, just do it mechanically, even if it is NOT
required by
the logic of the program).

That is horrible - not only because is yet another 'simple rule'/cargo
cult
thing, but also because it's actively obfuscating serious problems,
like
freeing the same pointer multiple times.

Feel free to be happy about people taking your advice - They won't ask
you to
debug their programs, after all

 

[Ian Collins]

That is horrible - not only because is yet another 'simple rule'/cargo
cult
thing, but also because it's actively obfuscating serious problems,
like
freeing the same pointer multiple times.

If a pointer is null, you can free it as often as you like. Try to
dereference it and you'll soon know what's wrong. Dereferencing or
freeing an invalid pointer has undefined and indeterminate results.

 

[CBFalconer]

CBFalconer said:

Yes, but santosh knows that already. He is saying he prefers:

....

What santosh knows is immaterial here. What he said is material.
The idea is to clear up misconceptions that may be drawn by nubile
nymphlike newbies.

--
<http://www.cs.auckland.ac.nz/~pgut001/pubs/vista_cost.txt>
<http://www.securityfocus.com/columnists/423>

"A man who is right every time is not likely to do very much."
-- Francis Crick, co-discover of DNA
"There is nothing more amazing than stupidity in action."
-- Thomas Matthews

 

[Richard Heathfield]

Racaille said:

You're pushing another 'rule of thumb' (every time set a pointer to
NULL after free()ing it, just do it mechanically, even if it is NOT
required by the logic of the program).

No, I'm not pushing it at all. I'm expressing my opinion that it's a
very good idea, but I have certainly not claimed that not doing it is a
very bad idea. People make up their own minds.

That is horrible - not only because is yet another 'simple rule'/cargo
cult thing, but also because it's actively obfuscating serious
problems, like freeing the same pointer multiple times.

I don't understand why anyone would want to free the same pointer
multiple times, so I don't see why it's an issue.

Feel free to be happy about people taking your advice

I was offering my opinion, not my advice.

- They won't ask you to debug their programs, after all

You clearly haven't been here very long. People *do* ask me to debug
their programs. I find that task much easier when the bugs stand still
instead of hopping around all over memory.

 

[Richard Heathfield]

Ian Collins said:

If a pointer is null, you can free it as often as you like.

Zero, for preference. Why would you bother to pass NULL to free()?

Try to dereference it and you'll soon know what's wrong.

Quite so.

Dereferencing or
freeing an invalid pointer has undefined and indeterminate results.

....which is why indeterminate pointers are a bad idea - you can't tell
whether or not they are invalid.

 

[Richard Heathfield]

CBFalconer said:

Richard Heathfield wrote:

...

What santosh knows is immaterial here. What he said is material.

It is from what he said that I deduced what he knows.

 

[Chris Dollin]

Ian Collins said:


Zero, for preference. Why would you bother to pass NULL to free()?

You wouldn't (except perhaps for testing purposes).

But you might very well pass a null pointer to `free`.

struct stuff { int n; Other *other; };

struct stuff* newStuff() { ... result->other = 0; ... }

... code possibly setting `other` to a non-null pointer ...

finally

void freeStuff( struct stuff *stuff )
{
free( stuff->other );
free( stuff );
}

Of course you might argue that you could, maybe should, write

if (stuff) free( stuff );

but (a) why should you, since `free` is defined to work on
null pointers, and (b) if you write the sequence `if (x) free(x);`
several times, DRY says "make that a function!". At which point
you realise you needn't have bothered, because Mr Standard
was there first.

 

[raxitsheth2000]

dbansal said:



Drop the pointless obfuscatory cast.


And that one.


free(ptr);
ptr = NULL;

This is Interesting..! I am just putting small code to check some
stuff.


/* Start test1.c */

#include<stdio.h>
#include<string.h>
#include<malloc.h>
int main()
{

char *p;
char*q;

p=(char *) malloc(100);

/* I should do memset but ... */

strcpy(p,"raxit sheth");

q=p;

printf("\nP=%p : %s",p,p);
printf("\nQ=%p :%s",q,q);
fflush(stdout);
free(p);
/* p=NULL; */

printf("\n AFTER FREE ");
printf("\nP=%p : %s",p,p);
printf("\nQ=%p : %s",q,q);
return 0;

}
/* end */

output on linux box (valgrind shows double free error)
$gcc -g3 -Wall test1.c -o test1
$./test1


P=0x8049800 : raxit sheth
Q=0x8049800 :raxit sheth
AFTER FREE
P=0x8049800 : raxit sheth
Q=0x8049800 : raxit sheth


Now Uncommented p=NULL in above code. Output is

P=0x8049810 : raxit sheth
Q=0x8049810 :raxit sheth
AFTER FREE
P=(nil) : (null)
Q=0x8049810 : raxit sheth <----important


I think (correct me if I am wrong, I am learning here.)

1.this technique is useful in some case but not in all where pointer
assignment is doing like this way. q is pointing to same memory
location as of p before free, and doing p=NULL is not helpful for q.


2. normally sequence,

p=NULL;
malloc,
memset,bzero or equivalent
use of memory
free
May be p=NULL.

can be

p=NULL;
malloc,
memset,bzero or equivalent
use of memory
memset,<---Some practical limitation. I may not be knowing what
size to free and howmany bytes to memset.
free
(p=NULL ?)


any opinion on this ? Adv/Disadv/limitation/ ?


--Raxit Sheth

 

[Richard Heathfield]

(e-mail address removed) said:

/* Start test1.c */

#include<stdio.h>
#include<string.h>
#include<malloc.h>

No such header in C. For malloc()'s prototype, use this instead:

int main()
{

char *p;
char*q;

p=(char *) malloc(100);

The cast is unnecessary, as I have already explained to you.

/* I should do memset but ... */

No, you should check that the allocation succeeded before relying on it.

if(p != NULL)
{

strcpy(p,"raxit sheth");

That's fine.

q=p;

That's legal.

printf("\nP=%p : %s",p,p);

printf("P=%p : %s\n", (void *)p, p);

printf("\nQ=%p :%s",q,q);

printf("Q=%p : %s\n", (void *)q, q);

fflush(stdout);
free(p);

That's fine. The value that p and q shared is now indeterminate, so you
won't be able to evaluate p or q again until they've been given new
values.

/* p=NULL; */

Why comment that out?

printf("\n AFTER FREE ");
printf("\nP=%p : %s",p,p);

That's illegal, because you are using the value of an indeterminate
pointer.

printf("\nQ=%p : %s",q,q);
Likewise.

return 0;

}
/* end */

output on linux box

....is irrelevant, because you broke the rules of C, so any output you
get is not bound by those rules.

Now Uncommented p=NULL in above code.

It doesn't matter. The program remains broken because you are evaluating
p and q.

I think (correct me if I am wrong, I am learning here.)

1.this technique is useful in some case but not in all where pointer
assignment is doing like this way. q is pointing to same memory
location as of p before free, and doing p=NULL is not helpful for q.

If you give q a value which later becomes indeterminate, it's your job
to make sure you don't evaluate q.

any opinion on this ? Adv/Disadv/limitation/ ?

I recommend that you study the rules of C carefully before you decide on
a programming strategy for memory management.

 

[raxitsheth2000]

(e-mail address removed) said:




No such header in C. For malloc()'s prototype, use this instead:

#include <stdlib.h> agree



The cast is unnecessary, as I have already explained to you.

agree, but still its hard to unlearn the stuff learnt from book,
"malloc returns void * and you need to cast it"

No, you should check that the allocation succeeded before relying on it.

if(p != NULL)
{ agree,

That's fine.




That's legal.


printf("P=%p : %s\n", (void *)p, p);


printf("Q=%p : %s\n", (void *)q, q);


That's fine. The value that p and q shared is now indeterminate, so you
won't be able to evaluate p or q again until they've been given new
values.


Why comment that out?

both run commented and uncommented, belows are output of both.

That's illegal, because you are using the value of an indeterminate
pointer.

Yes, I Agree, its illegal.

But here we are trying to find out some stuff on what happen/can be
done to avoid illegal access after freeing the memory,

so i think its 'Known Illegal', and that is the actual point i want
to make, ( or any good example which is legal and still show simillar
stuff. ?)

I may want to learn if we use

free(p);p=NULL;

kind of stuff, is it anyhow helpful in avoiding this type of Illegal
stuff ?

...is irrelevant, because you broke the rules of C, so any output you
get is not bound by those rules.

we are discussing how to avoid of breaking C Rules ( or better what
may be outcome of accessing freed memory) and to explain this i need
to break the rules.(Correct me if i done wrong. or better example.)

It doesn't matter. The program remains broken because you are evaluating

p and q.

I know (and agree), code is still broken.

If you give q a value which later becomes indeterminate, it's your job
to make sure you don't evaluate q.

Yes, this is correct if programmer is very very very careful and Dont
work under difficult deadlines.

I recommend that you study the rules of C carefully before you decide on
a programming strategy for memory management.

can you recommend good resource ?

--Raxit Sheth

 

[Beej]

Short of writing an entire pointer management subsystem, there isn't
much you can do with a pointer to check it for validity, but you *can*
test it against NULL. If you know it's NULL, you know it's invalid. If
it isn't NULL, you can't be sure either way. So setting it to NULL when
it would otherwise be indeterminate is a Good Thing, because it
increases the amount of information available to you at a trivial cost.

At last... this thread is finally starting to boil down to "NULL makes
a great sentinel value."

-Beej

void delete_goat(GOAT *g)
{
free(g);
g = NULL;
}

 

[Chris Dollin]

At last... this thread is finally starting to boil down to "NULL makes
a great sentinel value."

-Beej

void delete_goat(GOAT *g)
{
free(g);
g = NULL;
}

This being a place where the assignment of NULL is completely
pointless.

 

[Richard Heathfield]

(e-mail address removed) said:

agree, but still its hard to unlearn the stuff learnt from book,
"malloc returns void * and you need to cast it"

It's because malloc returns void * that you _don't_ need to cast it!

Yes, I Agree, its illegal.

But here we are trying to find out some stuff on what happen/can be
done to avoid illegal access after freeing the memory,

Writing programs whose output is undefined is not a good way to find
stuff out.

I may want to learn if we use

free(p);p=NULL;

kind of stuff, is it anyhow helpful in avoiding this type of Illegal
stuff ?

I find it helpful to ensure that any object pointer *either* points to a
valid object *or* has a value of NULL. If I do this, then I can be
assured that if(p != NULL) { p points to a valid object } but the price
of this assurance is that I must set p to NULL if it no longer points
to a valid object.

we are discussing how to avoid of breaking C Rules ( or better what
may be outcome of accessing freed memory) and to explain this i need
to break the rules.(Correct me if i done wrong. or better example.)

You're wrong. Breaking the rules teaches you nothing about the language,
because by breaking the rules you are giving the implementation the
freedom to provide any behaviour whatsoever, or even no behaviour
whatsoever if it prefers.

Yes, this is correct if programmer is very very very careful and Dont
work under difficult deadlines.

Yes, programmers do have to be very very very careful. Because deadlines
are often unrealistic, mistakes happen. With experience, you will learn
how to identify these mistakes quickly.

can you recommend good resource ?

ISO/IEC 9899.

 

[Richard Heathfield]

Chris Dollin said:

This being a place where the assignment of NULL is completely
pointless.

....but then it's a pointless function anyway.

I write destructors like this:

void goat_delete(GOAT **g)
{
if(g != NULL)
{
if(*g != NULL)
{
GOAT *p = *g; /* purely for notational convenience */
horn_delete(&p->horn);
trollgun_delete(&p->trollgun);
leg_delete(&p->leg);
free(p);

*g = NULL; /* *not* a pointless assignment */
}
}
}

 

[Chris Dollin]

Chris Dollin said:


...but then it's a pointless function anyway.

It frees the store its argument points to and names what's
going on, so the /function/ isn't pointless.

I write destructors like this:

void goat_delete(GOAT **g)
{
if(g != NULL)
{
if(*g != NULL)
{
GOAT *p = *g; /* purely for notational convenience */
horn_delete(&p->horn);
trollgun_delete(&p->trollgun);
leg_delete(&p->leg);
free(p);

*g = NULL; /* *not* a pointless assignment */
}
}
}

Interesting. I'm trying to articulate why I find that overkill.
I /think/ it's that where I free something, either the place
I'm freeing it from is about to evaporate (as in your horn,
leg, and trollgun), so the assignment doesn't help, or I'm
about to assign a new value to that place, ditto. Probably because
we're writing different kinds of code. With a shared goal:
"don't let invalid pointers screw you up - get rid of them PDQ".

 

[Richard Heathfield]

[some re-flow]
Chris Dollin said:

Chris Dollin said:

Beej wrote:
void delete_goat(GOAT *g) { free(g); g = NULL; }

[...] where the assignment of NULL is completely pointless.

...but then it's a pointless function anyway.

It frees the store its argument points to and names what's
going on, so the /function/ isn't pointless.

free(whatever_you_would_have_passed_to_delete_goat); would do the same
trick, though - it frees the memory and (if the pointer is well-named)
documents what's going on as well, to anyone who knows what free() is
for - and if they don't know, why are we letting them touch production
code?

Interesting. I'm trying to articulate why I find that overkill.
I /think/ it's that where I free something, either the place
I'm freeing it from is about to evaporate (as in your horn,
leg, and trollgun), so the assignment doesn't help,

It might not only be goats that use trollguns, and anyway there may be
other circumstances where you need to destroy a trollgun other than the
one where you are destroying - sorry! deleting - a goat.

[...] Probably because
we're writing different kinds of code. With a shared goal:
"don't let invalid pointers screw you up - get rid of them PDQ".

Yeah. The thing is, different people think in different ways. There's
more than one way to do it, as the Perl people are so fond of reminding
us - and what seems natural and elegant to one person may well seem
kludge-ugly to another. There are lots of right ways. (And vastly more
wrong ways...)