iioc: dac: ltc2664: Fix span variable usage in ltc2664_channel_config()

[PATCH v3] iioc: dac: ltc2664: Fix span variable usage in ltc2664_channel_config()

Posted by Mohammed Anees 1 month, 3 weeks ago

In the current implementation of the ltc2664_channel_config() function,
a variable named span is declared and initialized to 0, intended to
capture the return value of the ltc2664_set_span() function. However,
the output of ltc2664_set_span() is directly assigned to chan->span,
leaving span unchanged. As a result, when the function later checks
if (span < 0), this condition will never trigger an error since
span remains 0, this flaw leads to ineffective error handling. Resolve
this issue by using the ret variable to get the return value and later
assign it if successful and remove unused span variable.

Fixes: 4cc2fc445d2e ("iio: dac: ltc2664: Add driver for LTC2664 and LTC2672")
Signed-off-by: Mohammed Anees <pvmohammedanees2003@gmail.com>
---
v3
- Fixed Styling issues
---
 drivers/iio/dac/ltc2664.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/dac/ltc2664.c b/drivers/iio/dac/ltc2664.c
index 5be5345ac5c8..67f14046cf77 100644
--- a/drivers/iio/dac/ltc2664.c
+++ b/drivers/iio/dac/ltc2664.c
@@ -516,7 +516,7 @@ static int ltc2664_channel_config(struct ltc2664_state *st)
 	const struct ltc2664_chip_info *chip_info = st->chip_info;
 	struct device *dev = &st->spi->dev;
 	u32 reg, tmp[2], mspan;
-	int ret, span = 0;
+	int ret;
 
 	mspan = LTC2664_MSPAN_SOFTSPAN;
 	ret = device_property_read_u32(dev, "adi,manual-span-operation-config",
@@ -579,20 +579,21 @@ static int ltc2664_channel_config(struct ltc2664_state *st)
 		ret = fwnode_property_read_u32_array(child, "output-range-microvolt",
 						     tmp, ARRAY_SIZE(tmp));
 		if (!ret && mspan == LTC2664_MSPAN_SOFTSPAN) {
-			chan->span = ltc2664_set_span(st, tmp[0] / 1000,
-						      tmp[1] / 1000, reg);
-			if (span < 0)
-				return dev_err_probe(dev, span,
+			ret = ltc2664_set_span(st, tmp[0] / 1000, tmp[1] / 1000, reg);
+			if (ret < 0)
+				return dev_err_probe(dev, ret,
 						     "Failed to set span\n");
+			chan->span = ret;
 		}
 
 		ret = fwnode_property_read_u32_array(child, "output-range-microamp",
 						     tmp, ARRAY_SIZE(tmp));
 		if (!ret) {
-			chan->span = ltc2664_set_span(st, 0, tmp[1] / 1000, reg);
-			if (span < 0)
-				return dev_err_probe(dev, span,
+			ret = ltc2664_set_span(st, 0, tmp[1] / 1000, reg);
+			if (ret < 0)
+				return dev_err_probe(dev, ret,
 						     "Failed to set span\n");
+			chan->span = ret;
 		}
 	}
 
-- 
2.46.0

Re: [PATCH v3] iioc: dac: ltc2664: Fix span variable usage in ltc2664_channel_config()

Posted by Jonathan Cameron 1 month, 2 weeks ago

On Sun,  6 Oct 2024 01:34:35 +0530
Mohammed Anees <pvmohammedanees2003@gmail.com> wrote:

> In the current implementation of the ltc2664_channel_config() function,
> a variable named span is declared and initialized to 0, intended to
> capture the return value of the ltc2664_set_span() function. However,
> the output of ltc2664_set_span() is directly assigned to chan->span,
> leaving span unchanged. As a result, when the function later checks
> if (span < 0), this condition will never trigger an error since
> span remains 0, this flaw leads to ineffective error handling. Resolve
> this issue by using the ret variable to get the return value and later
> assign it if successful and remove unused span variable.
> 
> Fixes: 4cc2fc445d2e ("iio: dac: ltc2664: Add driver for LTC2664 and LTC2672")
> Signed-off-by: Mohammed Anees <pvmohammedanees2003@gmail.com>
> ---
Hmm. I failed to send a reply to say I applied this.

Anyhow better late than never. Applied. Thanks!

Jonathan

> v3
> - Fixed Styling issues
> ---
>  drivers/iio/dac/ltc2664.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/iio/dac/ltc2664.c b/drivers/iio/dac/ltc2664.c
> index 5be5345ac5c8..67f14046cf77 100644
> --- a/drivers/iio/dac/ltc2664.c
> +++ b/drivers/iio/dac/ltc2664.c
> @@ -516,7 +516,7 @@ static int ltc2664_channel_config(struct ltc2664_state *st)
>  	const struct ltc2664_chip_info *chip_info = st->chip_info;
>  	struct device *dev = &st->spi->dev;
>  	u32 reg, tmp[2], mspan;
> -	int ret, span = 0;
> +	int ret;
>  
>  	mspan = LTC2664_MSPAN_SOFTSPAN;
>  	ret = device_property_read_u32(dev, "adi,manual-span-operation-config",
> @@ -579,20 +579,21 @@ static int ltc2664_channel_config(struct ltc2664_state *st)
>  		ret = fwnode_property_read_u32_array(child, "output-range-microvolt",
>  						     tmp, ARRAY_SIZE(tmp));
>  		if (!ret && mspan == LTC2664_MSPAN_SOFTSPAN) {
> -			chan->span = ltc2664_set_span(st, tmp[0] / 1000,
> -						      tmp[1] / 1000, reg);
> -			if (span < 0)
> -				return dev_err_probe(dev, span,
> +			ret = ltc2664_set_span(st, tmp[0] / 1000, tmp[1] / 1000, reg);
> +			if (ret < 0)
> +				return dev_err_probe(dev, ret,
>  						     "Failed to set span\n");
> +			chan->span = ret;
>  		}
>  
>  		ret = fwnode_property_read_u32_array(child, "output-range-microamp",
>  						     tmp, ARRAY_SIZE(tmp));
>  		if (!ret) {
> -			chan->span = ltc2664_set_span(st, 0, tmp[1] / 1000, reg);
> -			if (span < 0)
> -				return dev_err_probe(dev, span,
> +			ret = ltc2664_set_span(st, 0, tmp[1] / 1000, reg);
> +			if (ret < 0)
> +				return dev_err_probe(dev, ret,
>  						     "Failed to set span\n");
> +			chan->span = ret;
>  		}
>  	}
>