Re: [PATCH] iio: chemical: scd30: Cleanup initializations in scd30_float_to_fp()

[Sanjay Chitroda]

On 9 May 2026 4:25:00 am IST, Maxwell Doose <m32285159@xxxxxxxxx> wrote:
>The current variable declaration and initializations are barely readable
>and use comma separations across multiple lines. Refactor the
>initializations so that mantissa and exp have separate declarations and
>sign gets initialized later.
>
>Signed-off-by: Maxwell Doose <m32285159@xxxxxxxxx>
>---
> ps:
> Hi Jonathan, I noticed a potential divide-by-zero bug on line 241 in
> scd30_read_raw(), where the value of tmp is dictated by hardware.
> If the scd30_command_read() call on line 236 assigns 0 to tmp, then
> when we run:
> *val2 = 1000000000 / tmp;
> we'll get a divide-by-zero. Will send a patch for this later.
>
> best regards,
> max
> 
> drivers/iio/chemical/scd30_core.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/iio/chemical/scd30_core.c b/drivers/iio/chemical/scd30_core.c
>index a665fcb78806..be8c055be184 100644
>--- a/drivers/iio/chemical/scd30_core.c
>+++ b/drivers/iio/chemical/scd30_core.c
>@@ -89,10 +89,15 @@ static int scd30_reset(struct scd30_state *state)
> /* simplified float to fixed point conversion with a scaling factor of 0.01 */
> static int scd30_float_to_fp(int float32)
> {
>-	int fraction, shift,
>-	    mantissa = float32 & GENMASK(22, 0),
>-	    sign = (float32 & BIT(31)) ? -1 : 1,
>-	    exp = (float32 & ~BIT(31)) >> 23;
>+	int fraction, shift, sign;
>+	int mantissa = float32 & GENMASK(22, 0);
>+	int exp = (float32 & ~BIT(31)) >> 23;
>+
>+	/* Determine sign of received float based on IEEE 754 standard */

>+	if (float32 & BIT(31))
>+		sign = -1;
>+	else
>+		sign = 1;
Hi,

Thank you for the refactor change.

The previous version was more compact and readable. Splitting the variable declarations and expanding the sign assignment into an if/else block does not improve clarity significantly.

You can keep it simpler like:

sign = (float32 & BIT(31)) ? -1 : 1;

The IEEE 754 representation is already implicit from the bit manipulation, so additional comment is probably unnecessary.

Thanks, Sanjay
>
> 	/* special case 0 */
> 	if (!exp && !mantissa)
>--
>2.54.0
>