From: David Laight <david.laight.linux@gmail.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
Bjorn Helgaas <bhelgaas@google.com>
Subject: Re: [PATCH 25/44] drivers/pci: use min() instead of min_t()
Date: Mon, 24 Nov 2025 21:42:40 +0000 [thread overview]
Message-ID: <20251124214240.03af61f9@pumpkin> (raw)
In-Reply-To: <20251124210410.GA2708124@bhelgaas>
On Mon, 24 Nov 2025 15:04:10 -0600
Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Wed, Nov 19, 2025 at 10:41:21PM +0000, david.laight.linux@gmail.com wrote:
> > From: David Laight <david.laight.linux@gmail.com>
> >
> > min_t(unsigned int, a, b) casts an 'unsigned long' to 'unsigned int'.
> > Use min(a, b) instead as it promotes any 'unsigned int' to 'unsigned long'
> > and so cannot discard significant bits.
> >
> > In this case although pci_hotplug_bus_size is 'long' it is constrained
> > to be <= 255.
> >
> > Detected by an extra check added to min_t().
>
> I don't mind applying this, but it sure would be nice to have a hint
> at the max() and max_t() definitions about when and why to prefer one
> over the other.
When I've sorted out local commits for all the other 400 files I've
had to change to get allmodconfig to build I'm going to look at
minmax.h and checkpatch.pl.
The current bit in checkpatch that suggests transforming
min(a, (type)b) => min_t(type, a, b)
isn't even a good idea.
The latter is min((type)a, (type)b) - which isn't the same.
at least in with min(a, (type)b)) the truncation is obvious.
I think I'll try to get checkpatch to reject min_t(u8|s8|u16|s16, ...
outright - remember the values get promoted the 'int'.
Unless the code is doing something really obscure they can all be
replaced with a plain min().
Then get minmax.h to reject u8 casts when one of the values is 255
(and u16 casts with 65535) - particularly for clamp_t().
That will error a small number of files - but only a handful.
I need to find the #if that is set for a W=1 build so that I can 'error'
more of the 'dodgy' cases.
That might include all the u8|s8|u16|s16 ones.
But I'll have to leave all the u32 casts of u64 values (on 64bit) for later
(they are typically u32 and size_t).
I've not yet looked for u32 casts of u64 values (long v long long) on 32bit.
I suspect there are some real bugs there.
Maybe I'll try to get them into the W=2 build no one does :-)
I'm retired, need to find something to do ...
(apart from getting to PoGo level 80)
David
next prev parent reply other threads:[~2025-11-24 21:42 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-19 22:40 [PATCH 00/44] Change a lot of min_t() that might mask high bits david.laight.linux
2025-11-19 22:41 ` [PATCH 25/44] drivers/pci: use min() instead of min_t() david.laight.linux
2025-11-24 21:04 ` Bjorn Helgaas
2025-11-24 21:42 ` David Laight [this message]
2025-11-20 1:47 ` [PATCH 00/44] Change a lot of min_t() that might mask high bits Jakub Kicinski
2025-11-20 9:38 ` Lorenzo Stoakes
2025-11-20 14:52 ` (subset) " Jens Axboe
2025-11-24 9:49 ` Herbert Xu
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=20251124214240.03af61f9@pumpkin \
--to=david.laight.linux@gmail.com \
--cc=bhelgaas@google.com \
--cc=helgaas@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox