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

[Ilpo Järvinen]

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@xxxxxxxxxx>
> ---
>  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.