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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <88f281a31d8342c691b2a6b2666d4e91@AcuMS.aculab.com>
Date: Fri, 6 Dec 2024 13:24:09 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'Andy Shevchenko' <andriy.shevchenko@...ux.intel.com>,
	"linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC: Peter Rosin <peda@...ntia.se>, Jonathan Cameron <jic23@...nel.org>,
	Lars-Peter Clausen <lars@...afoo.de>
Subject: RE: [PATCH v1 1/4] iio: afe: rescale: Don't use ^ for booleans

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@...ux.intel.com>
> ---
>  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)