Re: sizeof(ptr) or sizeof(*ptr)?

[Paulo Marques <pmarques@grupopie.com>]

[-- Attachment #1: Type: text/plain, Size: 1560 bytes --]

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)

[-- Attachment #2: acpipatch --]
[-- Type: text/plain, Size: 569 bytes --]

--- ./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)