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:17:50 +0200	[thread overview]
Message-ID: <Y4Dcnha/JYoRl4HH@smile.fi.intel.com> (raw)
In-Reply-To: <cfc6c0f0fd4c4724890be8a8397c2cbe@AcuMS.aculab.com>

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);
> 
> This patch does two changes:
> - Replace typeof(x) with typeof((x) + 0) to promote char/short to int.
> - Add an (int) cast to constants between 0 and MAX_INT so the compiler
>   doesn't promote the 'other side' of the comparison to an unsinged type.
>   If this is done the type test is arranged to always succeed.
> 
> The following can also be done (with some lateral thought):
> - Allow all comparisons where both types are signed. 
> - Allow all comparisons where both types are unsigned. 
> - Allow comparisons where the larger type is signed.
> 
> In addition most of the min_t() calls are there to compare a signed type
> (that holds a non-negative value) with an unsigned value.
> The definition:
> #define min_unsigned(x,y) min((x) + 0u + 0ull, (y) + 0u + 0ull)
> will do an unsigned comparision without 'accidentally' masking off
> any non-zero high bits.
> 
> With those extra changes there can be a 'duck shoot' on min_t().

You have an issue with your email setup, i.e. you send two independent messages
(not a chain). It probably shows that either you don't use `git send-email` for
sending patch, or you missed --thread option to it.

-- 
With Best Regards,
Andy Shevchenko



  reply	other threads:[~2022-11-25 15:18 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 [this message]
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'
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=Y4Dcnha/JYoRl4HH@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.