[PATCH] ACPI fan: fix memleak on acpi_fan_add_fs() failure

[PATCH] ACPI fan: fix memleak on acpi_fan_add_fs() failure

[Alan Jenkins]

Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
---
 drivers/acpi/fan.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
index 947556e..eb1511e 100644
--- a/drivers/acpi/fan.c
+++ b/drivers/acpi/fan.c
@@ -283,13 +283,17 @@ static int acpi_fan_add(struct acpi_device *device)
 
 	result = acpi_fan_add_fs(device);
 	if (result)
-		goto end;
+		goto unregister;
 
 	printk(KERN_INFO PREFIX "%s [%s] (%s)\n",
 	       acpi_device_name(device), acpi_device_bid(device),
 	       !device->power.state ? "on" : "off");
 
-      end:
+unregister:
+	sysfs_remove_link(&device->dev.kobj, "thermal_cooling");
+	sysfs_remove_link(&cdev->device.kobj, "device");
+	thermal_cooling_device_unregister(cdev);
+end:
 	return result;
 }
 

Re: [PATCH] ACPI fan: fix memleak on acpi_fan_add_fs() failure

[ykzhao]

On Sun, 2009-09-13 at 18:28 +0800, Alan Jenkins wrote:
> Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
It sounds reasonable although we don't fail in calling the function of
acpi_fan_add_fs.

> ---
>  drivers/acpi/fan.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
> index 947556e..eb1511e 100644
> --- a/drivers/acpi/fan.c
> +++ b/drivers/acpi/fan.c
> @@ -283,13 +283,17 @@ static int acpi_fan_add(struct acpi_device *device)
>  
>  	result = acpi_fan_add_fs(device);
>  	if (result)
> -		goto end;
> +		goto unregister;
>  
>  	printk(KERN_INFO PREFIX "%s [%s] (%s)\n",
>  	       acpi_device_name(device), acpi_device_bid(device),
>  	       !device->power.state ? "on" : "off");

At the same time we should return directly if the result is zero. 
But in this patch it seems that it will also remove the sysfs I/F link
when the result is zero.

Thanks.
>  
> -      end:
> +unregister:
> +	sysfs_remove_link(&device->dev.kobj, "thermal_cooling");
> +	sysfs_remove_link(&cdev->device.kobj, "device");
> +	thermal_cooling_device_unregister(cdev);
> +end:
>  	return result;
>  }
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ACPI fan: fix memleak on acpi_fan_add_fs() failure

[Alan Jenkins]

ykzhao wrote:
> On Sun, 2009-09-13 at 18:28 +0800, Alan Jenkins wrote:
>   
>> Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
>>     
> It sounds reasonable although we don't fail in calling the function of
> acpi_fan_add_fs.
>   

Agreed, this applies to ENOMEM or other weird internal errors only.

>> ---
>>  drivers/acpi/fan.c |    8 ++++++--
>>  1 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
>> index 947556e..eb1511e 100644
>> --- a/drivers/acpi/fan.c
>> +++ b/drivers/acpi/fan.c
>> @@ -283,13 +283,17 @@ static int acpi_fan_add(struct acpi_device *device)
>>  
>>  	result = acpi_fan_add_fs(device);
>>  	if (result)
>> -		goto end;
>> +		goto unregister;
>>  
>>  	printk(KERN_INFO PREFIX "%s [%s] (%s)\n",
>>  	       acpi_device_name(device), acpi_device_bid(device),
>>  	       !device->power.state ? "on" : "off");
>>     
>
> At the same time we should return directly if the result is zero. 
> But in this patch it seems that it will also remove the sysfs I/F link
> when the result is zero.
>
> Thanks.
>   

Eek, thanks for pointing this out.  I'll send a fixed version
Alan