[PATCH 1/1] kernel-power: Deletion of an unnecessary check before the function call "vfree"

[PATCH 1/1] kernel-power: Deletion of an unnecessary check before the function call "vfree"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 16 Nov 2014 14:18:28 +0100

The vfree() function performs also input parameter validation. Thus the test
around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 kernel/power/swap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index aaa3261..337c7a9 100644
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -1374,7 +1374,7 @@ out_clean:
 				kthread_stop(data[thr].thr);
 		vfree(data);
 	}
-	if (page) vfree(page);
+	vfree(page);
 
 	return ret;
 }
-- 
2.1.3

Re: [PATCH 1/1] kernel-power: Deletion of an unnecessary check before the function call "vfree"

[Rafael J. Wysocki]

On Sunday, November 16, 2014 02:28:23 PM SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 16 Nov 2014 14:18:28 +0100
> 
> The vfree() function performs also input parameter validation. Thus the test
> around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Queued up for 3.19-rc1, thanks!

> ---
>  kernel/power/swap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/power/swap.c b/kernel/power/swap.c
> index aaa3261..337c7a9 100644
> --- a/kernel/power/swap.c
> +++ b/kernel/power/swap.c
> @@ -1374,7 +1374,7 @@ out_clean:
>  				kthread_stop(data[thr].thr);
>  		vfree(data);
>  	}
> -	if (page) vfree(page);
> +	vfree(page);
>  
>  	return ret;
>  }
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[PATCH 1/1] PM-wakeup: Deletion of an unnecessary check before the function call "wakeup_source_unregister"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 19 Nov 2014 12:21:24 +0100

The wakeup_source_unregister() function tests whether its argument is NULL
and then returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/base/power/wakeup.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index eb1bd2e..87dfc1d 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -267,8 +267,7 @@ int device_wakeup_disable(struct device *dev)
 		return -EINVAL;
 
 	ws = device_wakeup_detach(dev);
-	if (ws)
-		wakeup_source_unregister(ws);
+	wakeup_source_unregister(ws);
 
 	return 0;
 }
-- 
2.1.3

Re: [PATCH 1/1] PM-wakeup: Deletion of an unnecessary check before the function call "wakeup_source_unregister"

[Dan Carpenter]

On Wed, Nov 19, 2014 at 12:26:45PM +0100, SF Markus Elfring wrote:
> --- a/drivers/base/power/wakeup.c
> +++ b/drivers/base/power/wakeup.c
> @@ -267,8 +267,7 @@ int device_wakeup_disable(struct device *dev)
>  		return -EINVAL;
>  
>  	ws = device_wakeup_detach(dev);
> -	if (ws)
> -		wakeup_source_unregister(ws);
> +	wakeup_source_unregister(ws);

In the original code, it's clear that the programmer thought about what
happens when the device_wakeup_detach() returns NULL but in the new code
that's not clear.

I guess the information is still there in the git archive, but why hide
the good code by covering it with confusing code?

regards,
dan carpenter

Re: [PATCH 1/1] PM-wakeup: Deletion of an unnecessary check before the function call "wakeup_source_unregister"

[walter harms]

Am 19.11.2014 13:09, schrieb Dan Carpenter:
> On Wed, Nov 19, 2014 at 12:26:45PM +0100, SF Markus Elfring wrote:
>> --- a/drivers/base/power/wakeup.c
>> +++ b/drivers/base/power/wakeup.c
>> @@ -267,8 +267,7 @@ int device_wakeup_disable(struct device *dev)
>>  		return -EINVAL;
>>  
>>  	ws = device_wakeup_detach(dev);
>> -	if (ws)
>> -		wakeup_source_unregister(ws);
>> +	wakeup_source_unregister(ws);
> 
> In the original code, it's clear that the programmer thought about what
> happens when the device_wakeup_detach() returns NULL but in the new code
> that's not clear.
> 
> I guess the information is still there in the git archive, but why hide
> the good code by covering it with confusing code?
> 

hi,
just to add an other point of view ...

device_wakeup_detach returns dev->power.wakeup what is never NULL in this case.
(not visible here but a few line before exactly this is checked)

so this code can be compacted to:
wakeup_source_unregister(device_wakeup_detach(dev));

--readability

let the maintainer decide this byte-saving vs readability

re,
 wh

Re: [PATCH 1/1] PM-wakeup: Deletion of an unnecessary check before the function call "wakeup_source_unregister"

[Dan Carpenter]

On Wed, Nov 19, 2014 at 01:54:49PM +0100, walter harms wrote:
> device_wakeup_detach returns dev->power.wakeup what is never NULL in this case.
> (not visible here but a few line before exactly this is checked)

Huh?  I don't see a NULL check.

I think you may be confusing "dev->power.can_wakeup" with
"dev->power.wakeup"?

regards,
dan carpenter

Re: [PATCH 1/1] PM-wakeup: Deletion of an unnecessary check before the function call "wakeup_source_unregister"

[walter harms]

Am 19.11.2014 14:05, schrieb Dan Carpenter:
> On Wed, Nov 19, 2014 at 01:54:49PM +0100, walter harms wrote:
>> device_wakeup_detach returns dev->power.wakeup what is never NULL in this case.
>> (not visible here but a few line before exactly this is checked)
> 
> Huh?  I don't see a NULL check.
> 
> I think you may be confusing "dev->power.can_wakeup" with
> "dev->power.wakeup"?
> 
>

mea culpa,
you are right  dev->power.can_wakeup != dev->power.wakeup

therefore device_wakeup_detach(dev) CAN return NULL

re,
 wh

[PATCH 1/1] thermal: Exynos: Deletion of unnecessary checks before two function calls

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 21 Nov 2014 11:06:33 +0100

The functions cpufreq_cooling_unregister() and thermal_zone_device_unregister()
test whether their argument is NULL and then return immediately.
Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/thermal/samsung/exynos_thermal_common.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_thermal_common.c b/drivers/thermal/samsung/exynos_thermal_common.c
index 3f5ad25..1cbb9d0 100644
--- a/drivers/thermal/samsung/exynos_thermal_common.c
+++ b/drivers/thermal/samsung/exynos_thermal_common.c
@@ -417,12 +417,10 @@ void exynos_unregister_thermal(struct thermal_sensor_conf *sensor_conf)
 
 	th_zone = sensor_conf->pzone_data;
 
-	if (th_zone->therm_dev)
-		thermal_zone_device_unregister(th_zone->therm_dev);
+	thermal_zone_device_unregister(th_zone->therm_dev);
 
 	for (i = 0; i < th_zone->cool_dev_size; i++) {
-		if (th_zone->cool_dev[i])
-			cpufreq_cooling_unregister(th_zone->cool_dev[i]);
+		cpufreq_cooling_unregister(th_zone->cool_dev[i]);
 	}
 
 	dev_info(sensor_conf->dev,
-- 
2.1.3

Re: [PATCH 1/1] thermal: Exynos: Deletion of unnecessary checks before two function calls

[Julia Lawall]

On Fri, 21 Nov 2014, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 21 Nov 2014 11:06:33 +0100
>
> The functions cpufreq_cooling_unregister() and thermal_zone_device_unregister()
> test whether their argument is NULL and then return immediately.
> Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/thermal/samsung/exynos_thermal_common.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/thermal/samsung/exynos_thermal_common.c b/drivers/thermal/samsung/exynos_thermal_common.c
> index 3f5ad25..1cbb9d0 100644
> --- a/drivers/thermal/samsung/exynos_thermal_common.c
> +++ b/drivers/thermal/samsung/exynos_thermal_common.c
> @@ -417,12 +417,10 @@ void exynos_unregister_thermal(struct thermal_sensor_conf *sensor_conf)
>
>  	th_zone = sensor_conf->pzone_data;
>
> -	if (th_zone->therm_dev)
> -		thermal_zone_device_unregister(th_zone->therm_dev);
> +	thermal_zone_device_unregister(th_zone->therm_dev);
>
>  	for (i = 0; i < th_zone->cool_dev_size; i++) {
> -		if (th_zone->cool_dev[i])
> -			cpufreq_cooling_unregister(th_zone->cool_dev[i]);
> +		cpufreq_cooling_unregister(th_zone->cool_dev[i]);
>  	}

Now you have unnecessary {}

julia

>
>  	dev_info(sensor_conf->dev,
> --
> 2.1.3
>
>

Re: thermal: Exynos: Deletion of unnecessary checks before two function calls

[SF Markus Elfring]

>>  	for (i = 0; i < th_zone->cool_dev_size; i++) {
>> -		if (th_zone->cool_dev[i])
>> -			cpufreq_cooling_unregister(th_zone->cool_dev[i]);
>> +		cpufreq_cooling_unregister(th_zone->cool_dev[i]);
>>  	}
> 
> Now you have unnecessary {}

How are the chances that your source code transformation tool
can also consider the deletion of curly brackets in such an use case?

Can any more pretty-printing rules be integrated from a specific
coding style configuration?

Regards,
Markus

[PATCH v2] thermal: Exynos: Deletion of unnecessary checks before two function calls

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 21 Nov 2014 17:11:49 +0100

The functions cpufreq_cooling_unregister() and thermal_zone_device_unregister()
test whether their argument is NULL and then return immediately.
Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/thermal/samsung/exynos_thermal_common.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_thermal_common.c b/drivers/thermal/samsung/exynos_thermal_common.c
index 3f5ad25..b6be572 100644
--- a/drivers/thermal/samsung/exynos_thermal_common.c
+++ b/drivers/thermal/samsung/exynos_thermal_common.c
@@ -417,13 +417,10 @@ void exynos_unregister_thermal(struct thermal_sensor_conf *sensor_conf)
 
 	th_zone = sensor_conf->pzone_data;
 
-	if (th_zone->therm_dev)
-		thermal_zone_device_unregister(th_zone->therm_dev);
+	thermal_zone_device_unregister(th_zone->therm_dev);
 
-	for (i = 0; i < th_zone->cool_dev_size; i++) {
-		if (th_zone->cool_dev[i])
-			cpufreq_cooling_unregister(th_zone->cool_dev[i]);
-	}
+	for (i = 0; i < th_zone->cool_dev_size; ++i)
+		cpufreq_cooling_unregister(th_zone->cool_dev[i]);
 
 	dev_info(sensor_conf->dev,
 		"Exynos: Kernel Thermal management unregistered\n");
-- 
2.1.3

[PATCH 1/1] thermal: int3403: Delete a check before thermal_zone_device_unregister()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 1 Dec 2014 05:45:42 +0100

The thermal_zone_device_unregister() function tests whether its argument
is NULL and then returns immediately. Thus the test around the call
is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/thermal/int340x_thermal/int3403_thermal.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/thermal/int340x_thermal/int3403_thermal.c b/drivers/thermal/int340x_thermal/int3403_thermal.c
index 6e9fb62..790b8f6 100644
--- a/drivers/thermal/int340x_thermal/int3403_thermal.c
+++ b/drivers/thermal/int340x_thermal/int3403_thermal.c
@@ -293,8 +293,7 @@ static int int3403_sensor_add(struct int3403_priv *priv)
 	return 0;
 
  err_free_obj:
-	if (obj->tzone)
-		thermal_zone_device_unregister(obj->tzone);
+	thermal_zone_device_unregister(obj->tzone);
 	return result;
 }
 
-- 
2.1.3

Re: [PATCH 1/1] thermal: int3403: Delete a check before thermal_zone_device_unregister()

[Zhang Rui]

On Mon, 2014-12-01 at 05:50 +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 1 Dec 2014 05:45:42 +0100
> 
> The thermal_zone_device_unregister() function tests whether its argument
> is NULL and then returns immediately. Thus the test around the call
> is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

applied.

thanks,
rui
> ---
>  drivers/thermal/int340x_thermal/int3403_thermal.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/thermal/int340x_thermal/int3403_thermal.c b/drivers/thermal/int340x_thermal/int3403_thermal.c
> index 6e9fb62..790b8f6 100644
> --- a/drivers/thermal/int340x_thermal/int3403_thermal.c
> +++ b/drivers/thermal/int340x_thermal/int3403_thermal.c
> @@ -293,8 +293,7 @@ static int int3403_sensor_add(struct int3403_priv *priv)
>  	return 0;
>  
>   err_free_obj:
> -	if (obj->tzone)
> -		thermal_zone_device_unregister(obj->tzone);
> +	thermal_zone_device_unregister(obj->tzone);
>  	return result;
>  }
>  

[PATCH] ti-soc-thermal: Delete an unnecessary check before the function call "cpufreq_cooling_unregister"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 3 Feb 2015 11:15:14 +0100

The cpufreq_cooling_unregister() function tests whether its argument is NULL
and then returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
index 3fb054a..a38c175 100644
--- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
+++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
@@ -429,7 +429,7 @@ int ti_thermal_unregister_cpu_cooling(struct ti_bandgap *bgp, int id)
 
 	data = ti_bandgap_get_sensor_data(bgp, id);
 
-	if (data && data->cool_dev)
+	if (data)
 		cpufreq_cooling_unregister(data->cool_dev);
 
 	return 0;
-- 
2.2.2

[PATCH] cpufreq-dt: Delete an unnecessary check before the function call "cpufreq_cooling_unregister"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 3 Feb 2015 19:21:21 +0100

The cpufreq_cooling_unregister() function tests whether its argument is NULL
and then returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/cpufreq/cpufreq-dt.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index fde97d6..bab67db 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -320,8 +320,7 @@ static int cpufreq_exit(struct cpufreq_policy *policy)
 {
 	struct private_data *priv = policy->driver_data;
 
-	if (priv->cdev)
-		cpufreq_cooling_unregister(priv->cdev);
+	cpufreq_cooling_unregister(priv->cdev);
 	dev_pm_opp_free_cpufreq_table(priv->cpu_dev, &policy->freq_table);
 	of_free_opp_table(priv->cpu_dev);
 	clk_put(policy->clk);
-- 
2.2.2

Re: [PATCH] cpufreq-dt: Delete an unnecessary check before the function call "cpufreq_cooling_unregister"

[Rafael J. Wysocki]

On Tuesday, February 03, 2015 07:25:39 PM SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 3 Feb 2015 19:21:21 +0100
> 
> The cpufreq_cooling_unregister() function tests whether its argument is NULL
> and then returns immediately. Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Queued up for 3.20, thanks!

> ---
>  drivers/cpufreq/cpufreq-dt.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
> index fde97d6..bab67db 100644
> --- a/drivers/cpufreq/cpufreq-dt.c
> +++ b/drivers/cpufreq/cpufreq-dt.c
> @@ -320,8 +320,7 @@ static int cpufreq_exit(struct cpufreq_policy *policy)
>  {
>  	struct private_data *priv = policy->driver_data;
>  
> -	if (priv->cdev)
> -		cpufreq_cooling_unregister(priv->cdev);
> +	cpufreq_cooling_unregister(priv->cdev);
>  	dev_pm_opp_free_cpufreq_table(priv->cpu_dev, &policy->freq_table);
>  	of_free_opp_table(priv->cpu_dev);
>  	clk_put(policy->clk);
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[PATCH] PM-wakeup: Delete unnecessary checks before two function calls

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 28 Jun 2015 12:14:43 +0200

The functions dev_pm_disarm_wake_irq() and wakeup_source_unregister() test
whether their argument is NULL and then return immediately.
Thus the test around the calls is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/base/power/wakeup.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index 40f7160..3741bc2 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -341,8 +341,7 @@ void device_wakeup_arm_wake_irqs(void)
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(ws, &wakeup_sources, entry) {
-		if (ws->wakeirq)
-			dev_pm_arm_wake_irq(ws->wakeirq);
+		dev_pm_arm_wake_irq(ws->wakeirq);
 	}
 	rcu_read_unlock();
 }
@@ -358,8 +357,7 @@ void device_wakeup_disarm_wake_irqs(void)
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(ws, &wakeup_sources, entry) {
-		if (ws->wakeirq)
-			dev_pm_disarm_wake_irq(ws->wakeirq);
+		dev_pm_disarm_wake_irq(ws->wakeirq);
 	}
 	rcu_read_unlock();
 }
@@ -396,9 +394,7 @@ int device_wakeup_disable(struct device *dev)
 		return -EINVAL;
 
 	ws = device_wakeup_detach(dev);
-	if (ws)
-		wakeup_source_unregister(ws);
-
+	wakeup_source_unregister(ws);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(device_wakeup_disable);
-- 
2.4.4

Re: [PATCH] PM-wakeup: Delete unnecessary checks before two function calls

[Pavel Machek]

On Sun 2015-06-28 12:21:53, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 28 Jun 2015 12:14:43 +0200
> 
> The functions dev_pm_disarm_wake_irq() and wakeup_source_unregister() test
> whether their argument is NULL and then return immediately.
> Thus the test around the calls is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

ACK.


> ---
>  drivers/base/power/wakeup.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> index 40f7160..3741bc2 100644
> --- a/drivers/base/power/wakeup.c
> +++ b/drivers/base/power/wakeup.c
> @@ -341,8 +341,7 @@ void device_wakeup_arm_wake_irqs(void)
>  
>  	rcu_read_lock();
>  	list_for_each_entry_rcu(ws, &wakeup_sources, entry) {
> -		if (ws->wakeirq)
> -			dev_pm_arm_wake_irq(ws->wakeirq);
> +		dev_pm_arm_wake_irq(ws->wakeirq);
>  	}
>  	rcu_read_unlock();
>  }
> @@ -358,8 +357,7 @@ void device_wakeup_disarm_wake_irqs(void)
>  
>  	rcu_read_lock();
>  	list_for_each_entry_rcu(ws, &wakeup_sources, entry) {
> -		if (ws->wakeirq)
> -			dev_pm_disarm_wake_irq(ws->wakeirq);
> +		dev_pm_disarm_wake_irq(ws->wakeirq);
>  	}
>  	rcu_read_unlock();
>  }
> @@ -396,9 +394,7 @@ int device_wakeup_disable(struct device *dev)
>  		return -EINVAL;
>  
>  	ws = device_wakeup_detach(dev);
> -	if (ws)
> -		wakeup_source_unregister(ws);
> -
> +	wakeup_source_unregister(ws);
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(device_wakeup_disable);
> -- 
> 2.4.4

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH] CPPC-CPUFreq: Delete an unnecessary check before the function call "kfree"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 5 Nov 2015 21:10:29 +0100

The kfree() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/cpufreq/cppc_cpufreq.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 93c219f..e8cb334 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -166,8 +166,7 @@ static int __init cppc_cpufreq_init(void)
 
 out:
 	for_each_possible_cpu(i)
-		if (all_cpu_data[i])
-			kfree(all_cpu_data[i]);
+		kfree(all_cpu_data[i]);
 
 	kfree(all_cpu_data);
 	return -ENODEV;
-- 
2.6.2

Re: [PATCH] CPPC-CPUFreq: Delete an unnecessary check before the function call "kfree"

[Viresh Kumar]

On 05-11-15, 21:15, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 5 Nov 2015 21:10:29 +0100
> 
> The kfree() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/cpufreq/cppc_cpufreq.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

