[PATCH 28/44] drivers/usb/storage: use min() instead of min_t()

david.laight.linux@gmail.com posted 44 patches 1 week, 5 days ago
There is a newer version of this series
[PATCH 28/44] drivers/usb/storage: use min() instead of min_t()
Posted by david.laight.linux@gmail.com 1 week, 5 days ago
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 the 'unsigned long' value is small enough that the result
is ok.

Detected by an extra check added to min_t().

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

diff --git a/drivers/usb/storage/protocol.c b/drivers/usb/storage/protocol.c
index 9033e505db7f..0cff54ad90fa 100644
--- a/drivers/usb/storage/protocol.c
+++ b/drivers/usb/storage/protocol.c
@@ -139,8 +139,7 @@ unsigned int usb_stor_access_xfer_buf(unsigned char *buffer,
 		return cnt;
 
 	while (sg_miter_next(&miter) && cnt < buflen) {
-		unsigned int len = min_t(unsigned int, miter.length,
-				buflen - cnt);
+		unsigned int len = min(miter.length, buflen - cnt);
 
 		if (dir == FROM_XFER_BUF)
 			memcpy(buffer + cnt, miter.addr, len);
-- 
2.39.5
Re: [PATCH 28/44] drivers/usb/storage: use min() instead of min_t()
Posted by Alan Stern 1 week, 4 days ago
On Wed, Nov 19, 2025 at 10:41:24PM +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 the 'unsigned long' value is small enough that the result
> is ok.
> 
> Detected by an extra check added to min_t().

In fact, min_t(T, a, b) cannot go wrong as long as all the types are 
unsigned and at least one of a, b has type T or smaller.  Of course, in 
this situation there's no reason not to simply use min().  (And if both 
a and b have types larger than T, why would someone use min_t() like 
this in the first place?)

Regardless, the patch is fine with me.

Acked-by: Alan Stern <stern@rowland.harvard.edu>

Alan Stern

> Signed-off-by: David Laight <david.laight.linux@gmail.com>
> ---
>  drivers/usb/storage/protocol.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/storage/protocol.c b/drivers/usb/storage/protocol.c
> index 9033e505db7f..0cff54ad90fa 100644
> --- a/drivers/usb/storage/protocol.c
> +++ b/drivers/usb/storage/protocol.c
> @@ -139,8 +139,7 @@ unsigned int usb_stor_access_xfer_buf(unsigned char *buffer,
>  		return cnt;
>  
>  	while (sg_miter_next(&miter) && cnt < buflen) {
> -		unsigned int len = min_t(unsigned int, miter.length,
> -				buflen - cnt);
> +		unsigned int len = min(miter.length, buflen - cnt);
>  
>  		if (dir == FROM_XFER_BUF)
>  			memcpy(buffer + cnt, miter.addr, len);
> -- 
> 2.39.5
>
Re: [PATCH 28/44] drivers/usb/storage: use min() instead of min_t()
Posted by David Laight 1 week, 4 days ago
On Wed, 19 Nov 2025 21:59:42 -0500
Alan Stern <stern@rowland.harvard.edu> wrote:

> On Wed, Nov 19, 2025 at 10:41:24PM +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 the 'unsigned long' value is small enough that the result
> > is ok.
> > 
> > Detected by an extra check added to min_t().  
> 
> In fact, min_t(T, a, b) cannot go wrong as long as all the types are 
> unsigned and at least one of a, b has type T or smaller.

That is backwards, both a and b have to have types at least as large
as T (or rather values that will fit in T).
- which is exactly what people keep getting wrong.
Consider:
	u32 a = 4;
	u64 b = 0x100000001ull;
then:
	min_t(u32, a, b)
has value 1 not 4.

	David

>  Of course, in 
> this situation there's no reason not to simply use min().  (And if both 
> a and b have types larger than T, why would someone use min_t() like 
> this in the first place?)
> 
> Regardless, the patch is fine with me.
> 
> Acked-by: Alan Stern <stern@rowland.harvard.edu>
> 
> Alan Stern
> 
> > Signed-off-by: David Laight <david.laight.linux@gmail.com>
> > ---
> >  drivers/usb/storage/protocol.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/usb/storage/protocol.c b/drivers/usb/storage/protocol.c
> > index 9033e505db7f..0cff54ad90fa 100644
> > --- a/drivers/usb/storage/protocol.c
> > +++ b/drivers/usb/storage/protocol.c
> > @@ -139,8 +139,7 @@ unsigned int usb_stor_access_xfer_buf(unsigned char *buffer,
> >  		return cnt;
> >  
> >  	while (sg_miter_next(&miter) && cnt < buflen) {
> > -		unsigned int len = min_t(unsigned int, miter.length,
> > -				buflen - cnt);
> > +		unsigned int len = min(miter.length, buflen - cnt);
> >  
> >  		if (dir == FROM_XFER_BUF)
> >  			memcpy(buffer + cnt, miter.addr, len);
> > -- 
> > 2.39.5
> >
Re: [usb-storage] Re: [PATCH 28/44] drivers/usb/storage: use min() instead of min_t()
Posted by Alan Stern 1 week, 4 days ago
On Thu, Nov 20, 2025 at 09:18:02AM +0000, David Laight wrote:
> On Wed, 19 Nov 2025 21:59:42 -0500
> Alan Stern <stern@rowland.harvard.edu> wrote:
> 
> > On Wed, Nov 19, 2025 at 10:41:24PM +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 the 'unsigned long' value is small enough that the result
> > > is ok.
> > > 
> > > Detected by an extra check added to min_t().  
> > 
> > In fact, min_t(T, a, b) cannot go wrong as long as all the types are 
> > unsigned and at least one of a, b has type T or smaller.
> 
> That is backwards, both a and b have to have types at least as large
> as T (or rather values that will fit in T).
> - which is exactly what people keep getting wrong.
> Consider:
> 	u32 a = 4;
> 	u64 b = 0x100000001ull;
> then:
> 	min_t(u32, a, b)
> has value 1 not 4.

You are right.  For some reason I was thinking that the comparison took 
place before the casts, which doesn't make any sense.

Alan Stern