Re: [PATCH] [DISCUSS] Make the variable NULL after freeing it.

[Ingo Oeser]

Hi,

On Monday, 1. January 2007 07:37, Amit Choudhary wrote:
> --- Ingo Oeser <ioe-lkml@xxxxxxxxxx> wrote:
> > #define kfree_nullify(x) do { \
> > 	if (__builtin_constant_p(x)) { \
> > 		kfree(x); \
> > 	} else { \
> > 		typeof(x) *__addr_x = &x; \

Ok, I should change that line to 
		typeof(x) *__addr_x = &(x); \

> > 		kfree(*__addr_x); \
> > 		*__addr_x = NULL; \
> > 	} \
> > } while (0)
> > 
> > Regards
> > 
> > Ingo Oeser
> > 
> 
> This is a nice approach but what if someone does kfree_nullify(x+20).

Then this works, because the side effect (+20) is evaluated only once. 
AFAIK __builtin_constant_p() and typeof() are both free of side effects.

 
> I decided to keep it simple. If someone is calling kfree_nullify() with anything other than a
> simple variable, then they should call kfree().

kfree_nullify() has to replace kfree() to be of any use one day. So this is not an option.

Anybody thinking of "Hey, this must be NULL afterwards!", will set it to NULL himself.
Anybody else doesn't know or care about it, which is the case we like to catch.

> But definitely an approach that takes care of all 
> situations is the best but I cannot think of a macro that can handle all situations. The simple
> macro that I sent earlier will catch all the other usage at compile time. 

The problems I see are:
1. parameter to kfree is a value not a pointer 
    -> solved by using a macro instead of function, 
         but generate new (the other) problems
    -> take the address of the value there.
2. possible side effects of macro parameter usage 
   -> solved by assigning once only and using typeof
3. Constants don't have an address 
   -> need to check for constant

So apart from missing braces before taking the address, I don't see
any problem with my solution :-)

Should I send a patch?

> Please let me know if I have missed something.

I reviewed it and you missed side effects (kfree(x); x = NULL).

Regards

Ingo Oeser
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/