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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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)






[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux