Re: [PATCH v1 1/4] iio: afe: rescale: Don't use ^ for booleans

From: 'Andy Shevchenko'
Date: Fri Dec 06 2024 - 10:19:45 EST


On Fri, Dec 06, 2024 at 01:24:09PM +0000, David Laight wrote:
> From: Andy Shevchenko
> > Sent: 04 December 2024 01:33
> >
> > There are two (non-critical) issues with the code. First of all,
> > the eXclusive OR is not defined for booleans, so boolean to integer
> > promotion is required, Second, the u32 variable is used to keep
> > boolean value, so boolean is converted implicitly to the integer.
>
> Except there is no such thing as 'boolean' they are all integers.

I believe this is an exercise in linguistics as I'm not native speaker
but I am very well aware of the promotions to the integer values.

> And the compiler has to have some set of rules to handle the cases
> where the memory that hold the 'boolean' doesn't have the value 0 or 1.

No doubts.

...

> > * If only one of the rescaler elements or the schan scale is
> > * negative, the combined scale is negative.
> > */
> > - if (neg ^ ((rescale->numerator < 0) ^ (rescale->denominator < 0))) {
> > + if (neg != (rescale->numerator < 0 || rescale->denominator < 0)) {
>
> That is wrong, the || would also need to be !=.

Why do you think so? Maybe it's comment(s) that is(are) wrong?

> Which will all generate real pile of horrid code.
> (I think the x86 version will stun you.)

I think your remark is based on something, can you show the output to elaborate
what exactly becomes horrible in this case?

> I'm guessing that somewhere there is a:
> neg = value < 0;

Nope.

> Provided all the values are the same size (eg int/s32), in which case:
> neg = value;
> ...
> if ((neg ^ rescale->numerator ^ rescale->denominator) < 0)
> will be the desired test.

--
With Best Regards,
Andy Shevchenko