* [PATCH] serial: jsm: Use min_t instead of min
@ 2023-02-19 20:53 Mohammad Mahfooz
2023-02-20 11:52 ` Jiri Slaby
0 siblings, 1 reply; 6+ messages in thread
From: Mohammad Mahfooz @ 2023-02-19 20:53 UTC (permalink / raw)
To: linux-kernel
Cc: Mohammad Mahfooz, Greg Kroah-Hartman, Jiri Slaby,
Ilpo Järvinen, Johan Hovold, Alexander Vorwerk, linux-serial
Use min_t instead of min to cut down n further if needed.
Signed-off-by: Mohammad Mahfooz <mohammadmahfoozpersonal@gmail.com>
---
drivers/tty/serial/jsm/jsm_neo.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/jsm/jsm_neo.c b/drivers/tty/serial/jsm/jsm_neo.c
index 0c78f66276cd..4cce1e423b06 100644
--- a/drivers/tty/serial/jsm/jsm_neo.c
+++ b/drivers/tty/serial/jsm/jsm_neo.c
@@ -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
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] serial: jsm: Use min_t instead of min
2023-02-19 20:53 [PATCH] serial: jsm: Use min_t instead of min Mohammad Mahfooz
@ 2023-02-20 11:52 ` Jiri Slaby
2023-02-21 1:03 ` [PATCH] serial: jsm: Change n to unsigned int Mohammad Mahfooz
2023-02-21 12:48 ` [PATCH] serial: jsm: Use min_t instead of min David Laight
0 siblings, 2 replies; 6+ messages in thread
From: Jiri Slaby @ 2023-02-20 11:52 UTC (permalink / raw)
To: Mohammad Mahfooz, linux-kernel
Cc: Greg Kroah-Hartman, Ilpo Järvinen, Johan Hovold,
Alexander Vorwerk, linux-serial
On 19. 02. 23, 21:53, Mohammad Mahfooz wrote:
> Use min_t instead of min to cut down n further if needed.
>
> Signed-off-by: Mohammad Mahfooz <mohammadmahfoozpersonal@gmail.com>
> ---
> drivers/tty/serial/jsm/jsm_neo.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/jsm/jsm_neo.c b/drivers/tty/serial/jsm/jsm_neo.c
> index 0c78f66276cd..4cce1e423b06 100644
> --- a/drivers/tty/serial/jsm/jsm_neo.c
> +++ b/drivers/tty/serial/jsm/jsm_neo.c
> @@ -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?
--
js
suse labs
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] serial: jsm: Change n to unsigned int
2023-02-20 11:52 ` Jiri Slaby
@ 2023-02-21 1:03 ` Mohammad Mahfooz
2023-02-21 6:07 ` Jiri Slaby
2023-02-21 12:48 ` [PATCH] serial: jsm: Use min_t instead of min David Laight
1 sibling, 1 reply; 6+ messages in thread
From: Mohammad Mahfooz @ 2023-02-21 1:03 UTC (permalink / raw)
To: jirislaby
Cc: alexander.vorwerk, gregkh, ilpo.jarvinen, johan, linux-kernel,
linux-serial, mohammadmahfoozpersonal
Change n to an unsigned int and remove casts from min()s.
Signed-off-by: Mohammad Mahfooz <mohammadmahfoozpersonal@gmail.com>
---
drivers/tty/serial/jsm/jsm_neo.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/jsm/jsm_neo.c b/drivers/tty/serial/jsm/jsm_neo.c
index 4cce1e423b06..394121870436 100644
--- a/drivers/tty/serial/jsm/jsm_neo.c
+++ b/drivers/tty/serial/jsm/jsm_neo.c
@@ -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
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] serial: jsm: Change n to unsigned int
2023-02-21 1:03 ` [PATCH] serial: jsm: Change n to unsigned int Mohammad Mahfooz
@ 2023-02-21 6:07 ` Jiri Slaby
0 siblings, 0 replies; 6+ messages in thread
From: Jiri Slaby @ 2023-02-21 6:07 UTC (permalink / raw)
To: Mohammad Mahfooz
Cc: alexander.vorwerk, gregkh, ilpo.jarvinen, johan, linux-kernel,
linux-serial
On 21. 02. 23, 2:03, Mohammad Mahfooz wrote:
> Change n to an unsigned int and remove casts from min()s.
>
> Signed-off-by: Mohammad Mahfooz <mohammadmahfoozpersonal@gmail.com>
> ---
> drivers/tty/serial/jsm/jsm_neo.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tty/serial/jsm/jsm_neo.c b/drivers/tty/serial/jsm/jsm_neo.c
> index 4cce1e423b06..394121870436 100644
> --- a/drivers/tty/serial/jsm/jsm_neo.c
> +++ b/drivers/tty/serial/jsm/jsm_neo.c
> @@ -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);
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.
>
> /*
> * 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
--
js
suse labs
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] serial: jsm: Use min_t instead of min
2023-02-20 11:52 ` Jiri Slaby
2023-02-21 1:03 ` [PATCH] serial: jsm: Change n to unsigned int Mohammad Mahfooz
@ 2023-02-21 12:48 ` David Laight
2023-02-21 13:17 ` Ilpo Järvinen
1 sibling, 1 reply; 6+ messages in thread
From: David Laight @ 2023-02-21 12:48 UTC (permalink / raw)
To: 'Jiri Slaby', Mohammad Mahfooz,
linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman, Ilpo Järvinen, Johan Hovold,
Alexander Vorwerk, linux-serial@vger.kernel.org
From: Jiri Slaby
> Sent: 20 February 2023 11:52
>
> On 19. 02. 23, 21:53, Mohammad Mahfooz wrote:
> > Use min_t instead of min to cut down n further if needed.
> >
> > Signed-off-by: Mohammad Mahfooz <mohammadmahfoozpersonal@gmail.com>
> > ---
> > drivers/tty/serial/jsm/jsm_neo.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/serial/jsm/jsm_neo.c b/drivers/tty/serial/jsm/jsm_neo.c
> > index 0c78f66276cd..4cce1e423b06 100644
> > --- a/drivers/tty/serial/jsm/jsm_neo.c
> > +++ b/drivers/tty/serial/jsm/jsm_neo.c
> > @@ -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)
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] serial: jsm: Use min_t instead of min
2023-02-21 12:48 ` [PATCH] serial: jsm: Use min_t instead of min David Laight
@ 2023-02-21 13:17 ` Ilpo Järvinen
0 siblings, 0 replies; 6+ messages in thread
From: Ilpo Järvinen @ 2023-02-21 13:17 UTC (permalink / raw)
To: David Laight
Cc: 'Jiri Slaby', Mohammad Mahfooz,
linux-kernel@vger.kernel.org, Greg Kroah-Hartman, Johan Hovold,
Alexander Vorwerk, linux-serial@vger.kernel.org
On Tue, 21 Feb 2023, David Laight wrote:
> From: Jiri Slaby
> > Sent: 20 February 2023 11:52
> >
> > On 19. 02. 23, 21:53, Mohammad Mahfooz wrote:
> > > Use min_t instead of min to cut down n further if needed.
> > >
> > > Signed-off-by: Mohammad Mahfooz <mohammadmahfoozpersonal@gmail.com>
> > > ---
> > > drivers/tty/serial/jsm/jsm_neo.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/tty/serial/jsm/jsm_neo.c b/drivers/tty/serial/jsm/jsm_neo.c
> > > index 0c78f66276cd..4cce1e423b06 100644
> > > --- a/drivers/tty/serial/jsm/jsm_neo.c
> > > +++ b/drivers/tty/serial/jsm/jsm_neo.c
> > > @@ -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.
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).
--
i.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-02-21 13:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-19 20:53 [PATCH] serial: jsm: Use min_t instead of min Mohammad Mahfooz
2023-02-20 11:52 ` Jiri Slaby
2023-02-21 1:03 ` [PATCH] serial: jsm: Change n to unsigned int Mohammad Mahfooz
2023-02-21 6:07 ` Jiri Slaby
2023-02-21 12:48 ` [PATCH] serial: jsm: Use min_t instead of min David Laight
2023-02-21 13:17 ` Ilpo Järvinen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).