All of lore.kernel.org
 help / color / mirror / Atom feed
From: 'Andy Shevchenko' <andriy.shevchenko@linux.intel.com>
To: David Laight <David.Laight@aculab.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	'Joe Perches' <joe@perches.com>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH 0/1] Slightly relax the type checking done by min() and max().
Date: Fri, 25 Nov 2022 17:58:13 +0200	[thread overview]
Message-ID: <Y4DmFfj6G6+niZ+t@smile.fi.intel.com> (raw)
In-Reply-To: <7e594ad64e444d448c747688b8f28249@AcuMS.aculab.com>

On Fri, Nov 25, 2022 at 03:27:07PM +0000, David Laight wrote:
> From: Andy Shevchenko
> > Sent: 25 November 2022 15:21
> > On Fri, Nov 25, 2022 at 03:00:40PM +0000, David Laight wrote:
> > > The min() and max() defines include a type check to avoid the unexpected
> > >   behaviour when a negative value is compared against and unsigned value.
> > > However a lot of code hits this check and uses min_t() to avoid the error.
> > > Many of these are just plain wrong.
> > >
> > > Those casting to u8 or u16 are particularly suspect, eg:
> > > drivers/usb/misc/usb251xb.c:528:
> > > 		hub->max_current_sp = min_t(u8, property_u32 / 2000, 50);
> > 
> > I don't buy this. What's exactly wrong with this code?
> 
> Consider what happens if propery_u32 is 512000.
> The returned value is 0 not 50.

I considered that and there are two things to consider on your side:
1) it's coming from device property;
2) device property is validated using YAML schema.

On top of that, the wrong property is on the user. We have a lot of stuff that
user may put wrongly, but it's user's choice.

Any better example, please?

-- 
With Best Regards,
Andy Shevchenko



  reply	other threads:[~2022-11-25 15:58 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-25 15:00 [PATCH 0/1] Slightly relax the type checking done by min() and max() David Laight
2022-11-25 15:17 ` Andy Shevchenko
2022-11-25 15:22   ` David Laight
2022-11-25 15:20 ` Andy Shevchenko
2022-11-25 15:27   ` David Laight
2022-11-25 15:58     ` 'Andy Shevchenko' [this message]
2022-11-25 16:14       ` David Laight
2022-11-25 17:48         ` 'Andy Shevchenko'
2022-11-25 19:47           ` David Laight
2022-11-26  0:02             ` Andrew Morton
2022-11-26 10:21               ` David Laight

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=Y4DmFfj6G6+niZ+t@smile.fi.intel.com \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=David.Laight@aculab.com \
    --cc=akpm@linux-foundation.org \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=torvalds@linux-foundation.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.