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

Jiri Slaby (SUSE) posted 17 patches 2 years ago
[PATCH 14/17] tty: srmcons: use 'count' directly in srmcons_do_write()
Posted by Jiri Slaby (SUSE) 2 years ago
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
Re: [PATCH 14/17] tty: srmcons: use 'count' directly in srmcons_do_write()
Posted by Ilpo Järvinen 2 years ago
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 14/17] tty: srmcons: use 'count' directly in srmcons_do_write()
Posted by Richard Henderson 2 years ago
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 14/17] tty: srmcons: use 'count' directly in srmcons_do_write()
Posted by Jiri Slaby 2 years ago
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