RE: [PATCH v1 1/4] iio: afe: rescale: Don't use ^ for booleans
From: David Laight
Date: Fri Dec 06 2024 - 08:25:04 EST
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.
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.
>
> All these are not needed when variable is defined as boolean to begin
> with and operations are replaced by simple != and ||.
>
> Fixes: 701ee14da95d ("iio: afe: rescale: add INT_PLUS_{MICRO,NANO} support")
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
> drivers/iio/afe/iio-rescale.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/afe/iio-rescale.c b/drivers/iio/afe/iio-rescale.c
> index b6a46036d5ea..470dd7d41b2a 100644
> --- a/drivers/iio/afe/iio-rescale.c
> +++ b/drivers/iio/afe/iio-rescale.c
> @@ -26,7 +26,7 @@ int rescale_process_scale(struct rescale *rescale, int scale_type,
> int _val, _val2;
> s32 rem, rem2;
> u32 mult;
> - u32 neg;
> + bool neg;
>
> switch (scale_type) {
> case IIO_VAL_INT:
> @@ -95,7 +95,7 @@ int rescale_process_scale(struct rescale *rescale, int scale_type,
> * 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 !=.
Which will all generate real pile of horrid code.
(I think the x86 version will stun you.)
I'm guessing that somewhere there is a:
neg = value < 0;
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.
David
> if (*val)
> *val = -*val;
> else
> --
> 2.43.0.rc1.1336.g36b5255a03ac
>
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)