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().

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 drivers/pci/probe.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 0ce98e18b5a8..0f0d1b44d8c2 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -3163,8 +3163,7 @@ static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
 	 * bus number if there is room.
 	 */
 	if (bus->self && bus->self->is_hotplug_bridge) {
-		used_buses = max_t(unsigned int, available_buses,
-				   pci_hotplug_bus_size - 1);
+		used_buses = max(available_buses, pci_hotplug_bus_size - 1);
 		if (max - start < used_buses) {
 			max = start + used_buses;
 
-- 
2.39.5

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.

Applied to pci/misc for v6.19 with the following commit log:

  PCI: Use max() instead of max_t() to ease static analysis

  In this code:

    used_buses = max_t(unsigned int, available_buses,
                       pci_hotplug_bus_size - 1);

  max_t() casts the 'unsigned long' pci_hotplug_bus_size (either 32 or 64
  bits) to 'unsigned int' (32 bits) result type, so there's a potential of
  discarding significant bits.

  Instead, use max(a, b), which casts 'unsigned int' to 'unsigned long' and
  cannot discard significant bits.

  In this case, pci_hotplug_bus_size is constrained to <= 0xff by pci_setup()
  so this doesn't fix a bug, but it makes static analysis easier.

> Signed-off-by: David Laight <david.laight.linux@gmail.com>
> ---
>  drivers/pci/probe.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 0ce98e18b5a8..0f0d1b44d8c2 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -3163,8 +3163,7 @@ static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
>  	 * bus number if there is room.
>  	 */
>  	if (bus->self && bus->self->is_hotplug_bridge) {
> -		used_buses = max_t(unsigned int, available_buses,
> -				   pci_hotplug_bus_size - 1);
> +		used_buses = max(available_buses, pci_hotplug_bus_size - 1);
>  		if (max - start < used_buses) {
>  			max = start + used_buses;
>  
> -- 
> 2.39.5
>

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