serial: jsm: Use min_t instead of min
Subsystems (2)
PATCH 1/1 serial: jsm: Use min_t instead of min
Use min_t instead of min to cut down n further if needed.
--- drivers/tty/serial/jsm/jsm_neo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/tty/serial/jsm/jsm_neo.c +1 -1
@@ -350,7 +350,7 @@ static void neo_copy_data_from_uart_to_queue(struct jsm_channel *ch)* IBM pSeries platform. * 15 bytes max appears to be the magic number. */ - n =min ((u32 ) n, (u32 )12 ); + n =min_t (u32, n,12 );/* * Since we are grabbing the linestatus register, which
Signature · 1 line
2.34.1
Jiri Slaby
maintainerQuoted 12 lines · Mohammad Mahfooz
Use
min_tinstead of min to cut down n further if needed.Signed-off-by Mohammad Mahfooz <[email protected]>--- drivers/tty/serial/jsm/jsm_neo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/tty/serial/jsm/jsm_neo.c +0
-0
drivers/tty/serial/jsm/jsm_neo.c static void neo_copy_data_from_uart_to_queue(struct jsm_channel *ch) · line 353
Quoted 5 lines
drivers/tty/serial/jsm/jsm_neo.c +1
-1@@ -350,7 +350,7 @@ static void neo_copy_data_from_uart_to_queue(struct jsm_channel *ch)* IBM pSeries platform. * 15 bytes max appears to be the magic number. */ - n =min ((u32 ) n, (u32 )12 ); + n =min_t (u32, n,12 );
Nah, why is "n" signed in the first place? Could you fix that and all all those casts in min()s around instead?
Signature · 2 lines
js suse labs
PATCH 1/1 serial: jsm: Change n to unsigned int
Change n to an unsigned int and remove casts from min()s.
--- drivers/tty/serial/jsm/jsm_neo.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/tty/serial/jsm/jsm_neo.c +4 -4
@@ -277,7 +277,7 @@ static void neo_copy_data_from_uart_to_queue(struct jsm_channel *ch)int qleft =0 ;u8 linestatus =0 ;u8 error_mask =0 ; -int n =0 ; +u32 n =0 ;int total =0 ;u16 head;u16 tail;
@@ -342,15 +342,15 @@ static void neo_copy_data_from_uart_to_queue(struct jsm_channel *ch)break ;/* Make sure we don't go over the end of our queue */ - n =min (((u32 ) total), (RQUEUESIZE - (u32 ) head)); + n =min (total,RQUEUESIZE - head);/* * Cut down n even further if needed, this is to fix * a problem with memcpy_fromio() with the Neo on the * IBM pSeries platform. -* 15 bytes max appears to be the magic number. +* 12 bytes max appears to be the magic number. */ - n =min_t (u32, n,12 ); + n =min (n,12U );/* * Since we are grabbing the linestatus register, which
Signature · 1 line
2.34.1
Jiri Slaby
maintainerRe: [PATCH] serial: jsm: Change n to unsigned int
Quoted 21 lines · Mohammad Mahfooz
Change n to an unsigned int and remove casts from min()s.
Signed-off-by Mohammad Mahfooz <[email protected]>--- drivers/tty/serial/jsm/jsm_neo.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)drivers/tty/serial/jsm/jsm_neo.c +1
-1@@ -277,7 +277,7 @@ static void neo_copy_data_from_uart_to_queue(struct jsm_channel *ch)int qleft =0 ;u8 linestatus =0 ;u8 error_mask =0 ; -int n =0 ; +u32 n =0 ;int total =0 ;u16 head;u16 tail;
drivers/tty/serial/jsm/jsm_neo.c static void neo_copy_data_from_uart_to_queue(struct jsm_channel *ch) · line 345
Quoted 5 lines
drivers/tty/serial/jsm/jsm_neo.c +1
-1@@ -342,15 +342,15 @@ static void neo_copy_data_from_uart_to_queue(struct jsm_channel *ch)break ;/* Make sure we don't go over the end of our queue */ - n =min (((u32 ) total), (RQUEUESIZE - (u32 ) head)); + n =min (total,RQUEUESIZE - head);
This should warn now, right as total is signed too, right? And qleft. All should likely be unsigned. And the computation of qleft would need to be changed.
It's not as easy as this.
Quoted 13 lines
drivers/tty/serial/jsm/jsm_neo.c +2
-2/* * Cut down n even further if needed, this is to fix * a problem with memcpy_fromio() with the Neo on the * IBM pSeries platform. -* 15 bytes max appears to be the magic number. +* 12 bytes max appears to be the magic number. */ - n =min_t (u32, n,12 ); + n =min (n,12U );/* * Since we are grabbing the linestatus register, which
Signature · 2 lines
js suse labs
From: Jiri Slaby
Quoted 10 lines
Sent: 20 February 2023 11:52
Quoted 7 lines · Mohammad Mahfooz
Use
min_tinstead of min to cut down n further if needed.Signed-off-by Mohammad Mahfooz <[email protected]>--- drivers/tty/serial/jsm/jsm_neo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 13 lines
drivers/tty/serial/jsm/jsm_neo.c static void neo_copy_data_from_uart_to_queue(struct jsm_channel *ch) · line 353
Quoted 10 lines
drivers/tty/serial/jsm/jsm_neo.c +1
-1@@ -350,7 +350,7 @@ static void neo_copy_data_from_uart_to_queue(struct jsm_channel *ch)* IBM pSeries platform. * 15 bytes max appears to be the magic number. */ - n =min ((u32 ) n, (u32 )12 ); + n =min_t (u32, n,12 );Nah, why is "n" signed in the first place? Could you fix that and all all those casts in min()s around instead?
There is also a (IIRC) 'fifo_space -= 3;' in there that could also generate negatives.
I took one look at that function and ran away.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Quoted 25 lines · David Laight
From: Jiri Slaby
Quoted 10 lines
Sent: 20 February 2023 11:52
Quoted 7 lines · Mohammad Mahfooz
Use
min_tinstead of min to cut down n further if needed.Signed-off-by Mohammad Mahfooz <[email protected]>--- drivers/tty/serial/jsm/jsm_neo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)Quoted 13 lines
drivers/tty/serial/jsm/jsm_neo.c static void neo_copy_data_from_uart_to_queue(struct jsm_channel *ch) · line 353
Quoted 10 lines
drivers/tty/serial/jsm/jsm_neo.c +1
-1@@ -350,7 +350,7 @@ static void neo_copy_data_from_uart_to_queue(struct jsm_channel *ch)* IBM pSeries platform. * 15 bytes max appears to be the magic number. */ - n =min ((u32 ) n, (u32 )12 ); + n =min_t (u32, n,12 );Nah, why is "n" signed in the first place? Could you fix that and all all those casts in min()s around instead?
Quoted 4 lines
There is also a (IIRC) '
fifo_space-= 3;' in there that could also generate negatives.I took one look at that function and ran away.
min_t() casts before compare so it would never work correctly if a negative number is fed into min_t(u32, ...).
Not that negative FIFO "space" makes any sense (and should be prevent at its source to begin with).
Signature · 1 line
i.