All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] replace if with min
@ 2021-07-12 20:45 Salah Triki
  2021-07-19 16:12   ` Christophe Leroy
  0 siblings, 1 reply; 5+ messages in thread
From: Salah Triki @ 2021-07-12 20:45 UTC (permalink / raw)
  To: haren, mpe, benh, paulus, herbert, davem
  Cc: linuxppc-dev, linux-crypto, linux-kernel

Replace if with min in order to make code more clean.

Signed-off-by: Salah Triki <salah.triki@gmail.com>
---
 drivers/crypto/nx/nx-842.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/crypto/nx/nx-842.c b/drivers/crypto/nx/nx-842.c
index 2ab90ec10e61..0d1d5a463899 100644
--- 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);
 }
 
 static int nx842_crypto_add_header(struct nx842_crypto_header *hdr, u8 *buf)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] replace if with min
  2021-07-12 20:45 [PATCH] replace if with min Salah Triki
@ 2021-07-19 16:12   ` Christophe Leroy
  0 siblings, 0 replies; 5+ messages in thread
From: Christophe Leroy @ 2021-07-19 16:12 UTC (permalink / raw)
  To: Salah Triki
  Cc: linux-kernel, linux-crypto, linuxppc-dev, davem, herbert, paulus,
	benh, mpe, haren

Salah Triki <salah.triki@gmail.com> a écrit :

> Replace if with min in order to make code more clean.
>
> Signed-off-by: Salah Triki <salah.triki@gmail.com>
> ---
>  drivers/crypto/nx/nx-842.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/crypto/nx/nx-842.c b/drivers/crypto/nx/nx-842.c
> index 2ab90ec10e61..0d1d5a463899 100644
> --- 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.

>  }
>
>  static int nx842_crypto_add_header(struct nx842_crypto_header *hdr, u8 *buf)
> --
> 2.25.1



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] replace if with min
@ 2021-07-19 16:12   ` Christophe Leroy
  0 siblings, 0 replies; 5+ messages in thread
From: Christophe Leroy @ 2021-07-19 16:12 UTC (permalink / raw)
  To: Salah Triki
  Cc: herbert, linux-kernel, paulus, linux-crypto, linuxppc-dev, davem

Salah Triki <salah.triki@gmail.com> a écrit :

> Replace if with min in order to make code more clean.
>
> Signed-off-by: Salah Triki <salah.triki@gmail.com>
> ---
>  drivers/crypto/nx/nx-842.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/crypto/nx/nx-842.c b/drivers/crypto/nx/nx-842.c
> index 2ab90ec10e61..0d1d5a463899 100644
> --- 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.

>  }
>
>  static int nx842_crypto_add_header(struct nx842_crypto_header *hdr, u8 *buf)
> --
> 2.25.1



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] replace if with min
  2021-07-19 16:12   ` Christophe Leroy
@ 2021-07-19 17:04     ` Segher Boessenkool
  -1 siblings, 0 replies; 5+ messages in thread
From: Segher Boessenkool @ 2021-07-19 17:04 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Salah Triki, herbert, linux-kernel, paulus, linux-crypto,
	linuxppc-dev, davem

On Mon, Jul 19, 2021 at 06:12:05PM +0200, Christophe Leroy wrote:
> Salah Triki <salah.triki@gmail.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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] replace if with min
@ 2021-07-19 17:04     ` Segher Boessenkool
  0 siblings, 0 replies; 5+ messages in thread
From: Segher Boessenkool @ 2021-07-19 17:04 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: herbert, Salah Triki, linux-kernel, paulus, linux-crypto,
	linuxppc-dev, davem

On Mon, Jul 19, 2021 at 06:12:05PM +0200, Christophe Leroy wrote:
> Salah Triki <salah.triki@gmail.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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-07-19 17:11 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-12 20:45 [PATCH] replace if with min Salah Triki
2021-07-19 16:12 ` Christophe Leroy
2021-07-19 16:12   ` Christophe Leroy
2021-07-19 17:04   ` Segher Boessenkool
2021-07-19 17:04     ` Segher Boessenkool

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.