[PATCH] [BUGFIX] iio: ms65611_core: Fixes dereferencing regulator pointer

[PATCH] [BUGFIX] iio: ms65611_core: Fixes dereferencing regulator pointer

[Crt Mori]

Change in handling of the regulator description means that static
checkers correctly assume we could be using dereferenced pointer to the
regulator. In reality we will never get the -ENODEV error, as current
behavior flow does not predict it, because:
If the device tree or board file does not define suitable regulators for
the component, it will be substituted by a dummy regulator, or, if
regulators are disabled altogether, by stubs.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Crt Mori <cmo@melexis.com>
---
 drivers/iio/pressure/ms5611_core.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/pressure/ms5611_core.c b/drivers/iio/pressure/ms5611_core.c
index a74ed1f..6bd53e7 100644
--- a/drivers/iio/pressure/ms5611_core.c
+++ b/drivers/iio/pressure/ms5611_core.c
@@ -392,17 +392,14 @@ static int ms5611_init(struct iio_dev *indio_dev)
 
 	/* Enable attached regulator if any. */
 	st->vdd = devm_regulator_get(indio_dev->dev.parent, "vdd");
-	if (!IS_ERR(st->vdd)) {
-		ret = regulator_enable(st->vdd);
-		if (ret) {
-			dev_err(indio_dev->dev.parent,
-				"failed to enable Vdd supply: %d\n", ret);
-			return ret;
-		}
-	} else {
-		ret = PTR_ERR(st->vdd);
-		if (ret != -ENODEV)
-			return ret;
+	if (IS_ERR(st->vdd))
+		return PTR_ERR(st->vdd);
+
+	ret = regulator_enable(st->vdd);
+	if (ret) {
+		dev_err(indio_dev->dev.parent,
+			"failed to enable Vdd supply: %d\n", ret);
+		return ret;
 	}
 
 	ret = ms5611_reset(indio_dev);
-- 
2.10.1

Re: [PATCH] [BUGFIX] iio: ms65611_core: Fixes dereferencing regulator pointer

[Jonathan Cameron]

On 14/10/16 14:43, Crt Mori wrote:
> Change in handling of the regulator description means that static
> checkers correctly assume we could be using dereferenced pointer to the
> regulator. In reality we will never get the -ENODEV error, as current
> behavior flow does not predict it, because:
> If the device tree or board file does not define suitable regulators for
> the component, it will be substituted by a dummy regulator, or, if
> regulators are disabled altogether, by stubs.
> 
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Crt Mori <cmo@melexis.com>
Applied to the togreg branch of iio.git and pushed out as testing for the
autobuilders to play with it.

Thanks,

Jonathan
> ---
>  drivers/iio/pressure/ms5611_core.c | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iio/pressure/ms5611_core.c b/drivers/iio/pressure/ms5611_core.c
> index a74ed1f..6bd53e7 100644
> --- a/drivers/iio/pressure/ms5611_core.c
> +++ b/drivers/iio/pressure/ms5611_core.c
> @@ -392,17 +392,14 @@ static int ms5611_init(struct iio_dev *indio_dev)
>  
>  	/* Enable attached regulator if any. */
>  	st->vdd = devm_regulator_get(indio_dev->dev.parent, "vdd");
> -	if (!IS_ERR(st->vdd)) {
> -		ret = regulator_enable(st->vdd);
> -		if (ret) {
> -			dev_err(indio_dev->dev.parent,
> -				"failed to enable Vdd supply: %d\n", ret);
> -			return ret;
> -		}
> -	} else {
> -		ret = PTR_ERR(st->vdd);
> -		if (ret != -ENODEV)
> -			return ret;
> +	if (IS_ERR(st->vdd))
> +		return PTR_ERR(st->vdd);
> +
> +	ret = regulator_enable(st->vdd);
> +	if (ret) {
> +		dev_err(indio_dev->dev.parent,
> +			"failed to enable Vdd supply: %d\n", ret);
> +		return ret;
>  	}
>  
>  	ret = ms5611_reset(indio_dev);
>