Re: [PATCH] include/linux/slab.h: new KFREE() macro.

From: Vadim Lobanov
Date: Sun Jan 07 2007 - 18:23:44 EST


On Sun, 2007-01-07 at 14:43 -0800, Amit Choudhary wrote:
> Any strong reason why not? x has some value that does not make sense and can create only problems.
> And as I explained, it can result in longer code too. So, why keep this value around. Why not
> re-initialize it to NULL.

Because it looks really STRANGE(tm). Consider the following function,
which is essentially what you're proposing in macro-ized form:
void foobar(void)
{
void *ptr;

ptr = kmalloc(...);
// actual work here
kfree(ptr);
ptr = NULL;
}
Reading code like that makes me say "wtf?", simply because 'ptr' is not
used thereafter, so setting it to NULL is both pointless and confusing
(it looks out-of-place, and therefore makes me wonder if there's
something stupidly tricky going on).

Also, arguably, your demonstration of why the lack of the proposed
KFREE() macro results in longer code is invalid. Whereas you wrote:
pointer *arr_x[size_x];
pointer *arr_y[size_y];
pointer *arr_z[size_z];
That really should have been:
pointer *arr[size_x + size_y + size_z];
or:
pointer **arr[3] = { arr_x, arr_y, arr_z };
In which case, the you only need one path in the function to handle
allocation failures, rather than the three that you were arguing for.

> If x should not be re-initialized to NULL, then by the same logic, we should not even initialize
> local variables. And all of us know that local variables should be initialized.

That's some strange and confused logic then. Here's my stab at the same
logical premise: "Using uninitialized values is bad." Notice how that,
in and of itself, makes no statements regarding freed pointers, since
the intent is not to use them after they've been freed anyway.

-- Vadim Lobanov


-
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/