Search Linux Wireless

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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxxxxx/

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




[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux