linux-kernel - Re: [PATCH] sfc: replace min/max nesting with clamp()

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: <20250817154205.6044124e@pumpkin>
Date: Sun, 17 Aug 2025 15:42:05 +0100
From: David Laight <david.laight.linux@...il.com>
To: Xichao Zhao <zhao.xichao@...o.com>
Cc: ecree.xilinx@...il.com, andrew+netdev@...n.ch, davem@...emloft.net,
 edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
 netdev@...r.kernel.org, linux-net-drivers@....com,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] sfc: replace min/max nesting with clamp()

On Tue, 12 Aug 2025 14:50:26 +0800
Xichao Zhao <zhao.xichao@...o.com> wrote:

> The clamp() macro explicitly expresses the intent of constraining
> a value within bounds.Therefore, replacing min(max(a, b), c) with
> clamp(val, lo, hi) can improve code readability.

I think you can do a better job of the line wraps?
The first and third won't exceed 80 cols split onto two lines.
They might be 86 on one line - plausibly ok (but I like 80 and they are splittable).

	David

> 
> Signed-off-by: Xichao Zhao <zhao.xichao@...o.com>
> ---
>  drivers/net/ethernet/sfc/efx_channels.c       | 4 ++--
>  drivers/net/ethernet/sfc/falcon/efx.c         | 5 ++---
>  drivers/net/ethernet/sfc/siena/efx_channels.c | 4 ++--
>  3 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sfc/efx_channels.c b/drivers/net/ethernet/sfc/efx_channels.c
> index 06b4f52713ef..0f66324ed351 100644
> --- a/drivers/net/ethernet/sfc/efx_channels.c
> +++ b/drivers/net/ethernet/sfc/efx_channels.c
> @@ -216,8 +216,8 @@ static int efx_allocate_msix_channels(struct efx_nic *efx,
>  
>  	if (efx_separate_tx_channels) {
>  		efx->n_tx_channels =
> -			min(max(n_channels / 2, 1U),
> -			    efx->max_tx_channels);
> +			clamp(n_channels / 2, 1U,
> +			      efx->max_tx_channels);
>  		efx->tx_channel_offset =
>  			n_channels - efx->n_tx_channels;
>  		efx->n_rx_channels =
> diff --git a/drivers/net/ethernet/sfc/falcon/efx.c b/drivers/net/ethernet/sfc/falcon/efx.c
> index b07f7e4e2877..d19fbf8732ff 100644
> --- a/drivers/net/ethernet/sfc/falcon/efx.c
> +++ b/drivers/net/ethernet/sfc/falcon/efx.c
> @@ -1394,9 +1394,8 @@ static int ef4_probe_interrupts(struct ef4_nic *efx)
>  			if (n_channels > extra_channels)
>  				n_channels -= extra_channels;
>  			if (ef4_separate_tx_channels) {
> -				efx->n_tx_channels = min(max(n_channels / 2,
> -							     1U),
> -							 efx->max_tx_channels);
> +				efx->n_tx_channels = clamp(n_channels / 2, 1U,
> +							   efx->max_tx_channels);
>  				efx->n_rx_channels = max(n_channels -
>  							 efx->n_tx_channels,
>  							 1U);
> diff --git a/drivers/net/ethernet/sfc/siena/efx_channels.c b/drivers/net/ethernet/sfc/siena/efx_channels.c
> index d120b3c83ac0..703419866d18 100644
> --- a/drivers/net/ethernet/sfc/siena/efx_channels.c
> +++ b/drivers/net/ethernet/sfc/siena/efx_channels.c
> @@ -217,8 +217,8 @@ static int efx_allocate_msix_channels(struct efx_nic *efx,
>  
>  	if (efx_siena_separate_tx_channels) {
>  		efx->n_tx_channels =
> -			min(max(n_channels / 2, 1U),
> -			    efx->max_tx_channels);
> +			clamp(n_channels / 2, 1U,
> +			      efx->max_tx_channels);
>  		efx->tx_channel_offset =
>  			n_channels - efx->n_tx_channels;
>  		efx->n_rx_channels =