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 <[email protected]>
---
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
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 <[email protected]>
---
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
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
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 <[email protected]>
---
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
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 ?
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?
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 :)