2010-06-08 03:50:47

by Joe Perches

[permalink] [raw]
Subject: [PATCH 00/12] treewide: Remove unnecessary kmalloc casts

Trivial cleanups

Joe Perches (12):
arch/cris: Remove unnecessary kmalloc casts
arch/powerpc: Remove unnecessary kmalloc casts
arch/x86/kernel/tlb_uv.c: Remove unnecessary kmalloc casts
drivers/gpu/drm: Remove unnecessary kmalloc casts
drivers/isdn/capi/capidrv.c: Remove unnecessary kmalloc casts
drivers/media: Remove unnecesary kmalloc casts
drivers/message/i2o/i20_config.c: Remove unnecessary kmalloc casts
drivers/parisc/iosapic.c: Remove unnecessary kzalloc cast
drivers/s390: Remove unnecessary kmalloc casts
drivers/serial/icom.c: Remove unnecessary kmalloc casts
drivers/usb/storage/isd200.c: Remove unnecessary kmalloc casts
fs/ufs/util.c: Remove unnecessary kmalloc casts


2010-06-08 03:50:53

by Joe Perches

[permalink] [raw]
Subject: [PATCH 10/12] drivers/serial/icom.c: Remove unnecessary kmalloc casts

Signed-off-by: Joe Perches <[email protected]>
---
drivers/serial/icom.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/serial/icom.c b/drivers/serial/icom.c
index 53a4682..105e25c 100644
--- a/drivers/serial/icom.c
+++ b/drivers/serial/icom.c
@@ -1416,8 +1416,7 @@ static int __devinit icom_alloc_adapter(struct icom_adapter
struct icom_adapter *cur_adapter_entry;
struct list_head *tmp;

- icom_adapter = (struct icom_adapter *)
- kzalloc(sizeof(struct icom_adapter), GFP_KERNEL);
+ icom_adapter = kzalloc(sizeof(struct icom_adapter), GFP_KERNEL);

if (!icom_adapter) {
return -ENOMEM;
--
1.7.1.244.gbdc4

2010-06-08 03:50:51

by Joe Perches

[permalink] [raw]
Subject: [PATCH 04/12] drivers/gpu/drm: Remove unnecessary kmalloc casts

Signed-off-by: Joe Perches <[email protected]>
---
drivers/gpu/drm/drm_sman.c | 4 +---
drivers/gpu/drm/i915/i915_dma.c | 3 +--
2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_sman.c b/drivers/gpu/drm/drm_sman.c
index 463aed9..3466458 100644
--- a/drivers/gpu/drm/drm_sman.c
+++ b/drivers/gpu/drm/drm_sman.c
@@ -59,9 +59,7 @@ drm_sman_init(struct drm_sman * sman, unsigned int num_managers,
{
int ret = 0;

- sman->mm = (struct drm_sman_mm *) kcalloc(num_managers,
- sizeof(*sman->mm),
- GFP_KERNEL);
+ sman->mm = kcalloc(num_managers, sizeof(*sman->mm), GFP_KERNEL);
if (!sman->mm) {
ret = -ENOMEM;
goto out;
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index b2ebf02..a1744bd 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2276,8 +2276,7 @@ int i915_driver_open(struct drm_device *dev, struct drm_file *file_priv)
struct drm_i915_file_private *i915_file_priv;

DRM_DEBUG_DRIVER("\n");
- i915_file_priv = (struct drm_i915_file_private *)
- kmalloc(sizeof(*i915_file_priv), GFP_KERNEL);
+ i915_file_priv = kmalloc(sizeof(*i915_file_priv), GFP_KERNEL);

if (!i915_file_priv)
return -ENOMEM;
--
1.7.1.244.gbdc4

2010-06-08 03:50:52

by Joe Perches

[permalink] [raw]
Subject: [PATCH 05/12] drivers/isdn/capi/capidrv.c: Remove unnecessary kmalloc casts

Signed-off-by: Joe Perches <[email protected]>
---
drivers/isdn/capi/capidrv.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/isdn/capi/capidrv.c b/drivers/isdn/capi/capidrv.c
index bf55ed5..02e0182 100644
--- a/drivers/isdn/capi/capidrv.c
+++ b/drivers/isdn/capi/capidrv.c
@@ -469,8 +469,7 @@ static int capidrv_add_ack(struct capidrv_ncci *nccip,
{
struct ncci_datahandle_queue *n, **pp;

- n = (struct ncci_datahandle_queue *)
- kmalloc(sizeof(struct ncci_datahandle_queue), GFP_ATOMIC);
+ n = kmalloc(sizeof(struct ncci_datahandle_queue), GFP_ATOMIC);
if (!n) {
printk(KERN_ERR "capidrv: kmalloc ncci_datahandle failed\n");
return -1;
--
1.7.1.244.gbdc4

2010-06-08 03:51:25

by Joe Perches

[permalink] [raw]
Subject: [PATCH 12/12] fs/ufs/util.c: Remove unnecessary kmalloc casts

Signed-off-by: Joe Perches <[email protected]>
---
fs/ufs/util.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/fs/ufs/util.c b/fs/ufs/util.c
index 85a7fc9..9c44f32 100644
--- a/fs/ufs/util.c
+++ b/fs/ufs/util.c
@@ -26,8 +26,7 @@ struct ufs_buffer_head * _ubh_bread_ (struct ufs_sb_private_info * uspi,
count = size >> uspi->s_fshift;
if (count > UFS_MAXFRAG)
return NULL;
- ubh = (struct ufs_buffer_head *)
- kmalloc (sizeof (struct ufs_buffer_head), GFP_KERNEL);
+ ubh = kmalloc(sizeof(struct ufs_buffer_head), GFP_KERNEL);
if (!ubh)
return NULL;
ubh->fragment = fragment;
--
1.7.1.244.gbdc4

2010-06-08 03:51:27

by Joe Perches

[permalink] [raw]
Subject: [PATCH 11/12] drivers/usb/storage/isd200.c: Remove unnecessary kmalloc casts

Signed-off-by: Joe Perches <[email protected]>
---
drivers/usb/storage/isd200.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
index e9cbc14..6b9982c 100644
--- a/drivers/usb/storage/isd200.c
+++ b/drivers/usb/storage/isd200.c
@@ -1456,8 +1456,7 @@ static int isd200_init_info(struct us_data *us)
int retStatus = ISD200_GOOD;
struct isd200_info *info;

- info = (struct isd200_info *)
- kzalloc(sizeof(struct isd200_info), GFP_KERNEL);
+ info = kzalloc(sizeof(struct isd200_info), GFP_KERNEL);
if (!info)
retStatus = ISD200_ERROR;
else {
--
1.7.1.244.gbdc4

2010-06-08 03:51:48

by Joe Perches

[permalink] [raw]
Subject: [PATCH 09/12] drivers/s390: Remove unnecessary kmalloc casts

Signed-off-by: Joe Perches <[email protected]>
---
drivers/s390/block/dasd_devmap.c | 3 +--
drivers/s390/char/con3215.c | 4 ++--
2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/s390/block/dasd_devmap.c b/drivers/s390/block/dasd_devmap.c
index 34d51dd..8c962dc 100644
--- a/drivers/s390/block/dasd_devmap.c
+++ b/drivers/s390/block/dasd_devmap.c
@@ -409,8 +409,7 @@ dasd_add_busid(const char *bus_id, int features)
struct dasd_devmap *devmap, *new, *tmp;
int hash;

- new = (struct dasd_devmap *)
- kzalloc(sizeof(struct dasd_devmap), GFP_KERNEL);
+ new = kzalloc(sizeof(struct dasd_devmap), GFP_KERNEL);
if (!new)
return ERR_PTR(-ENOMEM);
spin_lock(&dasd_devmap_lock);
diff --git a/drivers/s390/char/con3215.c b/drivers/s390/char/con3215.c
index 59ec073..6752c42 100644
--- a/drivers/s390/char/con3215.c
+++ b/drivers/s390/char/con3215.c
@@ -888,8 +888,8 @@ static int __init con3215_init(void)
if (IS_ERR(cdev))
return -ENODEV;

- raw3215[0] = raw = (struct raw3215_info *)
- kzalloc(sizeof(struct raw3215_info), GFP_KERNEL | GFP_DMA);
+ raw3215[0] = raw = kzalloc(sizeof(struct raw3215_info),
+ GFP_KERNEL | GFP_DMA);
raw->buffer = kzalloc(RAW3215_BUFFER_SIZE, GFP_KERNEL | GFP_DMA);
raw->inbuf = kzalloc(RAW3215_INBUF_SIZE, GFP_KERNEL | GFP_DMA);
raw->cdev = cdev;
--
1.7.1.244.gbdc4

2010-06-08 03:50:49

by Joe Perches

[permalink] [raw]
Subject: [PATCH 01/12] arch/cris: Remove unnecessary kmalloc casts

And separate declaration from allocation
Still no error checking on failure, but it probably doesn't matter.

Signed-off-by: Joe Perches <[email protected]>
---
arch/cris/arch-v32/mm/intmem.c | 13 +++++--------
1 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/arch/cris/arch-v32/mm/intmem.c b/arch/cris/arch-v32/mm/intmem.c
index 9e8b69c..1b17d92 100644
--- a/arch/cris/arch-v32/mm/intmem.c
+++ b/arch/cris/arch-v32/mm/intmem.c
@@ -33,8 +33,8 @@ static void crisv32_intmem_init(void)
{
static int initiated = 0;
if (!initiated) {
- struct intmem_allocation* alloc =
- (struct intmem_allocation*)kmalloc(sizeof *alloc, GFP_KERNEL);
+ struct intmem_allocation* alloc;
+ alloc = kmalloc(sizeof *alloc, GFP_KERNEL);
INIT_LIST_HEAD(&intmem_allocations);
intmem_virtual = ioremap(MEM_INTMEM_START + RESERVED_SIZE,
MEM_INTMEM_SIZE - RESERVED_SIZE);
@@ -62,9 +62,8 @@ void* crisv32_intmem_alloc(unsigned size, unsigned align)
if (allocation->status == STATUS_FREE &&
allocation->size >= size + alignment) {
if (allocation->size > size + alignment) {
- struct intmem_allocation* alloc =
- (struct intmem_allocation*)
- kmalloc(sizeof *alloc, GFP_ATOMIC);
+ struct intmem_allocation* alloc;
+ alloc = kmalloc(sizeof *alloc, GFP_ATOMIC);
alloc->status = STATUS_FREE;
alloc->size = allocation->size - size -
alignment;
@@ -74,9 +73,7 @@ void* crisv32_intmem_alloc(unsigned size, unsigned align)

if (alignment) {
struct intmem_allocation *tmp;
- tmp = (struct intmem_allocation *)
- kmalloc(sizeof *tmp,
- GFP_ATOMIC);
+ tmp = kmalloc(sizeof *tmp, GFP_ATOMIC);
tmp->offset = allocation->offset;
tmp->size = alignment;
tmp->status = STATUS_FREE;
--
1.7.1.244.gbdc4

2010-06-08 03:52:12

by Joe Perches

[permalink] [raw]
Subject: [PATCH 08/12] drivers/parisc/iosapic.c: Remove unnecessary kzalloc cast

Convert kzalloc to kcalloc

Signed-off-by: Joe Perches <[email protected]>
---
drivers/parisc/iosapic.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/parisc/iosapic.c b/drivers/parisc/iosapic.c
index c768367..3e6f56c 100644
--- a/drivers/parisc/iosapic.c
+++ b/drivers/parisc/iosapic.c
@@ -891,8 +891,8 @@ void *iosapic_register(unsigned long hpa)
isi->isi_version = iosapic_rd_version(isi);
isi->isi_num_vectors = IOSAPIC_IRDT_MAX_ENTRY(isi->isi_version) + 1;

- vip = isi->isi_vector = (struct vector_info *)
- kzalloc(sizeof(struct vector_info) * isi->isi_num_vectors, GFP_KERNEL);
+ vip = isi->isi_vector = kcalloc(isi->isi_num_vectors,
+ sizeof(struct vector_info), GFP_KERNEL);
if (vip == NULL) {
kfree(isi);
return NULL;
--
1.7.1.244.gbdc4

2010-06-08 03:52:14

by Joe Perches

[permalink] [raw]
Subject: [PATCH 06/12] drivers/media: Remove unnecesary kmalloc casts

Signed-off-by: Joe Perches <[email protected]>
---
drivers/media/dvb/siano/smscoreapi.c | 4 +---
drivers/media/video/w9968cf.c | 3 +--
2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/media/dvb/siano/smscoreapi.c b/drivers/media/dvb/siano/smscoreapi.c
index 0c87a3c..828bcc2 100644
--- a/drivers/media/dvb/siano/smscoreapi.c
+++ b/drivers/media/dvb/siano/smscoreapi.c
@@ -116,9 +116,7 @@ static struct smscore_registry_entry_t *smscore_find_registry(char *devpath)
return entry;
}
}
- entry = (struct smscore_registry_entry_t *)
- kmalloc(sizeof(struct smscore_registry_entry_t),
- GFP_KERNEL);
+ entry = kmalloc(sizeof(struct smscore_registry_entry_t), GFP_KERNEL);
if (entry) {
entry->mode = default_mode;
strcpy(entry->devpath, devpath);
diff --git a/drivers/media/video/w9968cf.c b/drivers/media/video/w9968cf.c
index d807eea..591fc6d 100644
--- a/drivers/media/video/w9968cf.c
+++ b/drivers/media/video/w9968cf.c
@@ -3429,8 +3429,7 @@ w9968cf_usb_probe(struct usb_interface* intf, const struct usb_device_id* id)
else
return -ENODEV;

- cam = (struct w9968cf_device*)
- kzalloc(sizeof(struct w9968cf_device), GFP_KERNEL);
+ cam = kzalloc(sizeof(struct w9968cf_device), GFP_KERNEL);
if (!cam)
return -ENOMEM;

--
1.7.1.244.gbdc4

2010-06-08 03:52:10

by Joe Perches

[permalink] [raw]
Subject: [PATCH 07/12] drivers/message/i2o/i20_config.c: Remove unnecessary kmalloc casts

Signed-off-by: Joe Perches <[email protected]>
---
drivers/message/i2o/i2o_config.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/message/i2o/i2o_config.c b/drivers/message/i2o/i2o_config.c
index c4b117f..3be46c5 100644
--- a/drivers/message/i2o/i2o_config.c
+++ b/drivers/message/i2o/i2o_config.c
@@ -1041,8 +1041,7 @@ static long i2o_cfg_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)

static int cfg_open(struct inode *inode, struct file *file)
{
- struct i2o_cfg_info *tmp =
- (struct i2o_cfg_info *)kmalloc(sizeof(struct i2o_cfg_info),
+ struct i2o_cfg_info *tmp = kmalloc(sizeof(struct i2o_cfg_info),
GFP_KERNEL);
unsigned long flags;

--
1.7.1.244.gbdc4

2010-06-08 03:52:56

by Joe Perches

[permalink] [raw]
Subject: [PATCH 03/12] arch/x86/kernel/tlb_uv.c: Remove unnecessary kmalloc casts

And convert a kmalloc/memset to kzalloc

Signed-off-by: Joe Perches <[email protected]>
---
arch/x86/kernel/tlb_uv.c | 15 +++++++--------
1 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/tlb_uv.c b/arch/x86/kernel/tlb_uv.c
index 7fea555..4c0a5e8 100644
--- a/arch/x86/kernel/tlb_uv.c
+++ b/arch/x86/kernel/tlb_uv.c
@@ -1141,8 +1141,9 @@ uv_activation_descriptor_init(int node, int pnode)
* each bau_desc is 64 bytes; there are 8 (UV_ITEMS_PER_DESCRIPTOR)
* per cpu; and up to 32 (UV_ADP_SIZE) cpu's per uvhub
*/
- bau_desc = (struct bau_desc *)kmalloc_node(sizeof(struct bau_desc)*
- UV_ADP_SIZE*UV_ITEMS_PER_DESCRIPTOR, GFP_KERNEL, node);
+ bau_desc = kmalloc_node(sizeof(struct bau_desc) *
+ UV_ADP_SIZE * UV_ITEMS_PER_DESCRIPTOR,
+ GFP_KERNEL, node);
BUG_ON(!bau_desc);

pa = uv_gpa(bau_desc); /* need the real nasid*/
@@ -1200,9 +1201,9 @@ uv_payload_queue_init(int node, int pnode)
struct bau_payload_queue_entry *pqp_malloc;
struct bau_control *bcp;

- pqp = (struct bau_payload_queue_entry *) kmalloc_node(
- (DEST_Q_SIZE + 1) * sizeof(struct bau_payload_queue_entry),
- GFP_KERNEL, node);
+ pqp = kmalloc_node((DEST_Q_SIZE + 1) *
+ sizeof(struct bau_payload_queue_entry),
+ GFP_KERNEL, node);
BUG_ON(!pqp);
pqp_malloc = pqp;

@@ -1286,9 +1287,7 @@ static void uv_init_per_cpu(int nuvhubs)
};
struct uvhub_desc *uvhub_descs;

- uvhub_descs = (struct uvhub_desc *)
- kmalloc(nuvhubs * sizeof(struct uvhub_desc), GFP_KERNEL);
- memset(uvhub_descs, 0, nuvhubs * sizeof(struct uvhub_desc));
+ uvhub_descs = kzalloc(nuvhubs * sizeof(struct uvhub_desc), GFP_KERNEL);
for_each_present_cpu(cpu) {
bcp = &per_cpu(bau_control, cpu);
memset(bcp, 0, sizeof(struct bau_control));
--
1.7.1.244.gbdc4

2010-06-08 03:53:12

by Joe Perches

[permalink] [raw]
Subject: [PATCH 02/12] arch/powerpc: Remove unnecessary kmalloc casts

Signed-off-by: Joe Perches <[email protected]>
---
arch/powerpc/kernel/nvram_64.c | 3 +--
arch/powerpc/kvm/book3s.c | 4 ++--
2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
index 9cf197f..99c6dab 100644
--- a/arch/powerpc/kernel/nvram_64.c
+++ b/arch/powerpc/kernel/nvram_64.c
@@ -502,8 +502,7 @@ static int __init nvram_scan_partitions(void)
"detected: 0-length partition\n");
goto out;
}
- tmp_part = (struct nvram_partition *)
- kmalloc(sizeof(struct nvram_partition), GFP_KERNEL);
+ tmp_part = kmalloc(sizeof(struct nvram_partition), GFP_KERNEL);
err = -ENOMEM;
if (!tmp_part) {
printk(KERN_ERR "nvram_scan_partitions: kmalloc failed\n");
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index b998abf..2a8b9b6 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -1248,8 +1248,8 @@ struct kvm_vcpu *kvmppc_core_vcpu_create(struct kvm *kvm, unsigned int id)

memset(vcpu_book3s, 0, sizeof(struct kvmppc_vcpu_book3s));

- vcpu_book3s->shadow_vcpu = (struct kvmppc_book3s_shadow_vcpu *)
- kzalloc(sizeof(*vcpu_book3s->shadow_vcpu), GFP_KERNEL);
+ vcpu_book3s->shadow_vcpu = kzalloc(sizeof(*vcpu_book3s->shadow_vcpu),
+ GFP_KERNEL);
if (!vcpu_book3s->shadow_vcpu)
goto free_vcpu;

--
1.7.1.244.gbdc4

2010-06-08 04:45:00

by David Rientjes

[permalink] [raw]
Subject: Re: [PATCH 01/12] arch/cris: Remove unnecessary kmalloc casts

On Mon, 7 Jun 2010, Joe Perches wrote:

> And separate declaration from allocation
> Still no error checking on failure, but it probably doesn't matter.
>
> Signed-off-by: Joe Perches <[email protected]>
> ---
> arch/cris/arch-v32/mm/intmem.c | 13 +++++--------
> 1 files changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/arch/cris/arch-v32/mm/intmem.c b/arch/cris/arch-v32/mm/intmem.c
> index 9e8b69c..1b17d92 100644
> --- a/arch/cris/arch-v32/mm/intmem.c
> +++ b/arch/cris/arch-v32/mm/intmem.c
> @@ -33,8 +33,8 @@ static void crisv32_intmem_init(void)
> {
> static int initiated = 0;
> if (!initiated) {
> - struct intmem_allocation* alloc =
> - (struct intmem_allocation*)kmalloc(sizeof *alloc, GFP_KERNEL);
> + struct intmem_allocation* alloc;
> + alloc = kmalloc(sizeof *alloc, GFP_KERNEL);
> INIT_LIST_HEAD(&intmem_allocations);
> intmem_virtual = ioremap(MEM_INTMEM_START + RESERVED_SIZE,
> MEM_INTMEM_SIZE - RESERVED_SIZE);
> @@ -62,9 +62,8 @@ void* crisv32_intmem_alloc(unsigned size, unsigned align)
> if (allocation->status == STATUS_FREE &&
> allocation->size >= size + alignment) {
> if (allocation->size > size + alignment) {
> - struct intmem_allocation* alloc =
> - (struct intmem_allocation*)
> - kmalloc(sizeof *alloc, GFP_ATOMIC);
> + struct intmem_allocation* alloc;
> + alloc = kmalloc(sizeof *alloc, GFP_ATOMIC);
> alloc->status = STATUS_FREE;
> alloc->size = allocation->size - size -
> alignment;

Why not fix the checkpatch failures at the same time?

ERROR: "foo* bar" should be "foo *bar"
#26: FILE: arch/cris/arch-v32/mm/intmem.c:36:
+ struct intmem_allocation* alloc;

ERROR: "foo* bar" should be "foo *bar"
#38: FILE: arch/cris/arch-v32/mm/intmem.c:65:
+ struct intmem_allocation* alloc;

2010-06-08 15:21:11

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 01/12] arch/cris: Remove unnecessary kmalloc casts

On Mon, 2010-06-07 at 21:44 -0700, David Rientjes wrote:
> On Mon, 7 Jun 2010, Joe Perches wrote:
> > And separate declaration from allocation
> > Still no error checking on failure, but it probably doesn't matter.
[]
> Why not fix the checkpatch failures at the same time?

I think that's better done in a separate pass.

2010-06-09 07:20:13

by Milton Miller

[permalink] [raw]
Subject: Re: [PATCH 00/12] treewide: Remove unnecessary kmalloc casts


On Mon Jun 07 2010 at around 23:53:18 EST, Joe Perches wrote:
> Trivial cleanups
>
> arch/cris: Remove unnecessary kmalloc casts
> arch/powerpc: Remove unnecessary kmalloc casts
> arch/x86/kernel/tlb_uv.c: Remove unnecessary kmalloc casts
> drivers/gpu/drm: Remove unnecessary kmalloc casts
> drivers/isdn/capi/capidrv.c: Remove unnecessary kmalloc casts
> drivers/media: Remove unnecesary kmalloc casts
> drivers/message/i2o/i20_config.c: Remove unnecessary kmalloc casts
> drivers/parisc/iosapic.c: Remove unnecessary kzalloc cast
> drivers/s390: Remove unnecessary kmalloc casts
> drivers/serial/icom.c: Remove unnecessary kmalloc casts
> drivers/usb/storage/isd200.c: Remove unnecessary kmalloc casts
> fs/ufs/util.c: Remove unnecessary kmalloc casts
>

Joe,

Thanks for doing cleanups.

However, in this case you are removing casts that, while not necessary
for C, are indeed there for a reason.

Specifically, they are of the form
type *p;
<code>
p = (type *)kmalloc(sizeof(type), ...);

For example, from the powerpc patch:
> goto out;
> }
> - tmp_part = (struct nvram_partition *)
> - kmalloc(sizeof(struct nvram_partition), GFP_KERNEL);
> + tmp_part = kmalloc(sizeof(struct nvram_partition), GFP_KERNEL);
> err = -ENOMEM;

The reason they casts are present is to guard against someone changing
the type of p at the top of the function and not changing the type at
the kmalloc.

This was discussed some years ago, possibly around the time kcalloc
got its underflow detection.

Should we create a kmalloc wrapper that enforces this type saftey?
While we have added static analysis tools that can check these things,
and we have slab redzoning options, they tend to be run in batch by
"someone else" or in response to debugging a problem.


There may have been discussion of doing the above vs
p = kmalloc(sizeof(*p), ...);

I don't remember if it was programmer preference or issues when p is
an array type.


I am not subscribed so I don't have the full list that you sent
these to, and I know some maintainers have already taken some of
the series the last time you posted; could you please augment the
CC list.

thanks,
milton

2010-06-09 07:46:27

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 00/12] treewide: Remove unnecessary kmalloc casts

On Wed, 2010-06-09 at 02:20 -0500, Milton Miller wrote:
> On Mon Jun 07 2010 at around 23:53:18 EST, Joe Perches wrote:
> > Trivial cleanups
> The reason they casts are present is to guard against someone changing
> the type of p at the top of the function and not changing the type at
> the kmalloc.

That'd be true for most all of the kernel mallocs.

> I know some maintainers have already taken some of
> the series the last time you posted; could you please augment the
> CC list.

I did not send to trivial any of kmalloc cast removal patches that
were acked. The patches in this series were not acked, so I think
no CC list needs augmentation.

Original: http://lkml.org/lkml/2010/5/31/471
Newest: http://lkml.org/lkml/2010/6/7/428

cheers, Joe

2010-06-09 10:07:04

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH 00/12] treewide: Remove unnecessary kmalloc casts

Hi Milton,

On Wed, Jun 9, 2010 at 10:20 AM, Milton Miller <[email protected]> wrote:
> However, in this case you are removing casts that, while not necessary
> for C, are indeed there for a reason.
>
> Specifically, they are of the form
> ? type *p;
> ? <code>
> ? p = (type *)kmalloc(sizeof(type), ...);
>
> For example, from the powerpc patch:
>> goto out;
>> }
>> - tmp_part = (struct nvram_partition *)
>> - kmalloc(sizeof(struct nvram_partition), GFP_KERNEL);
>> + tmp_part = kmalloc(sizeof(struct nvram_partition), GFP_KERNEL);
>> err = -ENOMEM;
>
> The reason they casts are present is to guard against someone changing
> the type of p at the top of the function and not changing the type at
> the kmalloc.

If you're worried about that...

[snip]

> There may have been discussion of doing the above vs
> ? p = kmalloc(sizeof(*p), ...);

...it's better to use this form. There's actually a mention of this in
"Chapter 14: Allocating memory" of Documentation/CodingStyle. The
guard is not really a guard as someone can still change the "sizeof"
part to something else and the cast from "void *" will silently ignore
it.

Pekka

2010-06-14 11:04:14

by Jesper Nilsson

[permalink] [raw]
Subject: Re: [PATCH 01/12] arch/cris: Remove unnecessary kmalloc casts

On Tue, Jun 08, 2010 at 05:50:33AM +0200, Joe Perches wrote:
> And separate declaration from allocation
> Still no error checking on failure, but it probably doesn't matter.

Acked-by: Jesper Nilsson <[email protected]>

> Signed-off-by: Joe Perches <[email protected]>
> ---
> arch/cris/arch-v32/mm/intmem.c | 13 +++++--------
> 1 files changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/arch/cris/arch-v32/mm/intmem.c b/arch/cris/arch-v32/mm/intmem.c
> index 9e8b69c..1b17d92 100644
> --- a/arch/cris/arch-v32/mm/intmem.c
> +++ b/arch/cris/arch-v32/mm/intmem.c
> @@ -33,8 +33,8 @@ static void crisv32_intmem_init(void)
> {
> static int initiated = 0;
> if (!initiated) {
> - struct intmem_allocation* alloc =
> - (struct intmem_allocation*)kmalloc(sizeof *alloc, GFP_KERNEL);
> + struct intmem_allocation* alloc;
> + alloc = kmalloc(sizeof *alloc, GFP_KERNEL);
> INIT_LIST_HEAD(&intmem_allocations);
> intmem_virtual = ioremap(MEM_INTMEM_START + RESERVED_SIZE,
> MEM_INTMEM_SIZE - RESERVED_SIZE);
> @@ -62,9 +62,8 @@ void* crisv32_intmem_alloc(unsigned size, unsigned align)
> if (allocation->status == STATUS_FREE &&
> allocation->size >= size + alignment) {
> if (allocation->size > size + alignment) {
> - struct intmem_allocation* alloc =
> - (struct intmem_allocation*)
> - kmalloc(sizeof *alloc, GFP_ATOMIC);
> + struct intmem_allocation* alloc;
> + alloc = kmalloc(sizeof *alloc, GFP_ATOMIC);
> alloc->status = STATUS_FREE;
> alloc->size = allocation->size - size -
> alignment;
> @@ -74,9 +73,7 @@ void* crisv32_intmem_alloc(unsigned size, unsigned align)
>
> if (alignment) {
> struct intmem_allocation *tmp;
> - tmp = (struct intmem_allocation *)
> - kmalloc(sizeof *tmp,
> - GFP_ATOMIC);
> + tmp = kmalloc(sizeof *tmp, GFP_ATOMIC);
> tmp->offset = allocation->offset;
> tmp->size = alignment;
> tmp->status = STATUS_FREE;
> --
> 1.7.1.244.gbdc4
>
> --
> 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/
/^JN - Jesper Nilsson
--
Jesper Nilsson -- [email protected]

2010-06-16 15:58:06

by Jiri Kosina

[permalink] [raw]
Subject: Re: [PATCH 01/12] arch/cris: Remove unnecessary kmalloc casts

On Mon, 14 Jun 2010, Jesper Nilsson wrote:

> On Tue, Jun 08, 2010 at 05:50:33AM +0200, Joe Perches wrote:
> > And separate declaration from allocation
> > Still no error checking on failure, but it probably doesn't matter.
>
> Acked-by: Jesper Nilsson <[email protected]>

Applied, thanks.

--
Jiri Kosina
SUSE Labs, Novell Inc.