Re: [PATCH net-next 2/8] net/rds: Use max() to simplify the code

[Simon Horman <horms@kernel.org>]

On Sat, Aug 24, 2024 at 03:40:27PM +0800, Hongbo Li wrote:
> The target if-else can be replaced with max().
> 
> Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
> ---
>  net/rds/info.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/net/rds/info.c b/net/rds/info.c
> index b6b46a8214a0..8558b0a466b4 100644
> --- a/net/rds/info.c
> +++ b/net/rds/info.c
> @@ -194,10 +194,7 @@ int rds_info_getsockopt(struct socket *sock, int optname, char __user *optval,
>  	}
>  	ret = pin_user_pages_fast(start, nr_pages, FOLL_WRITE, pages);
>  	if (ret != nr_pages) {
> -		if (ret > 0)
> -			nr_pages = ret;
> -		else
> -			nr_pages = 0;
> +		nr_pages = max(ret, 0);

Along the same lines as Johannes Berg's comment on a different patch [1]
I think that there is a subtle but important difference, semantically,
between max() and that the existing code does, for which the best
description I can think of is setting a floor on the value.

Other than Johannes's comment, and now mine here, I think you will find
that, if you search the netdev ML, you will find this point being made
consistently, at least over the past year.

And yes, we understand that mathematically max() is doing the right thing.
But that is not the point that is being made here.

I suggest dropping this patch.
And any others like it.

[1] https://lore.kernel.org/all/d5f495b67fe6bf128e7a51b9fcfe11f70c9b66ae.camel@sipsolutions.net/

>  		ret = -EAGAIN; /* XXX ? */
>  		goto out;
>  	}
> -- 
> 2.34.1
> 
>