Messages in this thread Patch in this message | | | Date | Tue, 08 Mar 2005 13:20:29 +0000 | From | Paulo Marques <> | Subject | Re: sizeof(ptr) or sizeof(*ptr)? |
| |
Andrew Morton wrote: > "" <pmarques@grupopie.com> wrote: > >>Anyway, after improving the tool and checking for false positives, there is only >> one more suspicious piece of code in drivers/acpi/video.c:561 >> >> status = acpi_video_device_lcd_query_levels(device, &obj); >> >> if (obj && obj->type == ACPI_TYPE_PACKAGE && obj->package.count >= 2) { >> int count = 0; >> union acpi_object *o; >> >> br = kmalloc(sizeof &br, GFP_KERNEL); > > > yup, bug. > > >> if (!br) { >> printk(KERN_ERR "can't allocate memory\n"); >> } else { >> memset(br, 0, sizeof &br); >> br->levels = kmalloc(obj->package.count * sizeof &br->levels, GFP_KERNEL); > > > And another one, although it happens to work out OK. > > I'll get these all fixed up, thanks.
I just checked the 2.6.11-mm1 release and this is only half-fixed there, and it is the worst of both halves: the kmalloc only mallocs the size of a pointer, but the memset is fixed, so it memset's the size of a structure (oops). This is partially my fault for not sending the patch in the first place, together with the bug report.
The attached patch against 2.6.11-mm1 should fix the kmalloc.
By the way, I haven't got any response from an alsa developer about the bug in sound/core/control.c, but this is already fixed in 2.6.11-mm1, along with several other changes to that file. So the status is: it was a bug, but it is already fixed :)
-- Paulo Marques - www.grupopie.com
All that is necessary for the triumph of evil is that good men do nothing. Edmund Burke (1729 - 1797) --- ./drivers/acpi/video.c.orig 2005-03-08 13:07:42.000000000 +0000 +++ ./drivers/acpi/video.c 2005-03-08 13:09:05.000000000 +0000 @@ -564,11 +564,11 @@ acpi_video_device_find_cap (struct acpi_ int count = 0; union acpi_object *o; - br = kmalloc(sizeof &br, GFP_KERNEL); + br = kmalloc(sizeof(*br), GFP_KERNEL); if (!br) { printk(KERN_ERR "can't allocate memory\n"); } else { - memset(br, 0, sizeof *br); + memset(br, 0, sizeof(*br)); br->levels = kmalloc(obj->package.count * sizeof *(br->levels), GFP_KERNEL); if (!br->levels) | |