Thread (14 messages) flat view 14 messages, 7 authors, 2016-02-16

Re: [PATCH] PM / OPP: Initialize regulator pointer to an error value

From: Arnd Bergmann <arnd@arndb.de>
Date: 2016-02-15 20:38:37
Also in: lkml

On Monday 15 February 2016 21:56:42 Viresh Kumar wrote:
We are currently required to do two checks for regulator pointer:
IS_ERR() and IS_NULL().

And multiple instances are reported, about both of these not being used
consistently and so resulting in crashes.

Fix that by initializing regulator pointer with an error value and
checking it only against an error.

This makes code consistent and efficient.
There is usually something else wrong if you have to check for both.
Why exactly do you need to check for both IS_ERR and NULL?
quoted hunk ↗ jump to hunk
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index d7cd4e265766..146b6197d598 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -257,7 +257,7 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev)
 	}
 
 	reg = dev_opp->regulator;
-	if (IS_ERR_OR_NULL(reg)) {
+	if (IS_ERR(reg)) {
 		/* Regulator may not be required for device */
 		if (reg)
 			dev_err(dev, "%s: Invalid regulator (%ld)\n", __func__,
@@ -798,6 +798,9 @@ static struct device_opp *_add_device_opp(struct device *dev)
 		of_node_put(np);
 	}
 
+	/* Set regulator to a non-NULL error value */
+	dev_opp->regulator = ERR_PTR(-EFAULT);
+
 	/* Find clk for the device */
 	dev_opp->clk = clk_get(dev, NULL);
 	if (IS_ERR(dev_opp->clk)) {
-EFAULT has a very specific meaning (accessing an invalid pointer from
user space), I don't think you want that one.

	Arnd
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help