Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 7 Nov 2022 16:20:27 +0300
From:   Dan Carpenter <error27@gmail.com>
To:     Deepak R Varma <drv@mailo.com>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] staging: most: video: use min_t() for comparison and
 assignment
Message-ID: <Y2kGG3zuvvDLZKWq@kadam>
References: <Y2iHl5CuqyR2vEc8@qemulion>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Y2iHl5CuqyR2vEc8@qemulion>
Precedence: bulk

On Mon, Nov 07, 2022 at 09:50:39AM +0530, Deepak R Varma wrote:
> Simplify code by using min_t helper macro for logical evaluation
> and value assignment. Use the _t variant of min macro since the
> variable types are not same.
> This issue is identified by coccicheck using the minmax.cocci file.
> 
> Signed-off-by: Deepak R Varma <drv@mailo.com>
> ---
> 
> Changes in v2:
>    1. Revise patch description. No functional change.
> 
>  drivers/staging/most/video/video.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/most/video/video.c b/drivers/staging/most/video/video.c
> index ffa97ef21ea5..d5cc7eea3b52 100644
> --- a/drivers/staging/most/video/video.c
> +++ b/drivers/staging/most/video/video.c
> @@ -173,7 +173,7 @@ static ssize_t comp_vdev_read(struct file *filp, char __user *buf,
>  	while (count > 0 && data_ready(mdev)) {
>  		struct mbo *const mbo = get_top_mbo(mdev);
>  		int const rem = mbo->processed_length - fh->offs;
> -		int const cnt = rem < count ? rem : count;
> +		int const cnt = min_t(int, rem, count);

TL;DR use size_t instead of int.

Using "int" here is wrong.  size_t is unsigned long meaning that it has
64 bits to use to represent positive values.  (Let's ignore 32 bit
arches).  You have chopped it down to say that it now has 31 bits for
positives and if BIT(31) is set then treat it as negative.  Everything
which is larger than INT_MAX will be broken.

Fortunately, in this code the value of count will never go higher than
"INT_MAX - PAGE_SIZE" because Linus understands that it's easy to
introduce bugs like this.

regards,
dan carpenter