[PATCH 00/17] tty: small cleanups and fixes

[PATCH 00/17] tty: small cleanups and fixes

[Jiri Slaby (SUSE)]

This is a series to fix/clean up some obvious issues I revealed during
u8+size_t conversions (to be posted later).

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Jan Kara <jack@suse.com>
Cc: Laurentiu Tudor <laurentiu.tudor@nxp.com>
Cc: linux-alpha@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-usb@vger.kernel.org
Cc: Matt Turner <mattst88@gmail.com>
Cc: netdev@vger.kernel.org
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Richard Henderson <richard.henderson@linaro.org>

Jiri Slaby (SUSE) (17):
  tty: deprecate tty_write_message()
  tty: remove unneeded mbz from tiocsti()
  tty: fix tty_operations types in documentation
  tty: move locking docs out of Returns for functions in tty.h
  tty: amiserial: return from receive_chars() without goto
  tty: amiserial: use bool and rename overrun flag in receive_chars()
  tty: ehv_bytecha: use memcpy_and_pad() in local_ev_byte_channel_send()
  tty: goldfish: drop unneeded temporary variables
  tty: hso: don't emit load/unload info to the log
  tty: hso: don't initialize global serial_table
  tty: hvc_console: use flexible array for outbuf
  tty: nozomi: remove unused debugging DUMP()
  tty: srmcons: use 'buf' directly in srmcons_do_write()
  tty: srmcons: use 'count' directly in srmcons_do_write()
  tty: srmcons: make srmcons_do_write() return void
  tty: srmcons: switch need_cr to bool
  tty: srmcons: make 'str_cr' const and non-array

 arch/alpha/kernel/srmcons.c   | 29 +++++++++++++----------------
 drivers/net/usb/hso.c         | 11 -----------
 drivers/tty/amiserial.c       | 10 ++++------
 drivers/tty/ehv_bytechan.c    |  7 +++++--
 drivers/tty/goldfish.c        |  7 ++-----
 drivers/tty/hvc/hvc_console.c |  4 +---
 drivers/tty/hvc/hvc_console.h |  2 +-
 drivers/tty/nozomi.c          | 18 ------------------
 drivers/tty/tty_io.c          |  8 ++++++--
 include/linux/tty.h           | 12 +++++++-----
 include/linux/tty_driver.h    |  5 ++---
 11 files changed, 41 insertions(+), 72 deletions(-)

-- 
2.42.1

[PATCH 13/17] tty: srmcons: use 'buf' directly in srmcons_do_write()

[Jiri Slaby (SUSE)]

There is no need to have a separate iterator ('cur') through 'buf' in
srmcons_do_write(). 'buf' can be used directly which simplifies the code
a bit.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
Cc: linux-alpha@vger.kernel.org
---
 arch/alpha/kernel/srmcons.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/alpha/kernel/srmcons.c b/arch/alpha/kernel/srmcons.c
index d6139dbae4ac..b68c5af083cd 100644
--- a/arch/alpha/kernel/srmcons.c
+++ b/arch/alpha/kernel/srmcons.c
@@ -94,24 +94,23 @@ srmcons_do_write(struct tty_port *port, const char *buf, int count)
 	static char str_cr[1] = "\r";
 	long c, remaining = count;
 	srmcons_result result;
-	char *cur;
 	int need_cr;
 
-	for (cur = (char *)buf; remaining > 0; ) {
+	while (remaining > 0) {
 		need_cr = 0;
 		/* 
 		 * Break it up into reasonable size chunks to allow a chance
 		 * for input to get in
 		 */
 		for (c = 0; c < min_t(long, 128L, remaining) && !need_cr; c++)
-			if (cur[c] == '\n')
+			if (buf[c] == '\n')
 				need_cr = 1;
 		
 		while (c > 0) {
-			result.as_long = callback_puts(0, cur, c);
+			result.as_long = callback_puts(0, buf, c);
 			c -= result.bits.c;
 			remaining -= result.bits.c;
-			cur += result.bits.c;
+			buf += result.bits.c;
 
 			/*
 			 * Check for pending input iff a tty port was provided
-- 
2.42.1

[PATCH 14/17] tty: srmcons: use 'count' directly in srmcons_do_write()

[Jiri Slaby (SUSE)]

Similarly to 'buf' in the previous patch, there is no need to have a
separate counter ('remaining') in srmcons_do_write(). 'count' can be
used directly which simplifies the code a bit.

Note that the type of the current count ('c') is changed from 'long' to
'size_t' so that:
1) it is prepared for the upcoming change of 'count's type, and
2) is unsigned.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
Cc: linux-alpha@vger.kernel.org
---
 arch/alpha/kernel/srmcons.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/alpha/kernel/srmcons.c b/arch/alpha/kernel/srmcons.c
index b68c5af083cd..8025e2a882ed 100644
--- a/arch/alpha/kernel/srmcons.c
+++ b/arch/alpha/kernel/srmcons.c
@@ -92,24 +92,24 @@ static int
 srmcons_do_write(struct tty_port *port, const char *buf, int count)
 {
 	static char str_cr[1] = "\r";
-	long c, remaining = count;
+	size_t c;
 	srmcons_result result;
 	int need_cr;
 
-	while (remaining > 0) {
+	while (count > 0) {
 		need_cr = 0;
 		/* 
 		 * Break it up into reasonable size chunks to allow a chance
 		 * for input to get in
 		 */
-		for (c = 0; c < min_t(long, 128L, remaining) && !need_cr; c++)
+		for (c = 0; c < min_t(size_t, 128U, count) && !need_cr; c++)
 			if (buf[c] == '\n')
 				need_cr = 1;
 		
 		while (c > 0) {
 			result.as_long = callback_puts(0, buf, c);
 			c -= result.bits.c;
-			remaining -= result.bits.c;
+			count -= result.bits.c;
 			buf += result.bits.c;
 
 			/*
-- 
2.42.1

[PATCH 15/17] tty: srmcons: make srmcons_do_write() return void

[Jiri Slaby (SUSE)]

The return value of srmcons_do_write() is ignored as all characters are
pushed. So make srmcons_do_write() to return void.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
Cc: linux-alpha@vger.kernel.org
---
 arch/alpha/kernel/srmcons.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/alpha/kernel/srmcons.c b/arch/alpha/kernel/srmcons.c
index 8025e2a882ed..32bc098de7da 100644
--- a/arch/alpha/kernel/srmcons.c
+++ b/arch/alpha/kernel/srmcons.c
@@ -88,7 +88,7 @@ srmcons_receive_chars(struct timer_list *t)
 }
 
 /* called with callback_lock held */
-static int
+static void
 srmcons_do_write(struct tty_port *port, const char *buf, int count)
 {
 	static char str_cr[1] = "\r";
@@ -125,7 +125,6 @@ srmcons_do_write(struct tty_port *port, const char *buf, int count)
 				need_cr = 0;
 		}
 	}
-	return count;
 }
 
 static ssize_t
-- 
2.42.1

[PATCH 16/17] tty: srmcons: switch need_cr to bool

[Jiri Slaby (SUSE)]

'need_cr' is a flag, so type it properly to be a 'bool'. Move the
declaration into the loop too. That ensures the variable is initialized
properly even if the code was moved somehow.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
Cc: linux-alpha@vger.kernel.org
---
 arch/alpha/kernel/srmcons.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/alpha/kernel/srmcons.c b/arch/alpha/kernel/srmcons.c
index 32bc098de7da..c6b821afbfd3 100644
--- a/arch/alpha/kernel/srmcons.c
+++ b/arch/alpha/kernel/srmcons.c
@@ -94,17 +94,16 @@ srmcons_do_write(struct tty_port *port, const char *buf, int count)
 	static char str_cr[1] = "\r";
 	size_t c;
 	srmcons_result result;
-	int need_cr;
 
 	while (count > 0) {
-		need_cr = 0;
+		bool need_cr = false;
 		/* 
 		 * Break it up into reasonable size chunks to allow a chance
 		 * for input to get in
 		 */
 		for (c = 0; c < min_t(size_t, 128U, count) && !need_cr; c++)
 			if (buf[c] == '\n')
-				need_cr = 1;
+				need_cr = true;
 		
 		while (c > 0) {
 			result.as_long = callback_puts(0, buf, c);
@@ -122,7 +121,7 @@ srmcons_do_write(struct tty_port *port, const char *buf, int count)
 		while (need_cr) {
 			result.as_long = callback_puts(0, str_cr, 1);
 			if (result.bits.c > 0)
-				need_cr = 0;
+				need_cr = false;
 		}
 	}
 }
-- 
2.42.1

[PATCH 17/17] tty: srmcons: make 'str_cr' const and non-array

[Jiri Slaby (SUSE)]

'str_cr' contains a single character: \n. There is no need to declare it
as array. Declare it as a variable, make it const and pass a pointer to
it to callback_puts().

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
Cc: linux-alpha@vger.kernel.org
---
 arch/alpha/kernel/srmcons.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/alpha/kernel/srmcons.c b/arch/alpha/kernel/srmcons.c
index c6b821afbfd3..a6cff61706b5 100644
--- a/arch/alpha/kernel/srmcons.c
+++ b/arch/alpha/kernel/srmcons.c
@@ -91,7 +91,7 @@ srmcons_receive_chars(struct timer_list *t)
 static void
 srmcons_do_write(struct tty_port *port, const char *buf, int count)
 {
-	static char str_cr[1] = "\r";
+	static const char str_cr = '\r';
 	size_t c;
 	srmcons_result result;
 
@@ -119,7 +119,7 @@ srmcons_do_write(struct tty_port *port, const char *buf, int count)
 		}
 
 		while (need_cr) {
-			result.as_long = callback_puts(0, str_cr, 1);
+			result.as_long = callback_puts(0, &str_cr, 1);
 			if (result.bits.c > 0)
 				need_cr = false;
 		}
-- 
2.42.1

Re: [PATCH 14/17] tty: srmcons: use 'count' directly in srmcons_do_write()

[Ilpo Järvinen]

On Tue, 21 Nov 2023, Jiri Slaby (SUSE) wrote:

> Similarly to 'buf' in the previous patch, there is no need to have a
> separate counter ('remaining') in srmcons_do_write(). 'count' can be
> used directly which simplifies the code a bit.
> 
> Note that the type of the current count ('c') is changed from 'long' to
> 'size_t' so that:
> 1) it is prepared for the upcoming change of 'count's type, and
> 2) is unsigned.
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
> Cc: Matt Turner <mattst88@gmail.com>
> Cc: linux-alpha@vger.kernel.org
> ---
>  arch/alpha/kernel/srmcons.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/alpha/kernel/srmcons.c b/arch/alpha/kernel/srmcons.c
> index b68c5af083cd..8025e2a882ed 100644
> --- a/arch/alpha/kernel/srmcons.c
> +++ b/arch/alpha/kernel/srmcons.c
> @@ -92,24 +92,24 @@ static int
>  srmcons_do_write(struct tty_port *port, const char *buf, int count)
>  {
>  	static char str_cr[1] = "\r";
> -	long c, remaining = count;
> +	size_t c;
>  	srmcons_result result;
>  	int need_cr;
>  
> -	while (remaining > 0) {
> +	while (count > 0) {
>  		need_cr = 0;
>  		/* 
>  		 * Break it up into reasonable size chunks to allow a chance
>  		 * for input to get in
>  		 */
> -		for (c = 0; c < min_t(long, 128L, remaining) && !need_cr; c++)
> +		for (c = 0; c < min_t(size_t, 128U, count) && !need_cr; c++)
>  			if (buf[c] == '\n')
>  				need_cr = 1;
>  		
>  		while (c > 0) {
>  			result.as_long = callback_puts(0, buf, c);
>  			c -= result.bits.c;
> -			remaining -= result.bits.c;
> +			count -= result.bits.c;
>  			buf += result.bits.c;
>  
>  			/*
> 

The patches in the series are in pretty odd order and it was not told 
anywhere here that the return value is unused by the callers. I'd just 
reorder the patches.

-- 
 i.

Re: [PATCH 17/17] tty: srmcons: make 'str_cr' const and non-array

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 466 bytes --]

On Tue, 21 Nov 2023, Jiri Slaby (SUSE) wrote:

> 'str_cr' contains a single character: \n. There is no need to declare it

Aren't \r and \n different characters?

> -	static char str_cr[1] = "\r";
> +	static const char str_cr = '\r';

Thanks for making these cleanups.

I've reviewed all the patches in this series, so if I didn't comment a 
patch or when you address my remarks, feel free to add:

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

Re: [PATCH 13/17] tty: srmcons: use 'buf' directly in srmcons_do_write()

[Richard Henderson]

On 11/21/23 03:22, Jiri Slaby (SUSE) wrote:
> There is no need to have a separate iterator ('cur') through 'buf' in
> srmcons_do_write(). 'buf' can be used directly which simplifies the code
> a bit.
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
> Cc: Matt Turner <mattst88@gmail.com>
> Cc: linux-alpha@vger.kernel.org
> ---
>   arch/alpha/kernel/srmcons.c | 9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 15/17] tty: srmcons: make srmcons_do_write() return void

[Richard Henderson]

On 11/21/23 03:22, Jiri Slaby (SUSE) wrote:
> The return value of srmcons_do_write() is ignored as all characters are
> pushed. So make srmcons_do_write() to return void.
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
> Cc: Matt Turner <mattst88@gmail.com>
> Cc: linux-alpha@vger.kernel.org
> ---
>   arch/alpha/kernel/srmcons.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/alpha/kernel/srmcons.c b/arch/alpha/kernel/srmcons.c
> index 8025e2a882ed..32bc098de7da 100644
> --- a/arch/alpha/kernel/srmcons.c
> +++ b/arch/alpha/kernel/srmcons.c
> @@ -88,7 +88,7 @@ srmcons_receive_chars(struct timer_list *t)
>   }
>   
>   /* called with callback_lock held */
> -static int
> +static void
>   srmcons_do_write(struct tty_port *port, const char *buf, int count)
>   {
>   	static char str_cr[1] = "\r";
> @@ -125,7 +125,6 @@ srmcons_do_write(struct tty_port *port, const char *buf, int count)
>   				need_cr = 0;
>   		}
>   	}
> -	return count;
>   }
>   
>   static ssize_t

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 14/17] tty: srmcons: use 'count' directly in srmcons_do_write()

[Richard Henderson]

On 11/21/23 09:21, Ilpo Järvinen wrote:
> On Tue, 21 Nov 2023, Jiri Slaby (SUSE) wrote:
> 
>> Similarly to 'buf' in the previous patch, there is no need to have a
>> separate counter ('remaining') in srmcons_do_write(). 'count' can be
>> used directly which simplifies the code a bit.
>>
>> Note that the type of the current count ('c') is changed from 'long' to
>> 'size_t' so that:
>> 1) it is prepared for the upcoming change of 'count's type, and
>> 2) is unsigned.
>>
>> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
>> Cc: Richard Henderson <richard.henderson@linaro.org>
>> Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
>> Cc: Matt Turner <mattst88@gmail.com>
>> Cc: linux-alpha@vger.kernel.org
>> ---
>>   arch/alpha/kernel/srmcons.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/alpha/kernel/srmcons.c b/arch/alpha/kernel/srmcons.c
>> index b68c5af083cd..8025e2a882ed 100644
>> --- a/arch/alpha/kernel/srmcons.c
>> +++ b/arch/alpha/kernel/srmcons.c
>> @@ -92,24 +92,24 @@ static int
>>   srmcons_do_write(struct tty_port *port, const char *buf, int count)
>>   {
>>   	static char str_cr[1] = "\r";
>> -	long c, remaining = count;
>> +	size_t c;
>>   	srmcons_result result;
>>   	int need_cr;
>>   
>> -	while (remaining > 0) {
>> +	while (count > 0) {
>>   		need_cr = 0;
>>   		/*
>>   		 * Break it up into reasonable size chunks to allow a chance
>>   		 * for input to get in
>>   		 */
>> -		for (c = 0; c < min_t(long, 128L, remaining) && !need_cr; c++)
>> +		for (c = 0; c < min_t(size_t, 128U, count) && !need_cr; c++)
>>   			if (buf[c] == '\n')
>>   				need_cr = 1;
>>   		
>>   		while (c > 0) {
>>   			result.as_long = callback_puts(0, buf, c);
>>   			c -= result.bits.c;
>> -			remaining -= result.bits.c;
>> +			count -= result.bits.c;
>>   			buf += result.bits.c;
>>   
>>   			/*
>>
> 
> The patches in the series are in pretty odd order and it was not told
> anywhere here that the return value is unused by the callers. I'd just
> reorder the patches.
> 

Agreed, patch 15 needs to be before patch 14.  With that,

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 16/17] tty: srmcons: switch need_cr to bool

[Richard Henderson]

On 11/21/23 03:22, Jiri Slaby (SUSE) wrote:
> 'need_cr' is a flag, so type it properly to be a 'bool'. Move the
> declaration into the loop too. That ensures the variable is initialized
> properly even if the code was moved somehow.
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
> Cc: Matt Turner <mattst88@gmail.com>
> Cc: linux-alpha@vger.kernel.org
> ---
>   arch/alpha/kernel/srmcons.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 17/17] tty: srmcons: make 'str_cr' const and non-array

[Richard Henderson]

On 11/21/23 03:22, Jiri Slaby (SUSE) wrote:
> 'str_cr' contains a single character: \n. There is no need to declare it

\r

> as array. Declare it as a variable, make it const and pass a pointer to
> it to callback_puts().
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
> Cc: Matt Turner <mattst88@gmail.com>
> Cc: linux-alpha@vger.kernel.org
> ---
>   arch/alpha/kernel/srmcons.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/alpha/kernel/srmcons.c b/arch/alpha/kernel/srmcons.c
> index c6b821afbfd3..a6cff61706b5 100644
> --- a/arch/alpha/kernel/srmcons.c
> +++ b/arch/alpha/kernel/srmcons.c
> @@ -91,7 +91,7 @@ srmcons_receive_chars(struct timer_list *t)
>   static void
>   srmcons_do_write(struct tty_port *port, const char *buf, int count)
>   {
> -	static char str_cr[1] = "\r";
> +	static const char str_cr = '\r';

An array of one element is fine -- what's wrong with that?
Adding const is an improvement though.


r~

Re: [PATCH 17/17] tty: srmcons: make 'str_cr' const and non-array

[Richard Henderson]

On 11/21/23 03:22, Jiri Slaby (SUSE) wrote:
> 'str_cr' contains a single character: \n. There is no need to declare it
> as array. Declare it as a variable, make it const and pass a pointer to
> it to callback_puts().
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
> Cc: Matt Turner <mattst88@gmail.com>
> Cc: linux-alpha@vger.kernel.org
> ---
>   arch/alpha/kernel/srmcons.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/alpha/kernel/srmcons.c b/arch/alpha/kernel/srmcons.c
> index c6b821afbfd3..a6cff61706b5 100644
> --- a/arch/alpha/kernel/srmcons.c
> +++ b/arch/alpha/kernel/srmcons.c
> @@ -91,7 +91,7 @@ srmcons_receive_chars(struct timer_list *t)
>   static void
>   srmcons_do_write(struct tty_port *port, const char *buf, int count)
>   {
> -	static char str_cr[1] = "\r";
> +	static const char str_cr = '\r';

Best to remove this entirely...

>   	size_t c;
>   	srmcons_result result;
>   
> @@ -119,7 +119,7 @@ srmcons_do_write(struct tty_port *port, const char *buf, int count)
>   		}
>   
>   		while (need_cr) {
> -			result.as_long = callback_puts(0, str_cr, 1);
> +			result.as_long = callback_puts(0, &str_cr, 1);

... and simply use "\r" here.

Logically it adds one '\0' of const data, but it is virtually certain that even more bytes 
of padding are present anyway.  As a string literal it will be placed into .rodata.str1.1 
and packed with other strings.


r~

Re: [PATCH 14/17] tty: srmcons: use 'count' directly in srmcons_do_write()

[Jiri Slaby]

On 21. 11. 23, 18:48, Richard Henderson wrote:
> On 11/21/23 09:21, Ilpo Järvinen wrote:
>> On Tue, 21 Nov 2023, Jiri Slaby (SUSE) wrote:
>>
>>> Similarly to 'buf' in the previous patch, there is no need to have a
>>> separate counter ('remaining') in srmcons_do_write(). 'count' can be
>>> used directly which simplifies the code a bit.
>>>
>>> Note that the type of the current count ('c') is changed from 'long' to
>>> 'size_t' so that:
>>> 1) it is prepared for the upcoming change of 'count's type, and
>>> 2) is unsigned.
>>>
>>> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
>>> Cc: Richard Henderson <richard.henderson@linaro.org>
>>> Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
>>> Cc: Matt Turner <mattst88@gmail.com>
>>> Cc: linux-alpha@vger.kernel.org
>>> ---
>>>   arch/alpha/kernel/srmcons.c | 8 ++++----
>>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/alpha/kernel/srmcons.c b/arch/alpha/kernel/srmcons.c
>>> index b68c5af083cd..8025e2a882ed 100644
>>> --- a/arch/alpha/kernel/srmcons.c
>>> +++ b/arch/alpha/kernel/srmcons.c
>>> @@ -92,24 +92,24 @@ static int
>>>   srmcons_do_write(struct tty_port *port, const char *buf, int count)
>>>   {
>>>       static char str_cr[1] = "\r";
>>> -    long c, remaining = count;
>>> +    size_t c;
>>>       srmcons_result result;
>>>       int need_cr;
>>> -    while (remaining > 0) {
>>> +    while (count > 0) {
>>>           need_cr = 0;
>>>           /*
>>>            * Break it up into reasonable size chunks to allow a chance
>>>            * for input to get in
>>>            */
>>> -        for (c = 0; c < min_t(long, 128L, remaining) && !need_cr; c++)
>>> +        for (c = 0; c < min_t(size_t, 128U, count) && !need_cr; c++)
>>>               if (buf[c] == '\n')
>>>                   need_cr = 1;
>>>
>>>           while (c > 0) {
>>>               result.as_long = callback_puts(0, buf, c);
>>>               c -= result.bits.c;
>>> -            remaining -= result.bits.c;
>>> +            count -= result.bits.c;
>>>               buf += result.bits.c;
>>>               /*
>>>
>>
>> The patches in the series are in pretty odd order and it was not told
>> anywhere here that the return value is unused by the callers. I'd just
>> reorder the patches.
>>
> 
> Agreed, patch 15 needs to be before patch 14.  With that,

Ah, sure, I reordered the three to have buf and count changes close to 
each other, but didn't realize this.

thanks,
-- 
js
suse labs

Re: [PATCH 17/17] tty: srmcons: make 'str_cr' const and non-array

[Jiri Slaby]

On 21. 11. 23, 16:28, Ilpo Järvinen wrote:
> On Tue, 21 Nov 2023, Jiri Slaby (SUSE) wrote:
> 
>> 'str_cr' contains a single character: \n. There is no need to declare it
> 
> Aren't \r and \n different characters?

Definitely, this is a thinko -- I didn't remember properly what it 
contains when writing the log. Fixed.

> 
>> -	static char str_cr[1] = "\r";
>> +	static const char str_cr = '\r';
> 
> Thanks for making these cleanups.
> 
> I've reviewed all the patches in this series, so if I didn't comment a
> patch or when you address my remarks, feel free to add:

thanks,
-- 
js
suse labs

Re: [PATCH 00/17] tty: small cleanups and fixes

[Greg KH]

On Tue, Nov 21, 2023 at 10:22:41AM +0100, Jiri Slaby (SUSE) wrote:
> This is a series to fix/clean up some obvious issues I revealed during
> u8+size_t conversions (to be posted later).

I applied most of these except the last few, as I think you were going
to reorder them, right?

thanks,

greg k-h

Re: [PATCH 00/17] tty: small cleanups and fixes

[Jiri Slaby]

On 23. 11. 23, 21:19, Greg KH wrote:
> On Tue, Nov 21, 2023 at 10:22:41AM +0100, Jiri Slaby (SUSE) wrote:
>> This is a series to fix/clean up some obvious issues I revealed during
>> u8+size_t conversions (to be posted later).
> 
> I applied most of these except the last few, as I think you were going
> to reorder them, right?

Yes, great. I will rebase and see/resend what is missing.

thanks,
-- 
js
suse labs