[Patch 00/16] Remove unnecessary kmalloc casts

[Patch 00/16] Remove unnecessary kmalloc casts

[jack]

This patch series removes unnecessary kmalloc casts from various portions
of the kernel.

Jack Stone

-- 

[Patch 01/16] Remove unnecessary kmalloc casts from the cris arch.

[jack]

[-- Attachment #1: arch_cris_remove_kmalloc_casts.patch --]
[-- Type: text/plain, Size: 750 bytes --]

Signed-off-by: Jack Stone <jack@hawkeye.stone.uk.eu.org>

CC: Mikael Starvik <starvik@axis.com>

---
 arch/cris/arch-v32/mm/intmem.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: b/arch/cris/arch-v32/mm/intmem.c
===================================================================
--- a/arch/cris/arch-v32/mm/intmem.c
+++ b/arch/cris/arch-v32/mm/intmem.c
@@ -28,7 +28,7 @@ static void crisv32_intmem_init(void)
 	static int initiated = 0;
 	if (!initiated) {
 		struct intmem_allocation* alloc =
-		  (struct intmem_allocation*)kmalloc(sizeof *alloc, GFP_KERNEL);
+		  kmalloc(sizeof *alloc, GFP_KERNEL);
 		INIT_LIST_HEAD(&intmem_allocations);
 		intmem_virtual = ioremap(MEM_INTMEM_START, MEM_INTMEM_SIZE);
 		initiated = 1;

-- 

Re: [Patch 01/16] Remove unnecessary kmalloc casts from the cris arch.

[Stefan Richter]

jack@hawkeye.stone.uk.eu.org wrote:
> --- a/arch/cris/arch-v32/mm/intmem.c
> +++ b/arch/cris/arch-v32/mm/intmem.c
> @@ -28,7 +28,7 @@ static void crisv32_intmem_init(void)
>  	static int initiated = 0;
>  	if (!initiated) {
>  		struct intmem_allocation* alloc =
> -		  (struct intmem_allocation*)kmalloc(sizeof *alloc, GFP_KERNEL);
> +				kmalloc(sizeof *alloc, GFP_KERNEL);
>  		INIT_LIST_HEAD(&intmem_allocations);
>  		intmem_virtual = ioremap(MEM_INTMEM_START, MEM_INTMEM_SIZE);
>  		initiated = 1;
> 

Would be nice if you increased the indentation of the kmalloc line for
better readability, now that the expression is shorter.

In the rest of arch/cris/arch-v32/mm/intmem.c, one tab is typically used
for such continuation indentations.  However, intmem.c is not a prime
example for good whitespace usage, so you could as well use two tabs.

Also note that there are two more kmallocs with unnecessary cast in the
same file, at least in 2.6.23-rc1.  Would be nice if you cleaned up the
whole file at once.

There is also a shadowing of a local variable (tmp) by another local
variable of the same name.  Maybe you could submit another cleanup patch
for that too while you are at it?
-- 
Stefan Richter
-=====-=-=== =--- ----=
http://arcgraph.de/sr/

[Patch 02/16] Remove unnecessary kmalloc casts from the mips arch.

[jack]

[-- Attachment #1: arch_mips_remove_kmalloc_casts.patch --]
[-- Type: text/plain, Size: 1101 bytes --]

Signed-off-by: Jack Stone <jack@hawkeye.stone.uk.eu.org>

CC: Ralf Baechle <ralf@linux-mips.org>

---
 arch/mips/au1000/common/dbdma.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: b/arch/mips/au1000/common/dbdma.c
===================================================================
--- a/arch/mips/au1000/common/dbdma.c
+++ b/arch/mips/au1000/common/dbdma.c
@@ -396,7 +396,7 @@ au1xxx_dbdma_ring_alloc(u32 chanid, int 
 	 * and if we try that first we are likely to not waste larger
 	 * slabs of memory.
 	 */
-	desc_base = (u32)kmalloc(entries * sizeof(au1x_ddma_desc_t),
+	desc_base = kmalloc(entries * sizeof(au1x_ddma_desc_t),
 			GFP_KERNEL|GFP_DMA);
 	if (desc_base == 0)
 		return 0;
@@ -408,7 +408,7 @@ au1xxx_dbdma_ring_alloc(u32 chanid, int 
 		kfree((const void *)desc_base);
 		i = entries * sizeof(au1x_ddma_desc_t);
 		i += (sizeof(au1x_ddma_desc_t) - 1);
-		if ((desc_base = (u32)kmalloc(i, GFP_KERNEL|GFP_DMA)) == 0)
+		if ((desc_base = kmalloc(i, GFP_KERNEL|GFP_DMA)) == 0)
 			return 0;
 
 		desc_base = ALIGN_ADDR(desc_base, sizeof(au1x_ddma_desc_t));

-- 

Re: [Patch 02/16] Remove unnecessary kmalloc casts from the mips arch.

[Christian Borntraeger]

Am Dienstag, 31. Juli 2007 schrieb jack@hawkeye.stone.uk.eu.org:
> ===================================================================
> --- a/arch/mips/au1000/common/dbdma.c
> +++ b/arch/mips/au1000/common/dbdma.c
[...]
> -	desc_base = (u32)kmalloc(entries * sizeof(au1x_ddma_desc_t),
> +	desc_base = kmalloc(entries * sizeof(au1x_ddma_desc_t),
[...]
> -		if ((desc_base = (u32)kmalloc(i, GFP_KERNEL|GFP_DMA)) == 0)
> +		if ((desc_base = kmalloc(i, GFP_KERNEL|GFP_DMA)) == 0)

See the comments to the other patches. The cast is necessary.

Re: [Patch 02/16] Remove unnecessary kmalloc casts from the mips arch.

[Rene Herman]

On 07/31/2007 02:53 PM, jack@hawkeye.stone.uk.eu.org wrote:

> -	desc_base = (u32)kmalloc(entries * sizeof(au1x_ddma_desc_t),
> +	desc_base = kmalloc(entries * sizeof(au1x_ddma_desc_t),

The unnessary bit in kmalloc casts is due to void pointers being assignment 
compatible to other pointer types but in these patches you are also removing 
casts to unsigned long's and they should stay.

I noticed the same in patch 2, 4, 6, 8, 9 and 11 (cursory glance...)

Rene.

[Patch 03/16] Remove unnecessary kmalloc casts from the powerpc arch.

[jack]

[-- Attachment #1: arch_powerpc_remove_kmalloc_casts.patch --]
[-- Type: text/plain, Size: 627 bytes --]

Signed-off-by: Jack Stone <jack@hawkeye.stone.uk.eu.org>

CC: Paul Mackerras <paulus@samba.org>

---
 arch/powerpc/kernel/ibmebus.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: b/arch/powerpc/kernel/ibmebus.c
===================================================================
--- a/arch/powerpc/kernel/ibmebus.c
+++ b/arch/powerpc/kernel/ibmebus.c
@@ -383,7 +383,7 @@ static int ibmebus_match_path(struct dev
 
 static char *ibmebus_chomp(const char *in, size_t count)
 {
-	char *out = (char*)kmalloc(count + 1, GFP_KERNEL);
+	char *out = kmalloc(count + 1, GFP_KERNEL);
 	if (!out)
 		return NULL;
 

-- 

[Patch 04/16] Remove unnecessary kmalloc casts from the sparc arch.

[jack]

[-- Attachment #1: arch_sparc_remove_kmalloc_casts.patch --]
[-- Type: text/plain, Size: 571 bytes --]

Signed-off-by: Jack Stone <jack@hawkeye.stone.uk.eu.org>

CC: David Miller <davem@davemloft.net>

---
 arch/sparc/kernel/ebus.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: b/arch/sparc/kernel/ebus.c
===================================================================
--- a/arch/sparc/kernel/ebus.c
+++ b/arch/sparc/kernel/ebus.c
@@ -64,7 +64,7 @@ static struct ebus_device_irq *ebus_blac
  */
 static inline unsigned long ebus_alloc(size_t size)
 {
-	return (unsigned long)kmalloc(size, GFP_ATOMIC);
+	return kmalloc(size, GFP_ATOMIC);
 }
 
 /*

-- 

Re: [Patch 04/16] Remove unnecessary kmalloc casts from the sparc arch.

[WANG Cong]

On Tue, Jul 31, 2007 at 01:53:20PM +0100, jack@hawkeye.stone.uk.eu.org wrote:
>Signed-off-by: Jack Stone <jack@hawkeye.stone.uk.eu.org>
>
>CC: David Miller <davem@davemloft.net>
>
>---
> arch/sparc/kernel/ebus.c |    2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>Index: b/arch/sparc/kernel/ebus.c
>===================================================================
>--- a/arch/sparc/kernel/ebus.c
>+++ b/arch/sparc/kernel/ebus.c
>@@ -64,7 +64,7 @@ static struct ebus_device_irq *ebus_blac
>  */
> static inline unsigned long ebus_alloc(size_t size)
> {
>-	return (unsigned long)kmalloc(size, GFP_ATOMIC);
>+	return kmalloc(size, GFP_ATOMIC);

AFAIK, this cast is necessary, since void * is a pointer type while
unsigned long is not.

Regards.

-- 
To do great work, you have to have a pure mind. You can think only about the
mathematics. Everything else is human weakness. Accepting prizes is showing
weakness.

[Patch 05/16] Remove unnecessary kmalloc casts in the drivers subdirectory.

[jack]

[-- Attachment #1: drivers_char_remove_kmalloc_casts.patch --]
[-- Type: text/plain, Size: 588 bytes --]

Signed-off-by: Jack Stone <jack@hawkeye.stone.uk.eu.org>

CC: Greg Kroah-Hartman <gregkh@suse.de>

---
 drivers/char/tty_io.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: b/drivers/char/tty_io.c
===================================================================
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -2034,7 +2034,7 @@ static int init_dev(struct tty_driver *d
 	}
 
 	if (!*tp_loc) {
-		tp = (struct ktermios *) kmalloc(sizeof(struct ktermios),
+		tp = kmalloc(sizeof(struct ktermios),
 						GFP_KERNEL);
 		if (!tp)
 			goto free_mem_out;

-- 

Re: [Patch 05/16] Remove unnecessary kmalloc casts in the drivers subdirectory.

[Stefan Richter]

jack@hawkeye.stone.uk.eu.org wrote:
> --- a/drivers/char/tty_io.c
> +++ b/drivers/char/tty_io.c
> @@ -2034,7 +2034,7 @@ static int init_dev(struct tty_driver *d
>  	}
>  
>  	if (!*tp_loc) {
> -		tp = (struct ktermios *) kmalloc(sizeof(struct ktermios),
> +		tp = kmalloc(sizeof(struct ktermios),
>  						GFP_KERNEL);
>  		if (!tp)
>  			goto free_mem_out;
> 

Looks like you can pull the "GFP_KERNEL);" into the same line as the
kmalloc.  IMO a cleanup patch shouldn't leave inexplicable linebreaks
and other whitespace strangeness behind.
-- 
Stefan Richter
-=====-=-=== =--- ----=
http://arcgraph.de/sr/

[Patch 06/16] This removes unnecessary kmalloc casts and corrects a test for kmalloc failure.

[jack]

[-- Attachment #1: drivers_media_remove_kmalloc_casts.patch --]
[-- Type: text/plain, Size: 1209 bytes --]

Signed-off-by: Jack Stone <jack@hawkeye.stone.uk.eu.org>

CC: Ronald Bultje <rbultje@ronald.bitfreak.net>

---
 drivers/media/video/zoran_driver.c |   12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

Index: b/drivers/media/video/zoran_driver.c
===================================================================
--- a/drivers/media/video/zoran_driver.c
+++ b/drivers/media/video/zoran_driver.c
@@ -347,11 +347,8 @@ v4l_fbuffer_alloc (struct file *file)
 		if (fh->v4l_buffers.buffer_size <= MAX_KMALLOC_MEM) {
 			/* Use kmalloc */
 
-			mem =
-			    (unsigned char *) kmalloc(fh->v4l_buffers.
-						      buffer_size,
-						      GFP_KERNEL);
-			if (mem == 0) {
+			mem = kmalloc(fh->v4l_buffers.buffer_size, GFP_KERNEL);
+			if (!mem) {
 				dprintk(1,
 					KERN_ERR
 					"%s: v4l_fbuffer_alloc() - kmalloc for V4L buf %d failed\n",
@@ -520,10 +517,7 @@ jpg_fbuffer_alloc (struct file *file)
 
 		//if (alloc_contig) {
 		if (fh->jpg_buffers.need_contiguous) {
-			mem =
-			    (unsigned long) kmalloc(fh->jpg_buffers.
-						    buffer_size,
-						    GFP_KERNEL);
+			mem = kmalloc(fh->jpg_buffers.buffer_size, GFP_KERNEL);
 			if (mem == 0) {
 				dprintk(1,
 					KERN_ERR

-- 

Re: [Patch 06/16] This removes unnecessary kmalloc casts and corrects a test for kmalloc failure.

[Christian Borntraeger]

Am Dienstag, 31. Juli 2007 schrieb jack@hawkeye.stone.uk.eu.org:
> --- a/drivers/media/video/zoran_driver.c
> +++ b/drivers/media/video/zoran_driver.c
[...]
> -			mem =
> -			    (unsigned long) kmalloc(fh->jpg_buffers.
> -						    buffer_size,
> -						    GFP_KERNEL);
> +			mem = kmalloc(fh->jpg_buffers.buffer_size, GFP_KERNEL);
[...]

NAK. See the comments to the other patches. The cast is necessary.

[Patch 07/16] Remove unnecessary kmalloc casts in the mtd drivers.

[jack]

[-- Attachment #1: drivers_mtd_remove_kmalloc_casts.patch --]
[-- Type: text/plain, Size: 1862 bytes --]

Signed-off-by: Jack Stone <jack@hawkeye.stone.uk.eu.org>

CC: David Woodhouse <dwmw2@infradead.org>

---
 drivers/mtd/maps/pmcmsp-flash.c |   11 ++++-------
 drivers/mtd/maps/tqm8xxl.c      |    2 +-
 2 files changed, 5 insertions(+), 8 deletions(-)

Index: b/drivers/mtd/maps/pmcmsp-flash.c
===================================================================
--- a/drivers/mtd/maps/pmcmsp-flash.c
+++ b/drivers/mtd/maps/pmcmsp-flash.c
@@ -73,12 +73,9 @@ int __init init_msp_flash(void)
 		return -ENXIO;
 
 	printk(KERN_NOTICE "Found %d PMC flash devices\n", fcnt);
-	msp_flash = (struct mtd_info **)kmalloc(
-			fcnt * sizeof(struct map_info *), GFP_KERNEL);
-	msp_parts = (struct mtd_partition **)kmalloc(
-			fcnt * sizeof(struct mtd_partition *), GFP_KERNEL);
-	msp_maps = (struct map_info *)kmalloc(
-			fcnt * sizeof(struct mtd_info), GFP_KERNEL);
+	msp_flash = kmalloc(fcnt * sizeof(struct map_info *), GFP_KERNEL);
+	msp_parts = kmalloc(fcnt * sizeof(struct mtd_partition *), GFP_KERNEL);
+	msp_maps  = kmalloc(fcnt * sizeof(struct mtd_info), GFP_KERNEL);
 	memset(msp_maps, 0, fcnt * sizeof(struct mtd_info));
 
 	/* loop over the flash devices, initializing each */
@@ -95,7 +92,7 @@ int __init init_msp_flash(void)
 			continue;
 		}
 
-		msp_parts[i] = (struct mtd_partition *)kmalloc(
+		msp_parts[i] = kmalloc(
 			pcnt * sizeof(struct mtd_partition), GFP_KERNEL);
 		memset(msp_parts[i], 0, pcnt * sizeof(struct mtd_partition));
 
Index: b/drivers/mtd/maps/tqm8xxl.c
===================================================================
--- a/drivers/mtd/maps/tqm8xxl.c
+++ b/drivers/mtd/maps/tqm8xxl.c
@@ -141,7 +141,7 @@ int __init init_tqm_mtd(void)
 			goto error_mem;
 		}
 
-		map_banks[idx]->name = (char *)kmalloc(16, GFP_KERNEL);
+		map_banks[idx]->name = kmalloc(16, GFP_KERNEL);
 
 		if (!map_banks[idx]->name) {
 			ret = -ENOMEM;

-- 

[Patch 08/16] Remove unnecessary kmalloc casts in the parisc drivers.

[jack]

[-- Attachment #1: drivers_parisc_remove_kmalloc_casts.patch --]
[-- Type: text/plain, Size: 735 bytes --]

Signed-off-by: Jack Stone <jack@hawkeye.stone.uk.eu.org>

CC: Matthew Wilcox <matthew@will.cx>

---
 drivers/parisc/iosapic.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: b/drivers/parisc/iosapic.c
===================================================================
--- a/drivers/parisc/iosapic.c
+++ b/drivers/parisc/iosapic.c
@@ -243,7 +243,7 @@ static struct irt_entry *iosapic_alloc_i
 	 * if CONFIG_DEBUG_SLAB is enabled, then we can get only
 	 * 4-byte alignment on 32-bit kernels
 	 */
-	a = (unsigned long)kmalloc(sizeof(struct irt_entry) * num_entries + 8, GFP_KERNEL);
+	a = kmalloc(sizeof(struct irt_entry) * num_entries + 8, GFP_KERNEL);
 	a = (a + 7UL) & ~7UL;
 	return (struct irt_entry *)a;
 }

-- 

Re: [Patch 08/16] Remove unnecessary kmalloc casts in the parisc drivers.

[Christian Borntraeger]

Am Dienstag, 31. Juli 2007 schrieb jack@hawkeye.stone.uk.eu.org:
> -	a = (unsigned long)kmalloc(sizeof(struct irt_entry) * num_entries + 8, 
GFP_KERNEL);
> +	a = kmalloc(sizeof(struct irt_entry) * num_entries + 8, GFP_KERNEL);

NAK.
Same as in some other patches.
"void *" -> "ulong" is not auto casted and will give you a "warning: 
assignment makes integer from pointer without a cast".

Christian

[Patch 09/16] Remove unnecessary kmalloc casts in the pci subsystem.

[jack]

[-- Attachment #1: drivers_pci_remove_kmalloc_casts.patch --]
[-- Type: text/plain, Size: 576 bytes --]

Signed-off-by: Jack Stone <jack@hawkeye.stone.uk.eu.org>

CC: Greg Kroah-Hartman <gregkh@suse.de>

---
 drivers/pci/rom.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: b/drivers/pci/rom.c
===================================================================
--- a/drivers/pci/rom.c
+++ b/drivers/pci/rom.c
@@ -185,7 +185,7 @@ void __iomem *pci_map_rom_copy(struct pc
 			  IORESOURCE_ROM_BIOS_COPY))
 		return rom;
 
-	res->start = (unsigned long)kmalloc(*size, GFP_KERNEL);
+	res->start = kmalloc(*size, GFP_KERNEL);
 	if (!res->start)
 		return rom;
 

-- 

Re: [Patch 09/16] Remove unnecessary kmalloc casts in the pci subsystem.

[Christian Borntraeger]

Am Dienstag, 31. Juli 2007 schrieb jack@hawkeye.stone.uk.eu.org:
> --- a/drivers/pci/rom.c
> +++ b/drivers/pci/rom.c
> @@ -185,7 +185,7 @@ void __iomem *pci_map_rom_copy(struct pc
>  			  IORESOURCE_ROM_BIOS_COPY))
>  		return rom;
> 
> -	res->start = (unsigned long)kmalloc(*size, GFP_KERNEL);
> +	res->start = kmalloc(*size, GFP_KERNEL);

This looks wrong. void * doesnt need a cast to a pointer, but res->start is an 
integer u32 type, and therefore we need a cast, no?



-- 
IBM Deutschland Entwicklung GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Herbert Kircher 
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: [Patch 09/16] Remove unnecessary kmalloc casts in the pci subsystem.

[Satyam Sharma]

On Tue, 31 Jul 2007, Christian Borntraeger wrote:

> Am Dienstag, 31. Juli 2007 schrieb jack@hawkeye.stone.uk.eu.org:
> > --- a/drivers/pci/rom.c
> > +++ b/drivers/pci/rom.c
> > @@ -185,7 +185,7 @@ void __iomem *pci_map_rom_copy(struct pc
> >  			  IORESOURCE_ROM_BIOS_COPY))
> >  		return rom;
> > 
> > -	res->start = (unsigned long)kmalloc(*size, GFP_KERNEL);
> > +	res->start = kmalloc(*size, GFP_KERNEL);
> 
> This looks wrong.

Yup, a warning at the very least.

> void * doesnt need a cast to a pointer, but res->start is an 
> integer u32 type,

It better not be, else we have a bug already anyway. Pointers are 64-bit
on 64-bit archs. [ it turns out res->start is resource_size_t which is
set properly as per CONFIG_RESOURCES_64BIT which itself is set properly
as per CONFIG_64BIT, so everything is healthy and fine :-) ]

> and therefore we need a cast, no?

Yes, void * -> unsigned long cast is always ok, and what we need here.


Satyam

Re: [Patch 09/16] Remove unnecessary kmalloc casts in the pci subsystem.

[WANG Cong]

On Tue, Jul 31, 2007 at 08:00:12PM +0530, Satyam Sharma wrote:
>
>
>On Tue, 31 Jul 2007, Christian Borntraeger wrote:
>
>> Am Dienstag, 31. Juli 2007 schrieb jack@hawkeye.stone.uk.eu.org:
>> > --- a/drivers/pci/rom.c
>> > +++ b/drivers/pci/rom.c
>> > @@ -185,7 +185,7 @@ void __iomem *pci_map_rom_copy(struct pc
>> >  			  IORESOURCE_ROM_BIOS_COPY))
>> >  		return rom;
>> > 
>> > -	res->start = (unsigned long)kmalloc(*size, GFP_KERNEL);
>> > +	res->start = kmalloc(*size, GFP_KERNEL);
>> 
>> This looks wrong.
>
>Yup, a warning at the very least.
>
>> void * doesnt need a cast to a pointer, but res->start is an 
>> integer u32 type,
>
>It better not be, else we have a bug already anyway. Pointers are 64-bit
>on 64-bit archs. [ it turns out res->start is resource_size_t which is
>set properly as per CONFIG_RESOURCES_64BIT which itself is set properly
>as per CONFIG_64BIT, so everything is healthy and fine :-) ]
>

I agree.

However, I think using resource_size_t is a bit better than unsigned long,
so that we don't need to check the defination of it.

- res->start = (unsigned long)kmalloc(*size, GFP_KERNEL);
+ res->start = (resource_size_t)kmalloc(*size, GFP_KERNEL);

Is this change OK?

-- 
To do great work, you have to have a pure mind. You can think only about the
mathematics. Everything else is human weakness. Accepting prizes is showing
weakness.

Re: [Patch 09/16] Remove unnecessary kmalloc casts in the pci subsystem.

[Satyam Sharma]

On Tue, 31 Jul 2007, WANG Cong wrote:

> On Tue, Jul 31, 2007 at 08:00:12PM +0530, Satyam Sharma wrote:
> > [...]
> >It better not be, else we have a bug already anyway. Pointers are 64-bit
> >on 64-bit archs. [ it turns out res->start is resource_size_t which is
> >set properly as per CONFIG_RESOURCES_64BIT which itself is set properly
> >as per CONFIG_64BIT, so everything is healthy and fine :-) ]
> >
> 
> I agree.
> 
> However, I think using resource_size_t is a bit better than unsigned long,
> so that we don't need to check the defination of it.
> 
> - res->start = (unsigned long)kmalloc(*size, GFP_KERNEL);
> + res->start = (resource_size_t)kmalloc(*size, GFP_KERNEL);
> 
> Is this change OK?

Yes, that would probably be clearer.

Re: [Patch 09/16] Remove unnecessary kmalloc casts in the pci subsystem.

[Greg KH]

On Tue, Jul 31, 2007 at 10:46:19PM +0800, WANG Cong wrote:
> On Tue, Jul 31, 2007 at 08:00:12PM +0530, Satyam Sharma wrote:
> >
> >
> >On Tue, 31 Jul 2007, Christian Borntraeger wrote:
> >
> >> Am Dienstag, 31. Juli 2007 schrieb jack@hawkeye.stone.uk.eu.org:
> >> > --- a/drivers/pci/rom.c
> >> > +++ b/drivers/pci/rom.c
> >> > @@ -185,7 +185,7 @@ void __iomem *pci_map_rom_copy(struct pc
> >> >  			  IORESOURCE_ROM_BIOS_COPY))
> >> >  		return rom;
> >> > 
> >> > -	res->start = (unsigned long)kmalloc(*size, GFP_KERNEL);
> >> > +	res->start = kmalloc(*size, GFP_KERNEL);
> >> 
> >> This looks wrong.
> >
> >Yup, a warning at the very least.
> >
> >> void * doesnt need a cast to a pointer, but res->start is an 
> >> integer u32 type,
> >
> >It better not be, else we have a bug already anyway. Pointers are 64-bit
> >on 64-bit archs. [ it turns out res->start is resource_size_t which is
> >set properly as per CONFIG_RESOURCES_64BIT which itself is set properly
> >as per CONFIG_64BIT, so everything is healthy and fine :-) ]
> >
> 
> I agree.
> 
> However, I think using resource_size_t is a bit better than unsigned long,
> so that we don't need to check the defination of it.
> 
> - res->start = (unsigned long)kmalloc(*size, GFP_KERNEL);
> + res->start = (resource_size_t)kmalloc(*size, GFP_KERNEL);
> 
> Is this change OK?

Yes, that is the proper cast to have here.

thanks,

greg k-h

Re: [Patch 09/16] Remove unnecessary kmalloc casts in the pci subsystem.

[Andreas Schwab]

Greg KH <gregkh@suse.de> writes:

> On Tue, Jul 31, 2007 at 10:46:19PM +0800, WANG Cong wrote:
>> However, I think using resource_size_t is a bit better than unsigned long,
>> so that we don't need to check the defination of it.
>> 
>> - res->start = (unsigned long)kmalloc(*size, GFP_KERNEL);
>> + res->start = (resource_size_t)kmalloc(*size, GFP_KERNEL);
>> 
>> Is this change OK?
>
> Yes, that is the proper cast to have here.

It will generate a warning when resoure_size_t is 64bit on a 32bit arch.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [Patch 09/16] Remove unnecessary kmalloc casts in the pci subsystem.

[Greg KH]

On Tue, Jul 31, 2007 at 06:20:29PM +0200, Andreas Schwab wrote:
> Greg KH <gregkh@suse.de> writes:
> 
> > On Tue, Jul 31, 2007 at 10:46:19PM +0800, WANG Cong wrote:
> >> However, I think using resource_size_t is a bit better than unsigned long,
> >> so that we don't need to check the defination of it.
> >> 
> >> - res->start = (unsigned long)kmalloc(*size, GFP_KERNEL);
> >> + res->start = (resource_size_t)kmalloc(*size, GFP_KERNEL);
> >> 
> >> Is this change OK?
> >
> > Yes, that is the proper cast to have here.
> 
> It will generate a warning when resoure_size_t is 64bit on a 32bit arch.

How?  And if so, what should the proper cast be?

thanks,

greg k-h

Re: [Patch 09/16] Remove unnecessary kmalloc casts in the pci subsystem.

[Satyam Sharma]

On Tue, 31 Jul 2007, Greg KH wrote:

> On Tue, Jul 31, 2007 at 06:20:29PM +0200, Andreas Schwab wrote:
> > Greg KH <gregkh@suse.de> writes:
> > 
> > > On Tue, Jul 31, 2007 at 10:46:19PM +0800, WANG Cong wrote:
> > >> However, I think using resource_size_t is a bit better than unsigned long,
> > >> so that we don't need to check the defination of it.
> > >> 
> > >> - res->start = (unsigned long)kmalloc(*size, GFP_KERNEL);
> > >> + res->start = (resource_size_t)kmalloc(*size, GFP_KERNEL);
> > >> 
> > >> Is this change OK?
> > >
> > > Yes, that is the proper cast to have here.
> > 
> > It will generate a warning when resoure_size_t is 64bit on a 32bit arch.
> 
> How?

warning: cast from pointer to integer of different size

(it's from smaller to larger size, so not really truncation,
but gcc does barf)

> And if so, what should the proper cast be?

sizeof(unsigned long) == sizeof(void *) should always be true.


Satyam

Re: [Patch 09/16] Remove unnecessary kmalloc casts in the pci subsystem.

[Paul Mackerras]

Greg KH writes:

> On Tue, Jul 31, 2007 at 06:20:29PM +0200, Andreas Schwab wrote:
> > Greg KH <gregkh@suse.de> writes:
> > 
> > > On Tue, Jul 31, 2007 at 10:46:19PM +0800, WANG Cong wrote:
> > >> However, I think using resource_size_t is a bit better than unsigned long,
> > >> so that we don't need to check the defination of it.
> > >> 
> > >> - res->start = (unsigned long)kmalloc(*size, GFP_KERNEL);
> > >> + res->start = (resource_size_t)kmalloc(*size, GFP_KERNEL);
> > >> 
> > >> Is this change OK?
> > >
> > > Yes, that is the proper cast to have here.
> > 
> > It will generate a warning when resoure_size_t is 64bit on a 32bit arch.
> 
> How?  And if so, what should the proper cast be?

You'll get a "cast from pointer to different-sized integer" type of
warning.  The proper cast is unsigned long, just like the code is
without this patch. :)

Paul.

Re: [Patch 09/16] Remove unnecessary kmalloc casts in the pci subsystem.

[David Miller]

From: Christian Borntraeger <borntraeger@de.ibm.com>
Date: Tue, 31 Jul 2007 15:53:28 +0200

> Am Dienstag, 31. Juli 2007 schrieb jack@hawkeye.stone.uk.eu.org:
> > --- a/drivers/pci/rom.c
> > +++ b/drivers/pci/rom.c
> > @@ -185,7 +185,7 @@ void __iomem *pci_map_rom_copy(struct pc
> >  			  IORESOURCE_ROM_BIOS_COPY))
> >  		return rom;
> > 
> > -	res->start = (unsigned long)kmalloc(*size, GFP_KERNEL);
> > +	res->start = kmalloc(*size, GFP_KERNEL);
> 
> This looks wrong. void * doesnt need a cast to a pointer, but res->start is an 
> integer u32 type, and therefore we need a cast, no?

Similar issue with the arch/sparc/kernel/ebus.c change.

If the cast is being done to an integer type you can't
remove it.

[Patch 10/16] Remove unnecessary kmalloc casts in the irda drivers.

[jack]

[-- Attachment #1: drivers_net_irda_remove_kmalloc_casts.patch --]
[-- Type: text/plain, Size: 884 bytes --]

Signed-off-by: Jack Stone <jack@hawkeye.stone.uk.eu.org>

CC: Samuel Ortiz <samuel@sortiz.org>

---
 drivers/net/irda/kingsun-sir.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: b/drivers/net/irda/kingsun-sir.c
===================================================================
--- a/drivers/net/irda/kingsun-sir.c
+++ b/drivers/net/irda/kingsun-sir.c
@@ -509,12 +509,12 @@ static int kingsun_probe(struct usb_inte
 	spin_lock_init(&kingsun->lock);
 
 	/* Allocate input buffer */
-	kingsun->in_buf = (__u8 *)kmalloc(kingsun->max_rx, GFP_KERNEL);
+	kingsun->in_buf = kmalloc(kingsun->max_rx, GFP_KERNEL);
 	if (!kingsun->in_buf)
 		goto free_mem;
 
 	/* Allocate output buffer */
-	kingsun->out_buf = (__u8 *)kmalloc(KINGSUN_FIFO_SIZE, GFP_KERNEL);
+	kingsun->out_buf = kmalloc(KINGSUN_FIFO_SIZE, GFP_KERNEL);
 	if (!kingsun->out_buf)
 		goto free_mem;
 

-- 

[Patch 11/16] Remove unnecessary kmalloc casts in drivers/net.

[jack]

[-- Attachment #1: drivers_net_remove_kmalloc_casts.patch --]
[-- Type: text/plain, Size: 3266 bytes --]

Signed-off-by: Jack Stone <jack@hawkeye.stone.uk.eu.org>

CC: David Millver <davem@davemloft.net>

---
 drivers/net/gianfar.c |    6 ++----
 drivers/net/lance.c   |    3 +--
 drivers/net/s2io.c    |    4 ++--
 drivers/net/sgiseeq.c |    4 ++--
 4 files changed, 7 insertions(+), 10 deletions(-)

Index: b/drivers/net/lance.c
===================================================================
--- a/drivers/net/lance.c
+++ b/drivers/net/lance.c
@@ -539,8 +539,7 @@ static int __init lance_probe1(struct ne
 	if (lance_debug > 6) printk(" (#0x%05lx)", (unsigned long)lp);
 	dev->priv = lp;
 	lp->name = chipname;
-	lp->rx_buffs = (unsigned long)kmalloc(PKT_BUF_SZ*RX_RING_SIZE,
-						  GFP_DMA | GFP_KERNEL);
+	lp->rx_buffs = kmalloc(PKT_BUF_SZ*RX_RING_SIZE, GFP_DMA | GFP_KERNEL);
 	if (!lp->rx_buffs)
 		goto out_lp;
 	if (lance_need_isa_bounce_buffers) {
Index: b/drivers/net/gianfar.c
===================================================================
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -737,8 +737,7 @@ int startup_gfar(struct net_device *dev)
 	gfar_write(&regs->rbase0, addr);
 
 	/* Setup the skbuff rings */
-	priv->tx_skbuff =
-	    (struct sk_buff **) kmalloc(sizeof (struct sk_buff *) *
+	priv->tx_skbuff = kmalloc(sizeof (struct sk_buff *) *
 					priv->tx_ring_size, GFP_KERNEL);
 
 	if (NULL == priv->tx_skbuff) {
@@ -752,8 +751,7 @@ int startup_gfar(struct net_device *dev)
 	for (i = 0; i < priv->tx_ring_size; i++)
 		priv->tx_skbuff[i] = NULL;
 
-	priv->rx_skbuff =
-	    (struct sk_buff **) kmalloc(sizeof (struct sk_buff *) *
+	priv->rx_skbuff = kmalloc(sizeof (struct sk_buff *) *
 					priv->rx_ring_size, GFP_KERNEL);
 
 	if (NULL == priv->rx_skbuff) {
Index: b/drivers/net/s2io.c
===================================================================
--- a/drivers/net/s2io.c
+++ b/drivers/net/s2io.c
@@ -729,7 +729,7 @@ static int init_shared_mem(struct s2io_n
 				while (k != rxd_count[nic->rxd_mode]) {
 					ba = &mac_control->rings[i].ba[j][k];
 
-					ba->ba_0_org = (void *) kmalloc
+					ba->ba_0_org = kmalloc
 					    (BUF0_LEN + ALIGN_SIZE, GFP_KERNEL);
 					if (!ba->ba_0_org)
 						return -ENOMEM;
@@ -740,7 +740,7 @@ static int init_shared_mem(struct s2io_n
 					tmp &= ~((unsigned long) ALIGN_SIZE);
 					ba->ba_0 = (void *) tmp;
 
-					ba->ba_1_org = (void *) kmalloc
+					ba->ba_1_org = kmalloc
 					    (BUF1_LEN + ALIGN_SIZE, GFP_KERNEL);
 					if (!ba->ba_1_org)
 						return -ENOMEM;
Index: b/drivers/net/sgiseeq.c
===================================================================
--- a/drivers/net/sgiseeq.c
+++ b/drivers/net/sgiseeq.c
@@ -167,7 +167,7 @@ static int seeq_init_ring(struct net_dev
 		if (!sp->tx_desc[i].tdma.pbuf) {
 			unsigned long buffer;
 
-			buffer = (unsigned long) kmalloc(PKT_BUF_SZ, GFP_KERNEL);
+			buffer = kmalloc(PKT_BUF_SZ, GFP_KERNEL);
 			if (!buffer)
 				return -ENOMEM;
 			sp->tx_desc[i].buf_vaddr = CKSEG1ADDR(buffer);
@@ -181,7 +181,7 @@ static int seeq_init_ring(struct net_dev
 		if (!sp->rx_desc[i].rdma.pbuf) {
 			unsigned long buffer;
 
-			buffer = (unsigned long) kmalloc(PKT_BUF_SZ, GFP_KERNEL);
+			buffer = kmalloc(PKT_BUF_SZ, GFP_KERNEL);
 			if (!buffer)
 				return -ENOMEM;
 			sp->rx_desc[i].buf_vaddr = CKSEG1ADDR(buffer);

-- 

Re: [Patch 11/16] Remove unnecessary kmalloc casts in drivers/net.

[Christian Borntraeger]

Am Dienstag, 31. Juli 2007 schrieb jack@hawkeye.stone.uk.eu.org:
> --- a/drivers/net/lance.c
> +++ b/drivers/net/lance.c
[...]
> -	lp->rx_buffs = (unsigned long)kmalloc(PKT_BUF_SZ*RX_RING_SIZE,
> -						  GFP_DMA | GFP_KERNEL);
> +	lp->rx_buffs = kmalloc(PKT_BUF_SZ*RX_RING_SIZE, GFP_DMA | GFP_KERNEL);
[...]
> --- a/drivers/net/sgiseeq.c
> +++ b/drivers/net/sgiseeq.c
[...]
> -			buffer = (unsigned long) kmalloc(PKT_BUF_SZ, GFP_KERNEL);
> +			buffer = kmalloc(PKT_BUF_SZ, GFP_KERNEL);
[...]
> -			buffer = (unsigned long) kmalloc(PKT_BUF_SZ, GFP_KERNEL);
> +			buffer = kmalloc(PKT_BUF_SZ, GFP_KERNEL);

See the comments to the other patches. The cast is necessary.

[Patch 12/16] Remove unnecessary kmalloc casts in the libertas drivers.

[jack]

[-- Attachment #1: drivers_net_wireless_libertas_remove_kmalloc_casts.patch --]
[-- Type: text/plain, Size: 1301 bytes --]

Signed-off-by: Jack Stone <jack@hawkeye.stone.uk.eu.org>

CC: John Linville <linville@tuxdriver.com>

---
 drivers/net/wireless/libertas/debugfs.c |    2 +-
 drivers/net/wireless/libertas/ethtool.c |    3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

Index: b/drivers/net/wireless/libertas/debugfs.c
===================================================================
--- a/drivers/net/wireless/libertas/debugfs.c
+++ b/drivers/net/wireless/libertas/debugfs.c
@@ -1839,7 +1839,7 @@ static ssize_t wlan_debugfs_write(struct
 	char *p2;
 	struct debug_data *d = (struct debug_data *)f->private_data;
 
-	pdata = (char *)kmalloc(cnt, GFP_KERNEL);
+	pdata = kmalloc(cnt, GFP_KERNEL);
 	if (pdata == NULL)
 		return 0;
 
Index: b/drivers/net/wireless/libertas/ethtool.c
===================================================================
--- a/drivers/net/wireless/libertas/ethtool.c
+++ b/drivers/net/wireless/libertas/ethtool.c
@@ -60,8 +60,7 @@ static int libertas_ethtool_get_eeprom(s
 
 //      mutex_lock(&priv->mutex);
 
-	adapter->prdeeprom =
-		    (char *)kmalloc(eeprom->len+sizeof(regctrl), GFP_KERNEL);
+	adapter->prdeeprom = kmalloc(eeprom->len+sizeof(regctrl), GFP_KERNEL);
 	if (!adapter->prdeeprom)
 		return -ENOMEM;
 	memcpy(adapter->prdeeprom, &regctrl, sizeof(regctrl));

-- 

[Patch 13/16] Remove needless kmalloc casts in the wireless drivers.

[jack]

[-- Attachment #1: drivers_net_wireless_remove_kmalloc_casts.patch --]
[-- Type: text/plain, Size: 1372 bytes --]

Signed-off-by: Jack Stone <jack@hawkeye.stone.uk.eu.org>

CC: John Linville <linville@tuxdriver.com>

---
 drivers/net/wireless/ipw2100.c |   16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

Index: b/drivers/net/wireless/ipw2100.c
===================================================================
--- a/drivers/net/wireless/ipw2100.c
+++ b/drivers/net/wireless/ipw2100.c
@@ -3363,11 +3363,9 @@ static int ipw2100_msg_allocate(struct i
 	void *v;
 	dma_addr_t p;
 
-	priv->msg_buffers =
-	    (struct ipw2100_tx_packet *)kmalloc(IPW_COMMAND_POOL_SIZE *
-						sizeof(struct
-						       ipw2100_tx_packet),
-						GFP_KERNEL);
+	priv->msg_buffers = kmalloc(IPW_COMMAND_POOL_SIZE *
+					sizeof(struct ipw2100_tx_packet),
+					GFP_KERNEL);
 	if (!priv->msg_buffers) {
 		printk(KERN_ERR DRV_NAME ": %s: PCI alloc failed for msg "
 		       "buffers.\n", priv->net_dev->name);
@@ -4396,11 +4394,9 @@ static int ipw2100_tx_allocate(struct ip
 		return err;
 	}
 
-	priv->tx_buffers =
-	    (struct ipw2100_tx_packet *)kmalloc(TX_PENDED_QUEUE_LENGTH *
-						sizeof(struct
-						       ipw2100_tx_packet),
-						GFP_ATOMIC);
+	priv->tx_buffers = kmalloc(TX_PENDED_QUEUE_LENGTH *
+					sizeof(struct ipw2100_tx_packet),
+					GFP_ATOMIC);
 	if (!priv->tx_buffers) {
 		printk(KERN_ERR DRV_NAME
 		       ": %s: alloc failed form tx buffers.\n",

-- 

[Patch 14/16] Remove needless kmalloc casts in the zd1211rw drivers.

[jack]

[-- Attachment #1: drivers_net_wireless_zd1211rw_remove_kmalloc_casts.patch --]
[-- Type: text/plain, Size: 776 bytes --]

Signed-off-by: Jack Stone <jack@hawkeye.stone.uk.eu.org>

CC: Daniel Drake <dsd@gentoo.org>

---
 drivers/net/wireless/zd1211rw/zd_chip.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: b/drivers/net/wireless/zd1211rw/zd_chip.c
===================================================================
--- a/drivers/net/wireless/zd1211rw/zd_chip.c
+++ b/drivers/net/wireless/zd1211rw/zd_chip.c
@@ -115,7 +115,7 @@ int zd_ioread32v_locked(struct zd_chip *
 
 	/* Allocate a single memory block for values and addresses. */
 	count16 = 2*count;
-	a16 = (zd_addr_t *) kmalloc(count16 * (sizeof(zd_addr_t) + sizeof(u16)),
+	a16 = kmalloc(count16 * (sizeof(zd_addr_t) + sizeof(u16)),
 		                   GFP_KERNEL);
 	if (!a16) {
 		dev_dbg_f(zd_chip_dev(chip),

-- 

Re: [Patch 14/16] Remove needless kmalloc casts in the zd1211rw drivers.

[Daniel Drake]

jack@hawkeye.stone.uk.eu.org wrote:
> Signed-off-by: Jack Stone <jack@hawkeye.stone.uk.eu.org>
> 
> CC: Daniel Drake <dsd@gentoo.org>

NAK: this patch adds a sparse warning
zd_chip.c:116:15: warning: implicit cast to nocast type

> ---
>  drivers/net/wireless/zd1211rw/zd_chip.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: b/drivers/net/wireless/zd1211rw/zd_chip.c
> ===================================================================
> --- a/drivers/net/wireless/zd1211rw/zd_chip.c
> +++ b/drivers/net/wireless/zd1211rw/zd_chip.c
> @@ -115,7 +115,7 @@ int zd_ioread32v_locked(struct zd_chip *
>  
>  	/* Allocate a single memory block for values and addresses. */
>  	count16 = 2*count;
> -	a16 = (zd_addr_t *) kmalloc(count16 * (sizeof(zd_addr_t) + sizeof(u16)),
> +	a16 = kmalloc(count16 * (sizeof(zd_addr_t) + sizeof(u16)),
>  		                   GFP_KERNEL);
>  	if (!a16) {
>  		dev_dbg_f(zd_chip_dev(chip),
> 

Re: [Patch 14/16] Remove needless kmalloc casts in the zd1211rw drivers.

[Stefan Richter]

Daniel Drake wrote:
> jack@hawkeye.stone.uk.eu.org wrote:
>> Signed-off-by: Jack Stone <jack@hawkeye.stone.uk.eu.org>
>>
>> CC: Daniel Drake <dsd@gentoo.org>
> 
> NAK: this patch adds a sparse warning
> zd_chip.c:116:15: warning: implicit cast to nocast type

What does this warning mean?  a16 is defined as zd_addr_t *.

>> --- a/drivers/net/wireless/zd1211rw/zd_chip.c
>> +++ b/drivers/net/wireless/zd1211rw/zd_chip.c
>> @@ -115,7 +115,7 @@ int zd_ioread32v_locked(struct zd_chip *
>>  
>>      /* Allocate a single memory block for values and addresses. */
>>      count16 = 2*count;
>> -    a16 = (zd_addr_t *) kmalloc(count16 * (sizeof(zd_addr_t) +
>> sizeof(u16)),
>> +    a16 = kmalloc(count16 * (sizeof(zd_addr_t) + sizeof(u16)),
>>                             GFP_KERNEL);
>>      if (!a16) {
>>          dev_dbg_f(zd_chip_dev(chip),
>>

Also, why is there this cast right before that?

	zd_addr_t *a16 = (zd_addr_t *)NULL;

-- 
Stefan Richter
-=====-=-=== =--- ----=
http://arcgraph.de/sr/

Re: [Patch 14/16] Remove needless kmalloc casts in the zd1211rw drivers.

[Daniel Drake]

Stefan Richter wrote:
>> NAK: this patch adds a sparse warning
>> zd_chip.c:116:15: warning: implicit cast to nocast type
> 
> What does this warning mean?  a16 is defined as zd_addr_t *.

zd_addr_t is defined as
typedef u16 __nocast zd_addr_t;

The __nocast annotation forces us to use explicit casts for values of 
this type. This is really useful as we have functions which take an 
address and a value (e.g. register writes). It's very easy to get them 
in the wrong order and have a non-obvious bug. With this mechanism, 
sparse helps us avoid this.

The case with pointers may be a little unintuitive, but it's a small 
price to pay for a bug that bit us hard once before...

The number of other places we have to cast is very small due to use of 
the preprocessor. For example we have:

#define ZD_ADDR(base, offset) \
	((zd_addr_t)(((base) & ADDR_BASE_MASK) | ((offset) & ADDR_OFFSET_MASK)))

#define CTL_REG(offset)  ZD_ADDR(CR_BASE, offset)	/* byte addressing */

Then we have a few hundred registers defined in terms of CTL_REG.

> Also, why is there this cast right before that?
> 
> 	zd_addr_t *a16 = (zd_addr_t *)NULL;

For similar reasons.

Daniel

[Patch 15/16] Remove unnecesary kmalloc casts in the scsi drivers.

[jack]

[-- Attachment #1: drivers_scsi_remove_kmalloc_casts.patch --]
[-- Type: text/plain, Size: 1312 bytes --]

Signed-off-by: Jack Stone <jack@hawkeye.stone.uk.eu.org>

CC: James Bottomley <james.bottomley@steeleye.com>

---
 drivers/scsi/advansys.c |    2 +-
 drivers/scsi/osst.c     |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Index: b/drivers/scsi/advansys.c
===================================================================
--- a/drivers/scsi/advansys.c
+++ b/drivers/scsi/advansys.c
@@ -18513,7 +18513,7 @@ advansys_board_found(int iop, struct dev
 		 * Allocate buffer carrier structures. The total size
 		 * is about 4 KB, so allocate all at once.
 		 */
-		carrp = (ADV_CARR_T *) kmalloc(ADV_CARRIER_BUFSIZE, GFP_ATOMIC);
+		carrp = kmalloc(ADV_CARRIER_BUFSIZE, GFP_ATOMIC);
 		ASC_DBG1(1, "advansys_board_found: carrp 0x%lx\n", (ulong)carrp);
 
 		if (carrp == NULL) {
Index: b/drivers/scsi/osst.c
===================================================================
--- a/drivers/scsi/osst.c
+++ b/drivers/scsi/osst.c
@@ -5756,7 +5756,7 @@ static int osst_probe(struct device *dev
 	write_lock(&os_scsi_tapes_lock);
 	if (os_scsi_tapes == NULL) {
 		os_scsi_tapes =
-			(struct osst_tape **)kmalloc(osst_max_dev * sizeof(struct osst_tape *),
+			kmalloc(osst_max_dev * sizeof(struct osst_tape *),
 				   GFP_ATOMIC);
 		if (os_scsi_tapes == NULL) {
 			write_unlock(&os_scsi_tapes_lock);

-- 

Re: [Patch 15/16] Remove unnecesary kmalloc casts in the scsi drivers.

[James Bottomley]

On Tue, 2007-07-31 at 13:53 +0100, jack@hawkeye.stone.uk.eu.org wrote:
> plain text document attachment
> (drivers_scsi_remove_kmalloc_casts.patch)
> Signed-off-by: Jack Stone <jack@hawkeye.stone.uk.eu.org>
> 
> CC: James Bottomley <james.bottomley@steeleye.com>
> 
> ---
>  drivers/scsi/advansys.c |    2 +-
>  drivers/scsi/osst.c     |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

a) You need to copy linux-scsi on SCSI patches, but 
b) The advansys driver is in the process of being rewritten, so the
chances of this no longer applying are very high.  If you send it to the
list or matthew wilcox, he can fold it in to his updates.

James

[Patch 16/16] Remove unnecessary kmalloc casts in the jfs filesystem.

[jack]

[-- Attachment #1: fs_jfs_remove_kmalloc_casts.patch --]
[-- Type: text/plain, Size: 1028 bytes --]

Signed-off-by: Jack Stone <jack@hawkeye.stone.uk.eu.org>

CC: David Kleikamp <shaggy@austin.ibm.com>

---
 fs/jfs/jfs_dtree.c |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Index: b/fs/jfs/jfs_dtree.c
===================================================================
--- a/fs/jfs/jfs_dtree.c
+++ b/fs/jfs/jfs_dtree.c
@@ -592,9 +592,7 @@ int dtSearch(struct inode *ip, struct co
 	struct component_name ciKey;
 	struct super_block *sb = ip->i_sb;
 
-	ciKey.name =
-	    (wchar_t *) kmalloc((JFS_NAME_MAX + 1) * sizeof(wchar_t),
-				GFP_NOFS);
+	ciKey.name = kmalloc((JFS_NAME_MAX + 1) * sizeof(wchar_t), GFP_NOFS);
 	if (ciKey.name == 0) {
 		rc = -ENOMEM;
 		goto dtSearch_Exit2;
@@ -957,9 +955,7 @@ static int dtSplitUp(tid_t tid,
 	smp = split->mp;
 	sp = DT_PAGE(ip, smp);
 
-	key.name =
-	    (wchar_t *) kmalloc((JFS_NAME_MAX + 2) * sizeof(wchar_t),
-				GFP_NOFS);
+	key.name = kmalloc((JFS_NAME_MAX + 2) * sizeof(wchar_t), GFP_NOFS);
 	if (key.name == 0) {
 		DT_PUTPAGE(smp);
 		rc = -ENOMEM;

-- 

Re: [Patch 16/16] Remove unnecessary kmalloc casts in the jfs filesystem.

[Dave Kleikamp]

Looks good.  I'll push it through the jfs git tree.

Thanks,
Shaggy

On Tue, 2007-07-31 at 13:53 +0100, jack@hawkeye.stone.uk.eu.org wrote:

> Signed-off-by: Jack Stone <jack@hawkeye.stone.uk.eu.org>
> 
> CC: David Kleikamp <shaggy@austin.ibm.com>
> ---
>  fs/jfs/jfs_dtree.c |    8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> Index: b/fs/jfs/jfs_dtree.c
> ===================================================================
> --- a/fs/jfs/jfs_dtree.c
> +++ b/fs/jfs/jfs_dtree.c
> @@ -592,9 +592,7 @@ int dtSearch(struct inode *ip, struct co
>  	struct component_name ciKey;
>  	struct super_block *sb = ip->i_sb;
>  
> -	ciKey.name =
> -	    (wchar_t *) kmalloc((JFS_NAME_MAX + 1) * sizeof(wchar_t),
> -				GFP_NOFS);
> +	ciKey.name = kmalloc((JFS_NAME_MAX + 1) * sizeof(wchar_t), GFP_NOFS);
>  	if (ciKey.name == 0) {
>  		rc = -ENOMEM;
>  		goto dtSearch_Exit2;
> @@ -957,9 +955,7 @@ static int dtSplitUp(tid_t tid,
>  	smp = split->mp;
>  	sp = DT_PAGE(ip, smp);
>  
> -	key.name =
> -	    (wchar_t *) kmalloc((JFS_NAME_MAX + 2) * sizeof(wchar_t),
> -				GFP_NOFS);
> +	key.name = kmalloc((JFS_NAME_MAX + 2) * sizeof(wchar_t), GFP_NOFS);
>  	if (key.name == 0) {
>  		DT_PUTPAGE(smp);
>  		rc = -ENOMEM;
> 
-- 
David Kleikamp
IBM Linux Technology Center