[PATCH] bq2415x_charger: Delete unnecessary checks before the function call "of_node_put"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 6 Nov 2015 16:48:46 +0100

The of_node_put() function tests whether its argument is NULL and then
returns immediately. Thus the test around the calls is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/power/bq2415x_charger.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/power/bq2415x_charger.c b/drivers/power/bq2415x_charger.c
index 4afd768..27e8953 100644
--- a/drivers/power/bq2415x_charger.c
+++ b/drivers/power/bq2415x_charger.c
@@ -1704,7 +1704,7 @@ error_4:
 error_3:
 	bq2415x_power_supply_exit(bq);
 error_2:
-	if (bq && bq->notify_node)
+	if (bq)
 		of_node_put(bq->notify_node);
 	kfree(name);
 error_1:
@@ -1724,9 +1724,7 @@ static int bq2415x_remove(struct i2c_client *client)
 	if (bq->nb.notifier_call)
 		power_supply_unreg_notifier(&bq->nb);
 
-	if (bq->notify_node)
-		of_node_put(bq->notify_node);
-
+	of_node_put(bq->notify_node);
 	bq2415x_sysfs_exit(bq);
 	bq2415x_power_supply_exit(bq);
 
-- 
2.6.2

Re: [PATCH] bq2415x_charger: Delete unnecessary checks before the function call "of_node_put"

[Pali Rohár]

On Friday 06 November 2015 16:56:33 SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 6 Nov 2015 16:48:46 +0100
> 
> The of_node_put() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the calls is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Acked-by: Pali Rohár <pali.rohar@gmail.com>

-- 
Pali Rohár
pali.rohar@gmail.com

Re: [PATCH] CPPC-CPUFreq: Delete an unnecessary check before the function call "kfree"

[Rafael J. Wysocki]

On Friday, November 06, 2015 07:12:28 AM Viresh Kumar wrote:
> On 05-11-15, 21:15, SF Markus Elfring wrote:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Thu, 5 Nov 2015 21:10:29 +0100
> > 
> > The kfree() function tests whether its argument is NULL and then
> > returns immediately. Thus the test around the call is not needed.
> > 
> > This issue was detected by using the Coccinelle software.
> > 
> > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> > ---
> >  drivers/cpufreq/cppc_cpufreq.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Applied, thanks!

Rafael

Re: [PATCH] PM-wakeup: Delete unnecessary checks before two function calls

[SF Markus Elfring]

Am 28.06.2015 um 12:21 schrieb SF Markus Elfring:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 28 Jun 2015 12:14:43 +0200
> 
> The functions dev_pm_disarm_wake_irq() and wakeup_source_unregister() test
> whether their argument is NULL and then return immediately.
> Thus the test around the calls is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/base/power/wakeup.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> index 40f7160..3741bc2 100644
> --- a/drivers/base/power/wakeup.c
> +++ b/drivers/base/power/wakeup.c
> @@ -341,8 +341,7 @@ void device_wakeup_arm_wake_irqs(void)
>  
>  	rcu_read_lock();
>  	list_for_each_entry_rcu(ws, &wakeup_sources, entry) {
> -		if (ws->wakeirq)
> -			dev_pm_arm_wake_irq(ws->wakeirq);
> +		dev_pm_arm_wake_irq(ws->wakeirq);
>  	}
>  	rcu_read_unlock();
>  }
> @@ -358,8 +357,7 @@ void device_wakeup_disarm_wake_irqs(void)
>  
>  	rcu_read_lock();
>  	list_for_each_entry_rcu(ws, &wakeup_sources, entry) {
> -		if (ws->wakeirq)
> -			dev_pm_disarm_wake_irq(ws->wakeirq);
> +		dev_pm_disarm_wake_irq(ws->wakeirq);
>  	}
>  	rcu_read_unlock();
>  }
> @@ -396,9 +394,7 @@ int device_wakeup_disable(struct device *dev)
>  		return -EINVAL;
>  
>  	ws = device_wakeup_detach(dev);
> -	if (ws)
> -		wakeup_source_unregister(ws);
> -
> +	wakeup_source_unregister(ws);
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(device_wakeup_disable);
> 

