Messages in this thread | | | Date | Wed, 19 Jul 2006 14:53:31 +0100 | From | Paulo Marques <> | Subject | Re: [PATCH] drivers: Conversions from kmalloc+memset to k(z|c)alloc. |
| |
Panagiotis Issaris wrote: > [...] > --- a/drivers/char/consolemap.c > +++ b/drivers/char/consolemap.c > @@ -192,11 +192,9 @@ static void set_inverse_transl(struct vc > q = p->inverse_translations[i]; > > if (!q) { > - q = p->inverse_translations[i] = (unsigned char *) > - kmalloc(MAX_GLYPH, GFP_KERNEL); > + q = p->inverse_translations[i] = kzalloc(MAX_GLYPH, GFP_KERNEL); > if (!q) return; > } > - memset(q, 0, MAX_GLYPH);
This changes semantics here. Before, the data pointed by q was always cleared whether it was malloc'ed or not. Now it is only cleared if it is malloc'ed. I haven't checked the code to find out if this is ok, though.
> @@ -2704,8 +2702,7 @@ static void cardmap_set(struct cardmap * > if (p == NULL || (nr >> p->shift) >= CARDMAP_WIDTH) { > do { > /* need a new top level */ > - struct cardmap *np = kmalloc(sizeof(*np), GFP_KERNEL); > - memset(np, 0, sizeof(*np)); > + struct cardmap *np = kzalloc(sizeof(*np), GFP_KERNEL); > np->ptr[0] = p; > if (p != NULL) { > np->shift = p->shift + CARDMAP_ORDER; > @@ -2719,8 +2716,7 @@ static void cardmap_set(struct cardmap * > while (p->shift > 0) { > i = (nr >> p->shift) & CARDMAP_MASK; > if (p->ptr[i] == NULL) { > - struct cardmap *np = kmalloc(sizeof(*np), GFP_KERNEL); > - memset(np, 0, sizeof(*np)); > + struct cardmap *np = kzalloc(sizeof(*np), GFP_KERNEL); > np->shift = p->shift - CARDMAP_ORDER; > np->parent = p; > p->ptr[i] = np;
This is not your fault, but this code is using the return value from kmalloc (or kzalloc, now) without checking for NULL.
> diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c > index 76edbb6..71d4743 100644 > --- a/drivers/scsi/megaraid.c > +++ b/drivers/scsi/megaraid.c > @@ -4487,8 +4487,7 @@ mega_internal_command(adapter_t *adapter > scmd = &adapter->int_scmd; > memset(scmd, 0, sizeof(Scsi_Cmnd)); > > - sdev = kmalloc(sizeof(struct scsi_device), GFP_KERNEL); > - memset(sdev, 0, sizeof(struct scsi_device)); > + sdev = kzalloc(sizeof(struct scsi_device), GFP_KERNEL); > scmd->device = sdev; > > scmd->device->host = adapter->host;
Same here.
> --- a/drivers/video/offb.c > +++ b/drivers/video/offb.c > @@ -376,13 +376,12 @@ static void __init offb_init_fb(const ch > > size = sizeof(struct fb_info) + sizeof(u32) * 17; > > - info = kmalloc(size, GFP_ATOMIC); > + info = kzalloc(size, GFP_ATOMIC); > > if (info == 0) {
Again, not your fault, but "info == 0"? If you're doing a new version of the patch, please change this to NULL or !info, so that we don't confuse human readers :)
-- Paulo Marques - www.grupopie.com
"I can picture in my mind a world without war, a world without hate. And I can picture us attacking that world, because they'd never expect it." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |