* [PATCH 0/2] thermal: avoid using IS_ERR_OR_NULL() @ 2013-04-25 14:13 Eduardo Valentin 2013-04-25 14:13 ` [PATCH 1/2] thermal: thermal_core: remove usage of IS_ERR_OR_NULL Eduardo Valentin 2013-04-25 14:13 ` [PATCH 2/2] thermal: db8500_cpufreq_cooling: remove usage of IS_ERR_OR_NULL() Eduardo Valentin 0 siblings, 2 replies; 7+ messages in thread From: Eduardo Valentin @ 2013-04-25 14:13 UTC (permalink / raw) To: Zhang Rui; +Cc: linux-pm, linux-kernel, Eduardo Valentin Hello Rui, Here are two important patches I cooked while reading linux-arm archives. IS_ERR_OR_NULL() can cause serious problems and some ppl is even considering removing it [1]. For this reason, I am proposing the following cleanup under drivers/thermal/ Please consider them. BR, Eduardo Valentin (2): thermal: thermal_core: remove usage of IS_ERR_OR_NULL thermal: db8500_cpufreq_cooling: remove usage of IS_ERR_OR_NULL() drivers/thermal/db8500_cpufreq_cooling.c | 2 +- drivers/thermal/thermal_core.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) [1] - http://www.mail-archive.com/linux-omap@vger.kernel.org/msg78030.html -- 1.8.2.1.342.gfa7285d ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] thermal: thermal_core: remove usage of IS_ERR_OR_NULL 2013-04-25 14:13 [PATCH 0/2] thermal: avoid using IS_ERR_OR_NULL() Eduardo Valentin @ 2013-04-25 14:13 ` Eduardo Valentin 2013-04-25 14:13 ` [PATCH 2/2] thermal: db8500_cpufreq_cooling: remove usage of IS_ERR_OR_NULL() Eduardo Valentin 1 sibling, 0 replies; 7+ messages in thread From: Eduardo Valentin @ 2013-04-25 14:13 UTC (permalink / raw) To: Zhang Rui; +Cc: linux-pm, linux-kernel, Eduardo Valentin, Russell King This patch changes the driver to avoid the usage of IS_ERR_OR_NULL() macro. This macro can lead to dangerous results, like returning success (0) during a failure scenario (NULL pointer handling). The case present in this patch has simply be translated to normal check for NULL and if the pointer has an error code. The later case is needed because functions like thermal_zone_get_zone_by_name() could return an ERR_PTR(). Cc: Zhang Rui <rui.zhang@intel.com> Cc: Russell King <rmk@arm.linux.org.uk> Cc: linux-pm@vger.kernel.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com> --- drivers/thermal/thermal_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index f36cd44..d755440 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -388,7 +388,7 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp) enum thermal_trip_type type; #endif - if (IS_ERR_OR_NULL(tz)) + if (!tz || IS_ERR(tz)) goto exit; mutex_lock(&tz->lock); -- 1.8.2.1.342.gfa7285d ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] thermal: db8500_cpufreq_cooling: remove usage of IS_ERR_OR_NULL() 2013-04-25 14:13 [PATCH 0/2] thermal: avoid using IS_ERR_OR_NULL() Eduardo Valentin 2013-04-25 14:13 ` [PATCH 1/2] thermal: thermal_core: remove usage of IS_ERR_OR_NULL Eduardo Valentin @ 2013-04-25 14:13 ` Eduardo Valentin 2013-04-25 17:46 ` Russell King 1 sibling, 1 reply; 7+ messages in thread From: Eduardo Valentin @ 2013-04-25 14:13 UTC (permalink / raw) To: Zhang Rui Cc: linux-pm, linux-kernel, Eduardo Valentin, Russell King, Grant Likely, Rob Herring, Hongbo Zhang, devicetree-discuss This patch changes the driver to avoid the usage of IS_ERR_OR_NULL() macro. This macro can lead to dangerous results, like returning success (0) during a failure scenario (NULL pointer handling). The case present in this driver can be translated to a simple check for IS_ERR(), as the cpufreq_cooling_register() returns either a valid pointer or an ERR_PTR(). Cc: Zhang Rui <rui.zhang@intel.com> Cc: Russell King <rmk@arm.linux.org.uk> Cc: Grant Likely <grant.likely@secretlab.ca> Cc: Rob Herring <rob.herring@calxeda.com> Cc: Hongbo Zhang <hongbo.zhang@stericsson.com> Cc: linux-pm@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: devicetree-discuss@lists.ozlabs.org Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com> --- drivers/thermal/db8500_cpufreq_cooling.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/thermal/db8500_cpufreq_cooling.c b/drivers/thermal/db8500_cpufreq_cooling.c index 21419851..786d192 100644 --- a/drivers/thermal/db8500_cpufreq_cooling.c +++ b/drivers/thermal/db8500_cpufreq_cooling.c @@ -37,7 +37,7 @@ static int db8500_cpufreq_cooling_probe(struct platform_device *pdev) cpumask_set_cpu(0, &mask_val); cdev = cpufreq_cooling_register(&mask_val); - if (IS_ERR_OR_NULL(cdev)) { + if (IS_ERR(cdev)) { dev_err(&pdev->dev, "Failed to register cooling device\n"); return PTR_ERR(cdev); } -- 1.8.2.1.342.gfa7285d ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] thermal: db8500_cpufreq_cooling: remove usage of IS_ERR_OR_NULL() 2013-04-25 14:13 ` [PATCH 2/2] thermal: db8500_cpufreq_cooling: remove usage of IS_ERR_OR_NULL() Eduardo Valentin @ 2013-04-25 17:46 ` Russell King 2013-04-25 20:17 ` Eduardo Valentin 2013-04-26 10:19 ` Fabio Baltieri 0 siblings, 2 replies; 7+ messages in thread From: Russell King @ 2013-04-25 17:46 UTC (permalink / raw) To: Eduardo Valentin Cc: Zhang Rui, linux-pm, linux-kernel, Grant Likely, Rob Herring, Hongbo Zhang, devicetree-discuss On Thu, Apr 25, 2013 at 10:13:34AM -0400, Eduardo Valentin wrote: > diff --git a/drivers/thermal/db8500_cpufreq_cooling.c b/drivers/thermal/db8500_cpufreq_cooling.c > index 21419851..786d192 100644 > --- a/drivers/thermal/db8500_cpufreq_cooling.c > +++ b/drivers/thermal/db8500_cpufreq_cooling.c > @@ -37,7 +37,7 @@ static int db8500_cpufreq_cooling_probe(struct platform_device *pdev) > cpumask_set_cpu(0, &mask_val); > cdev = cpufreq_cooling_register(&mask_val); > > - if (IS_ERR_OR_NULL(cdev)) { > + if (IS_ERR(cdev)) { > dev_err(&pdev->dev, "Failed to register cooling device\n"); > return PTR_ERR(cdev); Correct. cpufreq_cooling_register() returns either an error-pointer or a valid pointer. -- Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] thermal: db8500_cpufreq_cooling: remove usage of IS_ERR_OR_NULL() 2013-04-25 17:46 ` Russell King @ 2013-04-25 20:17 ` Eduardo Valentin 2013-04-26 10:19 ` Fabio Baltieri 1 sibling, 0 replies; 7+ messages in thread From: Eduardo Valentin @ 2013-04-25 20:17 UTC (permalink / raw) To: Russell King Cc: Eduardo Valentin, Zhang Rui, linux-pm, linux-kernel, Grant Likely, Rob Herring, Hongbo Zhang, devicetree-discuss On 25-04-2013 13:46, Russell King wrote: > On Thu, Apr 25, 2013 at 10:13:34AM -0400, Eduardo Valentin wrote: >> diff --git a/drivers/thermal/db8500_cpufreq_cooling.c b/drivers/thermal/db8500_cpufreq_cooling.c >> index 21419851..786d192 100644 >> --- a/drivers/thermal/db8500_cpufreq_cooling.c >> +++ b/drivers/thermal/db8500_cpufreq_cooling.c >> @@ -37,7 +37,7 @@ static int db8500_cpufreq_cooling_probe(struct platform_device *pdev) >> cpumask_set_cpu(0, &mask_val); >> cdev = cpufreq_cooling_register(&mask_val); >> >> - if (IS_ERR_OR_NULL(cdev)) { >> + if (IS_ERR(cdev)) { >> dev_err(&pdev->dev, "Failed to register cooling device\n"); >> return PTR_ERR(cdev); > > Correct. cpufreq_cooling_register() returns either an error-pointer or > a valid pointer. > Thanks for your review! ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] thermal: db8500_cpufreq_cooling: remove usage of IS_ERR_OR_NULL() 2013-04-25 17:46 ` Russell King 2013-04-25 20:17 ` Eduardo Valentin @ 2013-04-26 10:19 ` Fabio Baltieri 2013-04-26 11:03 ` Russell King 1 sibling, 1 reply; 7+ messages in thread From: Fabio Baltieri @ 2013-04-26 10:19 UTC (permalink / raw) To: Russell King Cc: Eduardo Valentin, Zhang Rui, linux-pm, linux-kernel, Grant Likely, Rob Herring, Hongbo Zhang, devicetree-discuss, Linus Walleij On Thu, Apr 25, 2013 at 06:46:35PM +0100, Russell King wrote: > On Thu, Apr 25, 2013 at 10:13:34AM -0400, Eduardo Valentin wrote: > > diff --git a/drivers/thermal/db8500_cpufreq_cooling.c b/drivers/thermal/db8500_cpufreq_cooling.c > > index 21419851..786d192 100644 > > --- a/drivers/thermal/db8500_cpufreq_cooling.c > > +++ b/drivers/thermal/db8500_cpufreq_cooling.c > > @@ -37,7 +37,7 @@ static int db8500_cpufreq_cooling_probe(struct platform_device *pdev) > > cpumask_set_cpu(0, &mask_val); > > cdev = cpufreq_cooling_register(&mask_val); > > > > - if (IS_ERR_OR_NULL(cdev)) { > > + if (IS_ERR(cdev)) { > > dev_err(&pdev->dev, "Failed to register cooling device\n"); > > return PTR_ERR(cdev); > > Correct. cpufreq_cooling_register() returns either an error-pointer or > a valid pointer. Acked-by: Fabio Baltieri <fabio.baltieri@linaro.org> ...but more of these are going to appear forever, was the proposed removal of IS_ERR_OR_NULL rejected? Can't find any new message about that on the list. Fabio -- Fabio Baltieri ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] thermal: db8500_cpufreq_cooling: remove usage of IS_ERR_OR_NULL() 2013-04-26 10:19 ` Fabio Baltieri @ 2013-04-26 11:03 ` Russell King 0 siblings, 0 replies; 7+ messages in thread From: Russell King @ 2013-04-26 11:03 UTC (permalink / raw) To: Fabio Baltieri Cc: Eduardo Valentin, Zhang Rui, linux-pm, linux-kernel, Grant Likely, Rob Herring, Hongbo Zhang, devicetree-discuss, Linus Walleij On Fri, Apr 26, 2013 at 12:19:08PM +0200, Fabio Baltieri wrote: > On Thu, Apr 25, 2013 at 06:46:35PM +0100, Russell King wrote: > > On Thu, Apr 25, 2013 at 10:13:34AM -0400, Eduardo Valentin wrote: > > > diff --git a/drivers/thermal/db8500_cpufreq_cooling.c b/drivers/thermal/db8500_cpufreq_cooling.c > > > index 21419851..786d192 100644 > > > --- a/drivers/thermal/db8500_cpufreq_cooling.c > > > +++ b/drivers/thermal/db8500_cpufreq_cooling.c > > > @@ -37,7 +37,7 @@ static int db8500_cpufreq_cooling_probe(struct platform_device *pdev) > > > cpumask_set_cpu(0, &mask_val); > > > cdev = cpufreq_cooling_register(&mask_val); > > > > > > - if (IS_ERR_OR_NULL(cdev)) { > > > + if (IS_ERR(cdev)) { > > > dev_err(&pdev->dev, "Failed to register cooling device\n"); > > > return PTR_ERR(cdev); > > > > Correct. cpufreq_cooling_register() returns either an error-pointer or > > a valid pointer. > > Acked-by: Fabio Baltieri <fabio.baltieri@linaro.org> > > ...but more of these are going to appear forever, was the proposed > removal of IS_ERR_OR_NULL rejected? Can't find any new message about > that on the list. We first need to get rid of the existing users of it - I've got rid of most of those in arch/arm - but it seems that I never pushed that in the last merge window. Really it needs the cooperation of everyone to make it happen across the tree with everyone removing IS_ERR_OR_NULL() in their subsystem. There are currently 366 instances of this macro being used in the entire tree which is far too many to even mark it as deprecated. -- Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-04-26 11:03 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-04-25 14:13 [PATCH 0/2] thermal: avoid using IS_ERR_OR_NULL() Eduardo Valentin 2013-04-25 14:13 ` [PATCH 1/2] thermal: thermal_core: remove usage of IS_ERR_OR_NULL Eduardo Valentin 2013-04-25 14:13 ` [PATCH 2/2] thermal: db8500_cpufreq_cooling: remove usage of IS_ERR_OR_NULL() Eduardo Valentin 2013-04-25 17:46 ` Russell King 2013-04-25 20:17 ` Eduardo Valentin 2013-04-26 10:19 ` Fabio Baltieri 2013-04-26 11:03 ` Russell King
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).