[PATCH 1/4] make cdev attribuition the last step

[PATCH 1/4] make cdev attribuition the last step

[Glauber Costa]

This patch uses a temporary variable "cdev" instead of using
directly pr->cdev. Through it, we can tell later whether or not
this code was completed properly: by checking for pr->cdev != NULL
Signed-off-by: Glauber Costa <gcosta@redhat.com>
---
 drivers/acpi/processor_core.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index 75ccf5d..9480203 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -622,7 +622,7 @@ static int __cpuinit acpi_processor_star
 	int result = 0;
 	acpi_status status = AE_OK;
 	struct acpi_processor *pr;
-
+	struct thermal_cooling_device *cdev;
 
 	pr = acpi_driver_data(device);
 
@@ -668,24 +668,26 @@ #endif
 
 	acpi_processor_power_init(pr, device);
 
-	pr->cdev = thermal_cooling_device_register("Processor", device,
+	cdev = thermal_cooling_device_register("Processor", device,
 						&processor_cooling_ops);
-	if (pr->cdev)
+	if (cdev)
 		printk(KERN_INFO PREFIX
 			"%s is registered as cooling_device%d\n",
-			device->dev.bus_id, pr->cdev->id);
+			device->dev.bus_id, cdev->id);
 	else
 		goto end;
 
-	result = sysfs_create_link(&device->dev.kobj, &pr->cdev->device.kobj,
+	result = sysfs_create_link(&device->dev.kobj, &cdev->device.kobj,
 					"thermal_cooling");
 	if (result)
 		return result;
-	result = sysfs_create_link(&pr->cdev->device.kobj, &device->dev.kobj,
+	result = sysfs_create_link(&cdev->device.kobj, &device->dev.kobj,
 					"device");
 	if (result)
 		return result;
 
+	pr->cdev = cdev;
+
 	if (pr->flags.throttling) {
 		printk(KERN_INFO PREFIX "%s [%s] (supports",
 		       acpi_device_name(device), acpi_device_bid(device));
-- 
1.4.2

[PATCH 2/4] use pr->cdev as a condition for cleanup

[Glauber Costa]

The acpi_processor_start() function can fail before creating the
sysfs nodes for thermal cooling. In this case, pr->cdev will be NULL,
and the removal code will break.

This patch uses pr->cdev as a criteria for termination of the
mentioned code, and avoids it, if it was never initialized.

Signed-off-by: Glauber Costa <gcosta@redhat.com>
---
 drivers/acpi/processor_core.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index 9480203..5023410 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -811,10 +811,12 @@ static int acpi_processor_remove(struct 
 
 	acpi_processor_remove_fs(device);
 
-	sysfs_remove_link(&device->dev.kobj, "thermal_cooling");
-	sysfs_remove_link(&pr->cdev->device.kobj, "device");
-	thermal_cooling_device_unregister(pr->cdev);
-	pr->cdev = NULL;
+	if (pr->cdev) {
+		sysfs_remove_link(&device->dev.kobj, "thermal_cooling");
+		sysfs_remove_link(&pr->cdev->device.kobj, "device");
+		thermal_cooling_device_unregister(pr->cdev);
+		pr->cdev = NULL;
+	}
 
 	processors[pr->id] = NULL;
 
-- 
1.4.2

[PATCH 3/4] provide error handling for unclean objects

[Glauber Costa]

Previously, if acpi_processor_start() failed to
create sysfs nodes, it would leave stuff to be cleaned,
that won't perish in acpi_processor_remove(),
since we'll have pr->cdev == NULL.

This patch cleans those objects.

Signed-off-by: Glauber Costa <gcosta@redhat.com>
---
 drivers/acpi/processor_core.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index 5023410..06a230a 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -680,11 +680,12 @@ #endif
 	result = sysfs_create_link(&device->dev.kobj, &cdev->device.kobj,
 					"thermal_cooling");
 	if (result)
-		return result;
+		goto err_thermal;
+
 	result = sysfs_create_link(&cdev->device.kobj, &device->dev.kobj,
 					"device");
 	if (result)
-		return result;
+		goto err_sysfs;
 
 	pr->cdev = cdev;
 
@@ -694,9 +695,13 @@ #endif
 		printk(" %d throttling states", pr->throttling.state_count);
 		printk(")\n");
 	}
+	return result;
 
-      end:
-
+err_sysfs:
+	sysfs_remove_link(&device->dev.kobj, "thermal_cooling");
+err_thermal:
+	thermal_cooling_device_unregister(pr->cdev);
+end:
 	return result;
 }
 
-- 
1.4.2

[PATCH 4/4] remove goto statement

[Glauber Costa]

This patch removes goto statements in favour of plain returns
in places that had nothing left behind that would justify
such construction
---
 drivers/acpi/processor_core.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index 06a230a..70f62b6 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -651,7 +651,7 @@ static int __cpuinit acpi_processor_star
 
 	result = acpi_processor_add_fs(device);
 	if (result)
-		goto end;
+		return result;
 
 	status = acpi_install_notify_handler(pr->handle, ACPI_DEVICE_NOTIFY,
 					     acpi_processor_notify, pr);
@@ -675,7 +675,7 @@ #endif
 			"%s is registered as cooling_device%d\n",
 			device->dev.bus_id, cdev->id);
 	else
-		goto end;
+		return result;
 
 	result = sysfs_create_link(&device->dev.kobj, &cdev->device.kobj,
 					"thermal_cooling");
-- 
1.4.2

Re: [PATCH 4/4] remove goto statement

[Li Zefan]

Glauber Costa 写道:
> This patch removes goto statements in favour of plain returns
> in places that had nothing left behind that would justify
> such construction
> ---
>  drivers/acpi/processor_core.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
> index 06a230a..70f62b6 100644
> --- a/drivers/acpi/processor_core.c
> +++ b/drivers/acpi/processor_core.c
> @@ -651,7 +651,7 @@ static int __cpuinit acpi_processor_star
>  
>  	result = acpi_processor_add_fs(device);
>  	if (result)
> -		goto end;
> +		return result;
>  
>  	status = acpi_install_notify_handler(pr->handle, ACPI_DEVICE_NOTIFY,
>  					     acpi_processor_notify, pr);
> @@ -675,7 +675,7 @@ #endif
>  			"%s is registered as cooling_device%d\n",
>  			device->dev.bus_id, cdev->id);
>  	else
> -		goto end;
> +		return result;
>  
>  	result = sysfs_create_link(&device->dev.kobj, &cdev->device.kobj,
>  					"thermal_cooling");

Seems you forgot to remove the 'end' label ?

Re: [PATCH 4/4] remove goto statement

[Glauber Costa]

Li Zefan wrote:
> Glauber Costa 写道:
>> This patch removes goto statements in favour of plain returns
>> in places that had nothing left behind that would justify
>> such construction
>> ---
>>  drivers/acpi/processor_core.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
>> index 06a230a..70f62b6 100644
>> --- a/drivers/acpi/processor_core.c
>> +++ b/drivers/acpi/processor_core.c
>> @@ -651,7 +651,7 @@ static int __cpuinit acpi_processor_star
>>  
>>  	result = acpi_processor_add_fs(device);
>>  	if (result)
>> -		goto end;
>> +		return result;
>>  
>>  	status = acpi_install_notify_handler(pr->handle, ACPI_DEVICE_NOTIFY,
>>  					     acpi_processor_notify, pr);
>> @@ -675,7 +675,7 @@ #endif
>>  			"%s is registered as cooling_device%d\n",
>>  			device->dev.bus_id, cdev->id);
>>  	else
>> -		goto end;
>> +		return result;
>>  
>>  	result = sysfs_create_link(&device->dev.kobj, &cdev->device.kobj,
>>  					"thermal_cooling");
> 
> Seems you forgot to remove the 'end' label ?
yes, in fact, thanks for pointing.

However, the patches are as split up as I could do, and it should not
affect the others. I can re send this one, the whole series, or
whatever, depending on what the maintainer wants.

So, what's gonna be?

Re: [PATCH 4/4] remove goto statement

[Li Zefan]

Glauber Costa wrote:
> Li Zefan wrote:
>> Glauber Costa 写道:
>>> This patch removes goto statements in favour of plain returns
>>> in places that had nothing left behind that would justify
>>> such construction
>>> ---
>>>  drivers/acpi/processor_core.c |    4 ++--
>>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
>>> index 06a230a..70f62b6 100644
>>> --- a/drivers/acpi/processor_core.c
>>> +++ b/drivers/acpi/processor_core.c
>>> @@ -651,7 +651,7 @@ static int __cpuinit acpi_processor_star
>>>  
>>>  	result = acpi_processor_add_fs(device);
>>>  	if (result)
>>> -		goto end;
>>> +		return result;
>>>  
>>>  	status = acpi_install_notify_handler(pr->handle, ACPI_DEVICE_NOTIFY,
>>>  					     acpi_processor_notify, pr);
>>> @@ -675,7 +675,7 @@ #endif
>>>  			"%s is registered as cooling_device%d\n",
>>>  			device->dev.bus_id, cdev->id);
>>>  	else
>>> -		goto end;
>>> +		return result;
>>>  
>>>  	result = sysfs_create_link(&device->dev.kobj, &cdev->device.kobj,
>>>  					"thermal_cooling");
>> Seems you forgot to remove the 'end' label ?
> yes, in fact, thanks for pointing.
> 
> However, the patches are as split up as I could do, and it should not
> affect the others. I can re send this one, the whole series, or
> whatever, depending on what the maintainer wants.
> 
> So, what's gonna be?
> 

IMO a revised [PATCH 4/4] will do, since it won't affect the other 3 patches :)