How do you think about to integrate this update suggestion
into another source code repository?

Regards,
Markus

Re: [PATCH] PM-wakeup: Delete unnecessary checks before two function calls

[Rafael J. Wysocki]

On Friday, July 22, 2016 10:35:27 PM SF Markus Elfring wrote:
> Am 28.06.2015 um 12:21 schrieb SF Markus Elfring:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Sun, 28 Jun 2015 12:14:43 +0200
> > 
> > The functions dev_pm_disarm_wake_irq() and wakeup_source_unregister() test
> > whether their argument is NULL and then return immediately.
> > Thus the test around the calls is not needed.
> > 
> > This issue was detected by using the Coccinelle software.
> > 
> > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> > ---
> >  drivers/base/power/wakeup.c | 10 +++-------
> >  1 file changed, 3 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> > index 40f7160..3741bc2 100644
> > --- a/drivers/base/power/wakeup.c
> > +++ b/drivers/base/power/wakeup.c
> > @@ -341,8 +341,7 @@ void device_wakeup_arm_wake_irqs(void)
> >  
> >  	rcu_read_lock();
> >  	list_for_each_entry_rcu(ws, &wakeup_sources, entry) {
> > -		if (ws->wakeirq)
> > -			dev_pm_arm_wake_irq(ws->wakeirq);
> > +		dev_pm_arm_wake_irq(ws->wakeirq);
> >  	}
> >  	rcu_read_unlock();
> >  }
> > @@ -358,8 +357,7 @@ void device_wakeup_disarm_wake_irqs(void)
> >  
> >  	rcu_read_lock();
> >  	list_for_each_entry_rcu(ws, &wakeup_sources, entry) {
> > -		if (ws->wakeirq)
> > -			dev_pm_disarm_wake_irq(ws->wakeirq);
> > +		dev_pm_disarm_wake_irq(ws->wakeirq);
> >  	}
> >  	rcu_read_unlock();
> >  }
> > @@ -396,9 +394,7 @@ int device_wakeup_disable(struct device *dev)
> >  		return -EINVAL;
> >  
> >  	ws = device_wakeup_detach(dev);
> > -	if (ws)
> > -		wakeup_source_unregister(ws);
> > -
> > +	wakeup_source_unregister(ws);
> >  	return 0;
> >  }
> >  EXPORT_SYMBOL_GPL(device_wakeup_disable);
> > 
> 
> How do you think about to integrate this update suggestion
> into another source code repository?

I'm not really sure what you mean.

Thanks,
Rafael

Re: PM-wakeup: Delete unnecessary checks before two function calls

[SF Markus Elfring]

>> How do you think about to integrate this update suggestion
>> into another source code repository?
> 
> I'm not really sure what you mean.

Do you find the suggested source code change acceptable?

http://article.gmane.org/gmane.linux.power-management.general/61766
https://lkml.org/lkml/2015/6/28/21


Who would dare to commit it?

Regards,
Markus

Re: PM-wakeup: Delete unnecessary checks before two function calls

[Rafael J. Wysocki]

On Saturday, July 23, 2016 07:32:22 AM SF Markus Elfring wrote:
> >> How do you think about to integrate this update suggestion
> >> into another source code repository?
> > 
> > I'm not really sure what you mean.
> 
> Do you find the suggested source code change acceptable?
> 
> http://article.gmane.org/gmane.linux.power-management.general/61766
> https://lkml.org/lkml/2015/6/28/21

Yes, it is acceptable, but you also should remove some braces that
aren't necessary any more.

Thanks,
Rafael

[PATCH] power: generic-adc-battery: Delete unnecessary checks before the function call "iio_channel_release"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 23 Jul 2016 14:50:47 +0200

The iio_channel_release() function tests whether its argument is NULL
and then returns immediately. Thus the test around the calls is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/power/generic-adc-battery.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/power/generic-adc-battery.c b/drivers/power/generic-adc-battery.c
index edb36bf..8f7f24c 100644
--- a/drivers/power/generic-adc-battery.c
+++ b/drivers/power/generic-adc-battery.c
@@ -350,10 +350,8 @@ err_gpio:
 gpio_req_fail:
 	power_supply_unregister(adc_bat->psy);
 err_reg_fail:
-	for (chan = 0; chan < ARRAY_SIZE(gab_chan_name); chan++) {
-		if (adc_bat->channel[chan])
-			iio_channel_release(adc_bat->channel[chan]);
-	}
+	for (chan = 0; chan < ARRAY_SIZE(gab_chan_name); chan++)
+		iio_channel_release(adc_bat->channel[chan]);
 second_mem_fail:
 	kfree(psy_desc->properties);
 first_mem_fail:
@@ -373,11 +371,8 @@ static int gab_remove(struct platform_device *pdev)
 		gpio_free(pdata->gpio_charge_finished);
 	}
 
-	for (chan = 0; chan < ARRAY_SIZE(gab_chan_name); chan++) {
-		if (adc_bat->channel[chan])
-			iio_channel_release(adc_bat->channel[chan]);
-	}
-
+	for (chan = 0; chan < ARRAY_SIZE(gab_chan_name); chan++)
+		iio_channel_release(adc_bat->channel[chan]);
 	kfree(adc_bat->psy_desc.properties);
 	cancel_delayed_work(&adc_bat->bat_work);
 	return 0;
-- 
2.9.2

[PATCH] PM-wakeup: Delete unnecessary checks before three function calls

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 23 Jul 2016 17:04:00 +0200

The following functions test whether their argument is NULL
and then return immediately.
* dev_pm_arm_wake_irq
* dev_pm_disarm_wake_irq
* wakeup_source_unregister

Thus the test around the calls is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/base/power/wakeup.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index 5fb7718..c35d3f5 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -334,10 +334,8 @@ void device_wakeup_arm_wake_irqs(void)
 	struct wakeup_source *ws;
 
 	rcu_read_lock();
-	list_for_each_entry_rcu(ws, &wakeup_sources, entry) {
-		if (ws->wakeirq)
-			dev_pm_arm_wake_irq(ws->wakeirq);
-	}
+	list_for_each_entry_rcu(ws, &wakeup_sources, entry)
+		dev_pm_arm_wake_irq(ws->wakeirq);
 	rcu_read_unlock();
 }
 
@@ -351,10 +349,8 @@ void device_wakeup_disarm_wake_irqs(void)
 	struct wakeup_source *ws;
 
 	rcu_read_lock();
-	list_for_each_entry_rcu(ws, &wakeup_sources, entry) {
-		if (ws->wakeirq)
-			dev_pm_disarm_wake_irq(ws->wakeirq);
-	}
+	list_for_each_entry_rcu(ws, &wakeup_sources, entry)
+		dev_pm_disarm_wake_irq(ws->wakeirq);
 	rcu_read_unlock();
 }
 
@@ -390,9 +386,7 @@ int device_wakeup_disable(struct device *dev)
 		return -EINVAL;
 
 	ws = device_wakeup_detach(dev);
-	if (ws)
-		wakeup_source_unregister(ws);
-
+	wakeup_source_unregister(ws);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(device_wakeup_disable);
-- 
2.9.2

Re: [PATCH] PM-wakeup: Delete unnecessary checks before three function calls

[Pavel Machek]

On Sat 2016-07-23 17:14:00, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 23 Jul 2016 17:04:00 +0200
> 
> The following functions test whether their argument is NULL
> and then return immediately.
> * dev_pm_arm_wake_irq
> * dev_pm_disarm_wake_irq
> * wakeup_source_unregister
> 
> Thus the test around the calls is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Acked-by: Pavel Machek <pavel@ucw.cz>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html