Re: [PATCH v2] iio: adc: stm32-adc: fix possible division by zero in processed channel

[Fabrice Gasnier <fabrice.gasnier@foss.st.com>]

On 9/16/26 16:39, Andy Shevchenko wrote:

[ snip ]

> Let's look at the current code in this switch-case
> 

Hi Andy,

Thanks for you suggestions !

Let's keep in the picture the 'normal' raw case:

	case IIO_CHAN_INFO_RAW:
> 	case IIO_CHAN_INFO_PROCESSED:
> 		if (!iio_device_claim_direct(indio_dev))
> 			return -EBUSY;
> 		if (chan->type == IIO_VOLTAGE)
> 			ret = stm32_adc_single_conv(indio_dev, chan, val);
> 		else
> 			ret = -EINVAL;
> 
> 		if (mask == IIO_CHAN_INFO_PROCESSED)
> 			*val = STM32_ADC_VREFINT_VOLTAGE * adc->vrefint.vrefint_cal / *val;
> 
> 		iio_device_release_direct(indio_dev);
> 		return ret;
> 
> In the previous version you were trying to keep goto-less approach.
> However in the current state of affairs I don't think the goto is
> too bad (after all we need to backport this to the kernels that may
> not have IIO_DEV_ACQUIRE_DIRECT_MODE() macro).
> 
> Also checking 'ret' under another condition seems unusual.
> 
> What about
> 
> 	case IIO_CHAN_INFO_PROCESSED:
> 		if (!iio_device_claim_direct(indio_dev))
> 			return -EBUSY;
> 		if (chan->type == IIO_VOLTAGE)
> 			ret = stm32_adc_single_conv(indio_dev, chan, val);
> 		else
> 			ret = -EINVAL;
> 		iio_device_release_direct(indio_dev);
> 		if (ret)
> 			return ret;
> 
> 		if (*val == 0)
> 			return -EINVAL;

So *val == 0 is normally a valid value, for 'normal' raw channels.

> 
> 		if (mask == IIO_CHAN_INFO_PROCESSED)
> 			*val = STM32_ADC_VREFINT_VOLTAGE * adc->vrefint.vrefint_cal / *val;

With you suggestion, moving if (*val == 0) to here should be fine ?

		if (mask == IIO_CHAN_INFO_PROCESSED) {
			if (*val == 0)
				return -EINVAL;
			*val = STM32_ADC_VREFINT_VOLTAGE * adc->vrefint.vrefint_cal / *val;
		}

Best Regards,
Fabrice

> 
> 		return 0;
> 
> ? (Yes, we leave that line untouched, but we get the change cleaner.)
> 
>