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: <Z1MWBsCJsTHsqNey@smile.fi.intel.com>
Date: Fri, 6 Dec 2024 17:19:34 +0200
From: 'Andy Shevchenko' <andriy.shevchenko@...ux.intel.com>
To: David Laight <David.Laight@...lab.com>
Cc: "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	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

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