alloc_disk in linux-2.5.45/drivers/block/genhd.c calls
rand_initialize_disk, which can set disk->random to NULL on kmalloc
failure. disk_release does a kfree(disk->random) without checking.
I could add an "if(disk->random == NULL)" to disk_release,
memory allocation policy for disk->random does not need to be split
between genhd.c and and random.c, and disk->random is so small that
there is really no benefit to kmalloc'ing it.
More importantly, I want to reduce memory allocation error
branches (or, worse, lack of branches where they are needed as this
case as this case illustrates), especially because I'd eventually like
to have a variant of alloc_disk that doesn't do the kmalloc, so I can
embed the struct gendisk and have a gendisk initializer that doesn't
need an error branch (init_disk(disk, partitions_array,...), but
that's another matter).
So, here is a patch to embed disk->random. It is a net
deletion of ten lines. It also moves the declarations for
add_disk_randomness and rand_initialize_disk to <linux/random.h> where
the other add_foo_randomness and rand_initialize_foo routines are
declared.
--
Adam J. Richter __ ______________ 575 Oroville Road
[email protected] \ / Milpitas, California 95035
+1 408 309-6081 | g g d r a s i l United States of America
"Free Software For The Rest Of Us."
On Mon, 4 Nov 2002, Adam J. Richter wrote:
> alloc_disk in linux-2.5.45/drivers/block/genhd.c calls
> rand_initialize_disk, which can set disk->random to NULL on kmalloc
> failure. disk_release does a kfree(disk->random) without checking.
kfree(NULL) is guaranteed to be OK and is a very common idiom. Talk
to Linus about that - it's a religious issue and FWIW I'm on the same
side ("free(NULL) should be allowed"). IIRC, Linus had claimed quite
a few times that change of kfree() that would break kfree(NULL) would
be considered as a bug.
Al Viro wrote:
>On Mon, 4 Nov 2002, Adam J. Richter wrote:
>> alloc_disk in linux-2.5.45/drivers/block/genhd.c calls
>> rand_initialize_disk, which can set disk->random to NULL on kmalloc
>> failure. disk_release does a kfree(disk->random) without checking.
>kfree(NULL) is guaranteed to be OK and is a very common idiom. Talk
>to Linus about that - it's a religious issue and FWIW I'm on the same
>side ("free(NULL) should be allowed"). IIRC, Linus had claimed quite
>a few times that change of kfree() that would break kfree(NULL) would
>be considered as a bug.
Oops, sorry for not checking that possibility before posting.
I'd still like to see this patch integrated as a clean-up (it
does delete a few lines and should shrink vmlinux by a few bytes), but
it can certainly wait until post-crunch.
Adam J. Richter __ ______________ 575 Oroville Road
[email protected] \ / Milpitas, California 95035
+1 408 309-6081 | g g d r a s i l United States of America
"Free Software For The Rest Of Us."