public inbox for linux-staging@lists.linux.dev
 help / color / mirror / Atom feed
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
Date: Mon, 7 Nov 2022 16:20:27 +0300	[thread overview]
Message-ID: <Y2kGG3zuvvDLZKWq@kadam> (raw)
In-Reply-To: <Y2iHl5CuqyR2vEc8@qemulion>

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


  reply	other threads:[~2022-11-07 13:20 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-07  4:20 [PATCH v2] staging: most: video: use min_t() for comparison and assignment Deepak R Varma
2022-11-07 13:20 ` Dan Carpenter [this message]
2022-11-07 15:10   ` Deepak R Varma
2022-11-07 15:21     ` Dan Carpenter
2022-11-07 17:52       ` Deepak R Varma

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y2kGG3zuvvDLZKWq@kadam \
    --to=error27@gmail.com \
    --cc=drv@mailo.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox