[PATCH 03/14] tty: n_tty: use 'retval' for writes' retvals

Jiri Slaby (SUSE) posted 14 patches 2 years, 4 months ago
There is a newer version of this series
[PATCH 03/14] tty: n_tty: use 'retval' for writes' retvals
Posted by Jiri Slaby (SUSE) 2 years, 4 months ago
We have a separate misnomer 'c' to hold the retuned value from
tty->ops->write(). Instead, use already defined and properly typed
'retval'.

We have another variable 'num' to serve the same purpose in the OPOST
branch. We can use this 'retval' too. But just clear it in case of
EAGAIN.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/n_tty.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index f6fa4dbdf78f..e293d87b5362 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -2335,7 +2335,6 @@ static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
 {
 	const u8 *b = buf;
 	DEFINE_WAIT_FUNC(wait, woken_wake_function);
-	int c;
 	ssize_t retval = 0;
 
 	/* Job control check -- must be done at start (POSIX.1 7.1.1.4). */
@@ -2362,15 +2361,16 @@ static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
 		}
 		if (O_OPOST(tty)) {
 			while (nr > 0) {
-				ssize_t num = process_output_block(tty, b, nr);
-				if (num < 0) {
-					if (num == -EAGAIN)
-						break;
-					retval = num;
-					goto break_out;
+				retval = process_output_block(tty, b, nr);
+				if (retval == -EAGAIN) {
+					retval = 0;
+					break;
 				}
-				b += num;
-				nr -= num;
+				if (retval < 0)
+					goto break_out;
+
+				b += retval;
+				nr -= retval;
 				if (nr == 0)
 					break;
 				if (process_output(*b, tty) < 0)
@@ -2384,16 +2384,14 @@ static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
 
 			while (nr > 0) {
 				mutex_lock(&ldata->output_lock);
-				c = tty->ops->write(tty, b, nr);
+				retval = tty->ops->write(tty, b, nr);
 				mutex_unlock(&ldata->output_lock);
-				if (c < 0) {
-					retval = c;
+				if (retval < 0)
 					goto break_out;
-				}
-				if (!c)
+				if (!retval)
 					break;
-				b += c;
-				nr -= c;
+				b += retval;
+				nr -= retval;
 			}
 		}
 		if (!nr)
-- 
2.41.0
Re: [PATCH 03/14] tty: n_tty: use 'retval' for writes' retvals
Posted by Ilpo Järvinen 2 years, 4 months ago
On Wed, 16 Aug 2023, Jiri Slaby (SUSE) wrote:

> We have a separate misnomer 'c' to hold the retuned value from
> tty->ops->write(). Instead, use already defined and properly typed
> 'retval'.
> 
> We have another variable 'num' to serve the same purpose in the OPOST
> branch. We can use this 'retval' too. But just clear it in case of
> EAGAIN.
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> ---
>  drivers/tty/n_tty.c | 30 ++++++++++++++----------------
>  1 file changed, 14 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
> index f6fa4dbdf78f..e293d87b5362 100644
> --- a/drivers/tty/n_tty.c
> +++ b/drivers/tty/n_tty.c
> @@ -2335,7 +2335,6 @@ static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
>  {
>  	const u8 *b = buf;
>  	DEFINE_WAIT_FUNC(wait, woken_wake_function);
> -	int c;
>  	ssize_t retval = 0;
>  
>  	/* Job control check -- must be done at start (POSIX.1 7.1.1.4). */
> @@ -2362,15 +2361,16 @@ static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
>  		}
>  		if (O_OPOST(tty)) {
>  			while (nr > 0) {
> -				ssize_t num = process_output_block(tty, b, nr);
> -				if (num < 0) {
> -					if (num == -EAGAIN)
> -						break;
> -					retval = num;
> -					goto break_out;
> +				retval = process_output_block(tty, b, nr);
> +				if (retval == -EAGAIN) {
> +					retval = 0;
> +					break;
>  				}
> -				b += num;
> -				nr -= num;
> +				if (retval < 0)
> +					goto break_out;
> +
> +				b += retval;
> +				nr -= retval;
>  				if (nr == 0)
>  					break;
>  				if (process_output(*b, tty) < 0)
> @@ -2384,16 +2384,14 @@ static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
>  
>  			while (nr > 0) {
>  				mutex_lock(&ldata->output_lock);
> -				c = tty->ops->write(tty, b, nr);
> +				retval = tty->ops->write(tty, b, nr);
>  				mutex_unlock(&ldata->output_lock);
> -				if (c < 0) {
> -					retval = c;
> +				if (retval < 0)
>  					goto break_out;
> -				}
> -				if (!c)
> +				if (!retval)
>  					break;
> -				b += c;
> -				nr -= c;
> +				b += retval;
> +				nr -= retval;

Type might be better but these two don't look like a major improvement... 
To me it seems obvious there exists some variable name that is better than 
c or retval for this purpose. ;-)

-- 
 i.