linux-kernel - Re: [PATCH] replace if with min

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] [day] [month] [year] [list]

Message-ID: <20210719170422.GO1583@gate.crashing.org>
Date:   Mon, 19 Jul 2021 12:04:22 -0500
From:   Segher Boessenkool <segher@...nel.crashing.org>
To:     Christophe Leroy <christophe.leroy@...roup.eu>
Cc:     Salah Triki <salah.triki@...il.com>, herbert@...dor.apana.org.au,
        linux-kernel@...r.kernel.org, paulus@...ba.org,
        linux-crypto@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
        davem@...emloft.net
Subject: Re: [PATCH] replace if with min

On Mon, Jul 19, 2021 at 06:12:05PM +0200, Christophe Leroy wrote:
> Salah Triki <salah.triki@...il.com> a écrit :
> >Replace if with min in order to make code more clean.

> >--- a/drivers/crypto/nx/nx-842.c
> >+++ b/drivers/crypto/nx/nx-842.c
> >@@ -134,8 +134,7 @@ EXPORT_SYMBOL_GPL(nx842_crypto_exit);
> > static void check_constraints(struct nx842_constraints *c)
> > {
> > 	/* limit maximum, to always have enough bounce buffer to decompress 
> > 	*/
> >-	if (c->maximum > BOUNCE_BUFFER_SIZE)
> >-		c->maximum = BOUNCE_BUFFER_SIZE;
> >+	c->maximum = min(c->maximum, BOUNCE_BUFFER_SIZE);
> 
> For me the code is less clear with this change, and in addition it  
> slightly changes the behaviour. Before, the write was done only when  
> the value was changing. Now you rewrite the value always, even when it  
> doesn't change.

In both cases the compiler can decide to either write it more often than
strictly needed, depending on what it thinks best (and it usually has
better estimates than the programmer).  The behaviour is identical (and
the generated machine code is as well, in my testing).

The field name "maximum" is not the best choice, which makes the code
read a bit funny ("the min of max"), but the comment makes things pretty
clear.


Segher