Hi,
On Monday, 1. January 2007 07:37, Amit Choudhary wrote:
> --- Ingo Oeser <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
