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

[Andy Shevchenko]

On Sat, May 09, 2026 at 07:27:38AM -0500, Maxwell Doose wrote:
> On Fri, May 8, 2026 at 9:56 PM Sanjay Chitroda
> <sanjayembeddedse@xxxxxxxxx> wrote:
> >
> > 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.
> 
> Are you sure? I find it to be far more readable than the comma mess
> that there was before, plus not a huge fan of the ternary operator in
> such expressions. One of the goals I have as the new maintainer [1] of
> the scd30 driver is to get rid of "clever" code and replace it with
> more explicit and readable code. It's going to get optimized by the
> compiler anyways. As a wise man once said,
> 
> “Programs must be written for people to read, and only incidentally
> for machines to execute.”
> 
> Plus if we need to drop a printk() or whatever in there to see the
> variable assignment we can just add {} and add the printk(), no
> messing with the ternary required.

Sanjay, the comma operator is discouraged in the kernel. For this part I'm
fully on the Maxwell's side. As for if-else versus ternary it depends on the
maintainer, but in general if-else is considered better. (For instance, I'm
judging on case-by-case basis, and in some cases ternary makes a lot of sense.)
So, in this case I have no strong opinion about sign assignment.

> > You can keep it simpler like:
> >
> > sign = (float32 & BIT(31)) ? -1 : 1;
> 
> See above.
> 
> > The IEEE 754 representation is already implicit from the bit manipulation,
> > so additional comment is probably unnecessary.

Since we don't do floats in the kernel (all arithmetics is integer-based)
some might find the comment useful. But here as well, no strong opinion.

-- 
With Best Regards,
Andy Shevchenko