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

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

On 9/16/26 09:35, Andy Shevchenko wrote:
> On Tue, Sep 15, 2026 at 06:10:40PM +0200, Fabrice Gasnier wrote:
>> In case the conversion has failed or returned zero, processing *val
>> can lead to a division by zero.
>> Need to check for errors, or converted value is zero, before processing
>> the data.
>> In case converted value is zero, e.g. the Vrefint channel, this should
>> be considered as invalid in all case.
> 
> Something went very wrong with the indentation of the above.

Hi Andy,

Euh, sorry but I don't understand what's wrong with indentation in the
commit message ? Could you clarify ?

> 
> ...
> 
>> -		if (mask == IIO_CHAN_INFO_PROCESSED)
>> -			*val = STM32_ADC_VREFINT_VOLTAGE * adc->vrefint.vrefint_cal / *val;
>> +		if (mask == IIO_CHAN_INFO_PROCESSED) {
>> +			if (ret >= 0 && *val)
>> +				*val = STM32_ADC_VREFINT_VOLTAGE * adc->vrefint.vrefint_cal / *val;
>> +			else
>> +				ret = ret < 0 ? ret : -EINVAL;
>> +		}
> 
> Reuse of the *val makes code harder to follow. Add a temporary variable for
> this and do something like this (also note other simplifications)
> 
> 		tmp_choose_good_name = *val;

In case ret is an error; this lead to evaluate (e.g. read) *val from the
(maybe uninitialized) stack. Probably not an issue? But would prefer to
keep reading it only when ret >= 0, as done currently with the if
condition above ? (e.g. evaluate ret, before *val)

> 		...
> 		if (mask == IIO_CHAN_INFO_PROCESSED) {
> 			if (ret < 0)
> 				return ret;
> 			if (tmp == 0)
> 				return -EINVAL;

I've started with something similar before, but we can't return directly
here. Must call iio_device_release_direct() first. This would add more
lines.

So I chose to implement above ternary ret = ret < 0 ? ret : -EINVAL, to
fall-through.

Please advise on the preferred way,

Best Regards,
Fabrice

> 
> 			*val = STM32_ADC_VREFINT_VOLTAGE * adc->vrefint.vrefint_cal / tmp;
> 		}
> 
>>  		iio_device_release_direct(indio_dev);
>>  		return ret;
>