[PATCH v2 00/14] tty: n_tty: cleanup

[PATCH v2 00/14] tty: n_tty: cleanup

[Jiri Slaby (SUSE)]

This is another part (say part III.) of the previous type unification
across the tty layer[1]. This time, in n_tty line discipline. Apart from
type changes, this series contains a larger set of refactoring of the
code. Namely, separating hairy code into single functions for better
readability.

[1] https://lore.kernel.org/all/20230810091510.13006-1-jirislaby@kernel.org/

Note this is completely independent on "part II." (tty_buffer cleanup),
so those two can be applied in any order.

[v2]
 * resend as I intertwined the series with and older one
 * use better name for variable in 03/14

Jiri Slaby (SUSE) (14):
  tty: n_tty: make flow of n_tty_receive_buf_common() a bool
  tty: n_tty: use output character directly
  tty: n_tty: use 'num' for writes' counts
  tty: n_tty: use time_is_before_jiffies() in n_tty_receive_overrun()
  tty: n_tty: make n_tty_data::num_overrun unsigned
  tty: n_tty: use MASK() for masking out size bits
  tty: n_tty: move canon handling to a separate function
  tty: n_tty: move newline handling to a separate function
  tty: n_tty: remove unsigned char casts from character constants
  tty: n_tty: simplify chars_in_buffer()
  tty: n_tty: use u8 for chars and flags
  tty: n_tty: unify counts to size_t
  tty: n_tty: extract ECHO_OP processing to a separate function
  tty: n_tty: deduplicate copy code in n_tty_receive_buf_real_raw()

 drivers/tty/n_tty.c | 538 +++++++++++++++++++++++---------------------
 1 file changed, 278 insertions(+), 260 deletions(-)

-- 
2.42.0

[PATCH v2 01/14] tty: n_tty: make flow of n_tty_receive_buf_common() a bool

[Jiri Slaby (SUSE)]

The 'flow' parameter of n_tty_receive_buf_common() is meant to be a
boolean value. So use bool and alter call sites accordingly.

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

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index f44f38bb412e..8b2bacb3e40d 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1665,7 +1665,7 @@ static void __receive_buf(struct tty_struct *tty, const u8 *cp, const u8 *fp,
  */
 static size_t
 n_tty_receive_buf_common(struct tty_struct *tty, const u8 *cp, const u8 *fp,
-			 int count, int flow)
+			 int count, bool flow)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 	size_t rcvd = 0;
@@ -1748,13 +1748,13 @@ n_tty_receive_buf_common(struct tty_struct *tty, const u8 *cp, const u8 *fp,
 static void n_tty_receive_buf(struct tty_struct *tty, const u8 *cp,
 			      const u8 *fp, size_t count)
 {
-	n_tty_receive_buf_common(tty, cp, fp, count, 0);
+	n_tty_receive_buf_common(tty, cp, fp, count, false);
 }
 
 static size_t n_tty_receive_buf2(struct tty_struct *tty, const u8 *cp,
 				 const u8 *fp, size_t count)
 {
-	return n_tty_receive_buf_common(tty, cp, fp, count, 1);
+	return n_tty_receive_buf_common(tty, cp, fp, count, true);
 }
 
 /**
-- 
2.42.0

[PATCH v2 02/14] tty: n_tty: use output character directly

[Jiri Slaby (SUSE)]

There is no point to use a local variable to store the character when we
can pass it directly. This assignment comes from era when we used to do
get_user(c, b). We no longer need this, so fix this.

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

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 8b2bacb3e40d..f6fa4dbdf78f 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -2373,8 +2373,7 @@ static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
 				nr -= num;
 				if (nr == 0)
 					break;
-				c = *b;
-				if (process_output(c, tty) < 0)
+				if (process_output(*b, tty) < 0)
 					break;
 				b++; nr--;
 			}
-- 
2.42.0

[PATCH v2 03/14] tty: n_tty: use 'num' for writes' counts

[Jiri Slaby (SUSE)]

We have a separate misnomer 'c' to hold the retuned value from
tty->ops->write(). Instead, use 'num' already defined on another place
(and already properly typed).

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---

Notes:
    [v2] use 'name' instead of 'retval' as Ilpo suggests

 drivers/tty/n_tty.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index f6fa4dbdf78f..7f9fee4cf7cf 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -2335,8 +2335,7 @@ 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;
+	ssize_t num, retval = 0;
 
 	/* Job control check -- must be done at start (POSIX.1 7.1.1.4). */
 	if (L_TOSTOP(tty) && file->f_op->write_iter != redirected_tty_write) {
@@ -2362,7 +2361,7 @@ 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);
+				num = process_output_block(tty, b, nr);
 				if (num < 0) {
 					if (num == -EAGAIN)
 						break;
@@ -2384,16 +2383,16 @@ 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);
+				num = tty->ops->write(tty, b, nr);
 				mutex_unlock(&ldata->output_lock);
-				if (c < 0) {
-					retval = c;
+				if (num < 0) {
+					retval = num;
 					goto break_out;
 				}
-				if (!c)
+				if (!num)
 					break;
-				b += c;
-				nr -= c;
+				b += num;
+				nr -= num;
 			}
 		}
 		if (!nr)
-- 
2.42.0

[PATCH v2 04/14] tty: n_tty: use time_is_before_jiffies() in n_tty_receive_overrun()

[Jiri Slaby (SUSE)]

The jiffies tests in n_tty_receive_overrun() are simplified ratelimiting
(without locking). We could use struct ratelimit_state and the helpers,
but to me, it occurs to be too complex for this use case.

But the code currently tests both if the time passed (the first
time_after()) and if jiffies wrapped around (the second time_after()).
time_is_before_jiffies() takes care of both, provided overrun_time is
initialized at the allocation time.

So switch to time_is_before_jiffies(), the same what ratelimiting does.

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

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 7f9fee4cf7cf..c0b23e975877 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1173,8 +1173,7 @@ static void n_tty_receive_overrun(const struct tty_struct *tty)
 	struct n_tty_data *ldata = tty->disc_data;
 
 	ldata->num_overrun++;
-	if (time_after(jiffies, ldata->overrun_time + HZ) ||
-	    time_after(ldata->overrun_time, jiffies)) {
+	if (time_is_before_jiffies(ldata->overrun_time + HZ)) {
 		tty_warn(tty, "%d input overrun(s)\n", ldata->num_overrun);
 		ldata->overrun_time = jiffies;
 		ldata->num_overrun = 0;
-- 
2.42.0

[PATCH v2 05/14] tty: n_tty: make n_tty_data::num_overrun unsigned

[Jiri Slaby (SUSE)]

n_tty_data::num_overrun is unlikely to overflow in a second. But make it
explicitly unsigned to avoid printing negative values.

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

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index c0b23e975877..7f8f6cfa8843 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -99,7 +99,7 @@ struct n_tty_data {
 
 	/* private to n_tty_receive_overrun (single-threaded) */
 	unsigned long overrun_time;
-	int num_overrun;
+	unsigned int num_overrun;
 
 	/* non-atomic */
 	bool no_room;
@@ -1174,7 +1174,7 @@ static void n_tty_receive_overrun(const struct tty_struct *tty)
 
 	ldata->num_overrun++;
 	if (time_is_before_jiffies(ldata->overrun_time + HZ)) {
-		tty_warn(tty, "%d input overrun(s)\n", ldata->num_overrun);
+		tty_warn(tty, "%u input overrun(s)\n", ldata->num_overrun);
 		ldata->overrun_time = jiffies;
 		ldata->num_overrun = 0;
 	}
-- 
2.42.0

[PATCH v2 06/14] tty: n_tty: use MASK() for masking out size bits

[Jiri Slaby (SUSE)]

In n_tty, there is already a macro to mask out top bits from ring buffer
counters. It is MASK() added some time ago. So use it more in the code
to make it more readable.

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

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 7f8f6cfa8843..07b6a013b5ab 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -138,23 +138,23 @@ static inline size_t read_cnt(struct n_tty_data *ldata)
 
 static inline unsigned char read_buf(struct n_tty_data *ldata, size_t i)
 {
-	return ldata->read_buf[i & (N_TTY_BUF_SIZE - 1)];
+	return ldata->read_buf[MASK(i)];
 }
 
 static inline unsigned char *read_buf_addr(struct n_tty_data *ldata, size_t i)
 {
-	return &ldata->read_buf[i & (N_TTY_BUF_SIZE - 1)];
+	return &ldata->read_buf[MASK(i)];
 }
 
 static inline unsigned char echo_buf(struct n_tty_data *ldata, size_t i)
 {
 	smp_rmb(); /* Matches smp_wmb() in add_echo_byte(). */
-	return ldata->echo_buf[i & (N_TTY_BUF_SIZE - 1)];
+	return ldata->echo_buf[MASK(i)];
 }
 
 static inline unsigned char *echo_buf_addr(struct n_tty_data *ldata, size_t i)
 {
-	return &ldata->echo_buf[i & (N_TTY_BUF_SIZE - 1)];
+	return &ldata->echo_buf[MASK(i)];
 }
 
 /* If we are not echoing the data, perhaps this is a secret so erase it */
@@ -1359,7 +1359,7 @@ static void n_tty_receive_char_special(struct tty_struct *tty, unsigned char c,
 				put_tty_queue(c, ldata);
 
 handle_newline:
-			set_bit(ldata->read_head & (N_TTY_BUF_SIZE - 1), ldata->read_flags);
+			set_bit(MASK(ldata->read_head), ldata->read_flags);
 			put_tty_queue(c, ldata);
 			smp_store_release(&ldata->canon_head, ldata->read_head);
 			kill_fasync(&tty->fasync, SIGIO, POLL_IN);
@@ -1505,14 +1505,14 @@ n_tty_receive_buf_real_raw(const struct tty_struct *tty, const u8 *cp,
 	struct n_tty_data *ldata = tty->disc_data;
 	size_t n, head;
 
-	head = ldata->read_head & (N_TTY_BUF_SIZE - 1);
+	head = MASK(ldata->read_head);
 	n = min_t(size_t, count, N_TTY_BUF_SIZE - head);
 	memcpy(read_buf_addr(ldata, head), cp, n);
 	ldata->read_head += n;
 	cp += n;
 	count -= n;
 
-	head = ldata->read_head & (N_TTY_BUF_SIZE - 1);
+	head = MASK(ldata->read_head);
 	n = min_t(size_t, count, N_TTY_BUF_SIZE - head);
 	memcpy(read_buf_addr(ldata, head), cp, n);
 	ldata->read_head += n;
@@ -1779,8 +1779,7 @@ static void n_tty_set_termios(struct tty_struct *tty, const struct ktermios *old
 			ldata->canon_head = ldata->read_tail;
 			ldata->push = 0;
 		} else {
-			set_bit((ldata->read_head - 1) & (N_TTY_BUF_SIZE - 1),
-				ldata->read_flags);
+			set_bit(MASK(ldata->read_head - 1), ldata->read_flags);
 			ldata->canon_head = ldata->read_head;
 			ldata->push = 1;
 		}
@@ -1941,7 +1940,7 @@ static bool copy_from_read_buf(const struct tty_struct *tty,
 	size_t n;
 	bool is_eof;
 	size_t head = smp_load_acquire(&ldata->commit_head);
-	size_t tail = ldata->read_tail & (N_TTY_BUF_SIZE - 1);
+	size_t tail = MASK(ldata->read_tail);
 
 	n = min(head - ldata->read_tail, N_TTY_BUF_SIZE - tail);
 	n = min(*nr, n);
@@ -2004,7 +2003,7 @@ static bool canon_copy_from_read_buf(const struct tty_struct *tty,
 	canon_head = smp_load_acquire(&ldata->canon_head);
 	n = min(*nr, canon_head - ldata->read_tail);
 
-	tail = ldata->read_tail & (N_TTY_BUF_SIZE - 1);
+	tail = MASK(ldata->read_tail);
 	size = min_t(size_t, tail + n, N_TTY_BUF_SIZE);
 
 	n_tty_trace("%s: nr:%zu tail:%zu n:%zu size:%zu\n",
@@ -2466,7 +2465,7 @@ static unsigned long inq_canon(struct n_tty_data *ldata)
 	nr = head - tail;
 	/* Skip EOF-chars.. */
 	while (MASK(head) != MASK(tail)) {
-		if (test_bit(tail & (N_TTY_BUF_SIZE - 1), ldata->read_flags) &&
+		if (test_bit(MASK(tail), ldata->read_flags) &&
 		    read_buf(ldata, tail) == __DISABLED_CHAR)
 			nr--;
 		tail++;
-- 
2.42.0

[PATCH v2 07/14] tty: n_tty: move canon handling to a separate function

[Jiri Slaby (SUSE)]

n_tty_receive_char_special() is already complicated enough. Split the
canon handling to a separate function: n_tty_receive_char_canon().

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

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 07b6a013b5ab..0149dc9dd0b1 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1262,6 +1262,91 @@ static bool n_tty_receive_char_flow_ctrl(struct tty_struct *tty, unsigned char c
 	return true;
 }
 
+static bool n_tty_receive_char_canon(struct tty_struct *tty, u8 c)
+{
+	struct n_tty_data *ldata = tty->disc_data;
+
+	if (c == ERASE_CHAR(tty) || c == KILL_CHAR(tty) ||
+	    (c == WERASE_CHAR(tty) && L_IEXTEN(tty))) {
+		eraser(c, tty);
+		commit_echoes(tty);
+
+		return true;
+	}
+
+	if (c == LNEXT_CHAR(tty) && L_IEXTEN(tty)) {
+		ldata->lnext = 1;
+		if (L_ECHO(tty)) {
+			finish_erasing(ldata);
+			if (L_ECHOCTL(tty)) {
+				echo_char_raw('^', ldata);
+				echo_char_raw('\b', ldata);
+				commit_echoes(tty);
+			}
+		}
+
+		return true;
+	}
+
+	if (c == REPRINT_CHAR(tty) && L_ECHO(tty) && L_IEXTEN(tty)) {
+		size_t tail = ldata->canon_head;
+
+		finish_erasing(ldata);
+		echo_char(c, tty);
+		echo_char_raw('\n', ldata);
+		while (MASK(tail) != MASK(ldata->read_head)) {
+			echo_char(read_buf(ldata, tail), tty);
+			tail++;
+		}
+		commit_echoes(tty);
+
+		return true;
+	}
+
+	if (c == '\n') {
+		if (L_ECHO(tty) || L_ECHONL(tty)) {
+			echo_char_raw('\n', ldata);
+			commit_echoes(tty);
+		}
+		goto handle_newline;
+	}
+
+	if (c == EOF_CHAR(tty)) {
+		c = __DISABLED_CHAR;
+		goto handle_newline;
+	}
+
+	if ((c == EOL_CHAR(tty)) ||
+	    (c == EOL2_CHAR(tty) && L_IEXTEN(tty))) {
+		/*
+		 * XXX are EOL_CHAR and EOL2_CHAR echoed?!?
+		 */
+		if (L_ECHO(tty)) {
+			/* Record the column of first canon char. */
+			if (ldata->canon_head == ldata->read_head)
+				echo_set_canon_col(ldata);
+			echo_char(c, tty);
+			commit_echoes(tty);
+		}
+		/*
+		 * XXX does PARMRK doubling happen for
+		 * EOL_CHAR and EOL2_CHAR?
+		 */
+		if (c == (unsigned char) '\377' && I_PARMRK(tty))
+			put_tty_queue(c, ldata);
+
+handle_newline:
+		set_bit(MASK(ldata->read_head), ldata->read_flags);
+		put_tty_queue(c, ldata);
+		smp_store_release(&ldata->canon_head, ldata->read_head);
+		kill_fasync(&tty->fasync, SIGIO, POLL_IN);
+		wake_up_interruptible_poll(&tty->read_wait, EPOLLIN | EPOLLRDNORM);
+		return true;
+	}
+
+	return false;
+}
+
 static void n_tty_receive_char_special(struct tty_struct *tty, unsigned char c,
 				       bool lookahead_done)
 {
@@ -1296,77 +1381,8 @@ static void n_tty_receive_char_special(struct tty_struct *tty, unsigned char c,
 	} else if (c == '\n' && I_INLCR(tty))
 		c = '\r';
 
-	if (ldata->icanon) {
-		if (c == ERASE_CHAR(tty) || c == KILL_CHAR(tty) ||
-		    (c == WERASE_CHAR(tty) && L_IEXTEN(tty))) {
-			eraser(c, tty);
-			commit_echoes(tty);
-			return;
-		}
-		if (c == LNEXT_CHAR(tty) && L_IEXTEN(tty)) {
-			ldata->lnext = 1;
-			if (L_ECHO(tty)) {
-				finish_erasing(ldata);
-				if (L_ECHOCTL(tty)) {
-					echo_char_raw('^', ldata);
-					echo_char_raw('\b', ldata);
-					commit_echoes(tty);
-				}
-			}
-			return;
-		}
-		if (c == REPRINT_CHAR(tty) && L_ECHO(tty) && L_IEXTEN(tty)) {
-			size_t tail = ldata->canon_head;
-
-			finish_erasing(ldata);
-			echo_char(c, tty);
-			echo_char_raw('\n', ldata);
-			while (MASK(tail) != MASK(ldata->read_head)) {
-				echo_char(read_buf(ldata, tail), tty);
-				tail++;
-			}
-			commit_echoes(tty);
-			return;
-		}
-		if (c == '\n') {
-			if (L_ECHO(tty) || L_ECHONL(tty)) {
-				echo_char_raw('\n', ldata);
-				commit_echoes(tty);
-			}
-			goto handle_newline;
-		}
-		if (c == EOF_CHAR(tty)) {
-			c = __DISABLED_CHAR;
-			goto handle_newline;
-		}
-		if ((c == EOL_CHAR(tty)) ||
-		    (c == EOL2_CHAR(tty) && L_IEXTEN(tty))) {
-			/*
-			 * XXX are EOL_CHAR and EOL2_CHAR echoed?!?
-			 */
-			if (L_ECHO(tty)) {
-				/* Record the column of first canon char. */
-				if (ldata->canon_head == ldata->read_head)
-					echo_set_canon_col(ldata);
-				echo_char(c, tty);
-				commit_echoes(tty);
-			}
-			/*
-			 * XXX does PARMRK doubling happen for
-			 * EOL_CHAR and EOL2_CHAR?
-			 */
-			if (c == (unsigned char) '\377' && I_PARMRK(tty))
-				put_tty_queue(c, ldata);
-
-handle_newline:
-			set_bit(MASK(ldata->read_head), ldata->read_flags);
-			put_tty_queue(c, ldata);
-			smp_store_release(&ldata->canon_head, ldata->read_head);
-			kill_fasync(&tty->fasync, SIGIO, POLL_IN);
-			wake_up_interruptible_poll(&tty->read_wait, EPOLLIN | EPOLLRDNORM);
-			return;
-		}
-	}
+	if (ldata->icanon && n_tty_receive_char_canon(tty, c))
+		return;
 
 	if (L_ECHO(tty)) {
 		finish_erasing(ldata);
-- 
2.42.0

[PATCH v2 08/14] tty: n_tty: move newline handling to a separate function

[Jiri Slaby (SUSE)]

Currently, n_tty handles the newline in a label in
n_tty_receive_char_canon(). That is invoked from two more places. Split
this code to a separate function and avoid the label in this case.

This makes the code flow more understandable.

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

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 0149dc9dd0b1..632516d7b487 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1262,6 +1262,17 @@ static bool n_tty_receive_char_flow_ctrl(struct tty_struct *tty, unsigned char c
 	return true;
 }
 
+static void n_tty_receive_handle_newline(struct tty_struct *tty, u8 c)
+{
+	struct n_tty_data *ldata = tty->disc_data;
+
+	set_bit(MASK(ldata->read_head), ldata->read_flags);
+	put_tty_queue(c, ldata);
+	smp_store_release(&ldata->canon_head, ldata->read_head);
+	kill_fasync(&tty->fasync, SIGIO, POLL_IN);
+	wake_up_interruptible_poll(&tty->read_wait, EPOLLIN | EPOLLRDNORM);
+}
+
 static bool n_tty_receive_char_canon(struct tty_struct *tty, u8 c)
 {
 	struct n_tty_data *ldata = tty->disc_data;
@@ -1308,12 +1319,16 @@ static bool n_tty_receive_char_canon(struct tty_struct *tty, u8 c)
 			echo_char_raw('\n', ldata);
 			commit_echoes(tty);
 		}
-		goto handle_newline;
+		n_tty_receive_handle_newline(tty, c);
+
+		return true;
 	}
 
 	if (c == EOF_CHAR(tty)) {
 		c = __DISABLED_CHAR;
-		goto handle_newline;
+		n_tty_receive_handle_newline(tty, c);
+
+		return true;
 	}
 
 	if ((c == EOL_CHAR(tty)) ||
@@ -1335,12 +1350,8 @@ static bool n_tty_receive_char_canon(struct tty_struct *tty, u8 c)
 		if (c == (unsigned char) '\377' && I_PARMRK(tty))
 			put_tty_queue(c, ldata);
 
-handle_newline:
-		set_bit(MASK(ldata->read_head), ldata->read_flags);
-		put_tty_queue(c, ldata);
-		smp_store_release(&ldata->canon_head, ldata->read_head);
-		kill_fasync(&tty->fasync, SIGIO, POLL_IN);
-		wake_up_interruptible_poll(&tty->read_wait, EPOLLIN | EPOLLRDNORM);
+		n_tty_receive_handle_newline(tty, c);
+
 		return true;
 	}
 
-- 
2.42.0

[PATCH v2 09/14] tty: n_tty: remove unsigned char casts from character constants

[Jiri Slaby (SUSE)]

We compile with -funsigned-char, so all character constants are already
unsigned chars. Therefore, remove superfluous casts.

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

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 632516d7b487..369f5dd9cc4b 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1347,7 +1347,7 @@ static bool n_tty_receive_char_canon(struct tty_struct *tty, u8 c)
 		 * XXX does PARMRK doubling happen for
 		 * EOL_CHAR and EOL2_CHAR?
 		 */
-		if (c == (unsigned char) '\377' && I_PARMRK(tty))
+		if (c == '\377' && I_PARMRK(tty))
 			put_tty_queue(c, ldata);
 
 		n_tty_receive_handle_newline(tty, c);
@@ -1409,7 +1409,7 @@ static void n_tty_receive_char_special(struct tty_struct *tty, unsigned char c,
 	}
 
 	/* PARMRK doubling check */
-	if (c == (unsigned char) '\377' && I_PARMRK(tty))
+	if (c == '\377' && I_PARMRK(tty))
 		put_tty_queue(c, ldata);
 
 	put_tty_queue(c, ldata);
@@ -1444,7 +1444,7 @@ static void n_tty_receive_char(struct tty_struct *tty, unsigned char c)
 		commit_echoes(tty);
 	}
 	/* PARMRK doubling check */
-	if (c == (unsigned char) '\377' && I_PARMRK(tty))
+	if (c == '\377' && I_PARMRK(tty))
 		put_tty_queue(c, ldata);
 	put_tty_queue(c, ldata);
 }
-- 
2.42.0

[PATCH v2 10/14] tty: n_tty: simplify chars_in_buffer()

[Jiri Slaby (SUSE)]

The 'if' in chars_in_buffer() is misleadingly inverted. And since the
only difference is the head used for computation, cache the head using
ternary operator. And use that in return directly.

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

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 369f5dd9cc4b..e722065b2db4 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -219,13 +219,9 @@ static void n_tty_kick_worker(const struct tty_struct *tty)
 static ssize_t chars_in_buffer(const struct tty_struct *tty)
 {
 	const struct n_tty_data *ldata = tty->disc_data;
-	ssize_t n = 0;
+	size_t head = ldata->icanon ? ldata->canon_head : ldata->commit_head;
 
-	if (!ldata->icanon)
-		n = ldata->commit_head - ldata->read_tail;
-	else
-		n = ldata->canon_head - ldata->read_tail;
-	return n;
+	return head - ldata->read_tail;
 }
 
 /**
-- 
2.42.0

[PATCH v2 11/14] tty: n_tty: use u8 for chars and flags

[Jiri Slaby (SUSE)]

Unify with the tty layer and use u8 for both chars and flags.

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

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index e722065b2db4..cf42b8b4ad2e 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -109,9 +109,9 @@ struct n_tty_data {
 	unsigned char push:1;
 
 	/* shared by producer and consumer */
-	char read_buf[N_TTY_BUF_SIZE];
+	u8 read_buf[N_TTY_BUF_SIZE];
 	DECLARE_BITMAP(read_flags, N_TTY_BUF_SIZE);
-	unsigned char echo_buf[N_TTY_BUF_SIZE];
+	u8 echo_buf[N_TTY_BUF_SIZE];
 
 	/* consumer-published */
 	size_t read_tail;
@@ -136,23 +136,23 @@ static inline size_t read_cnt(struct n_tty_data *ldata)
 	return ldata->read_head - ldata->read_tail;
 }
 
-static inline unsigned char read_buf(struct n_tty_data *ldata, size_t i)
+static inline u8 read_buf(struct n_tty_data *ldata, size_t i)
 {
 	return ldata->read_buf[MASK(i)];
 }
 
-static inline unsigned char *read_buf_addr(struct n_tty_data *ldata, size_t i)
+static inline u8 *read_buf_addr(struct n_tty_data *ldata, size_t i)
 {
 	return &ldata->read_buf[MASK(i)];
 }
 
-static inline unsigned char echo_buf(struct n_tty_data *ldata, size_t i)
+static inline u8 echo_buf(struct n_tty_data *ldata, size_t i)
 {
 	smp_rmb(); /* Matches smp_wmb() in add_echo_byte(). */
 	return ldata->echo_buf[MASK(i)];
 }
 
-static inline unsigned char *echo_buf_addr(struct n_tty_data *ldata, size_t i)
+static inline u8 *echo_buf_addr(struct n_tty_data *ldata, size_t i)
 {
 	return &ldata->echo_buf[MASK(i)];
 }
@@ -303,7 +303,7 @@ static void n_tty_check_unthrottle(struct tty_struct *tty)
  *  * n_tty_receive_buf()/producer path:
  *	caller holds non-exclusive %termios_rwsem
  */
-static inline void put_tty_queue(unsigned char c, struct n_tty_data *ldata)
+static inline void put_tty_queue(u8 c, struct n_tty_data *ldata)
 {
 	*read_buf_addr(ldata, ldata->read_head) = c;
 	ldata->read_head++;
@@ -377,7 +377,7 @@ static void n_tty_flush_buffer(struct tty_struct *tty)
  * character. We use this to correctly compute the on-screen size of the
  * character when printing.
  */
-static inline int is_utf8_continuation(unsigned char c)
+static inline int is_utf8_continuation(u8 c)
 {
 	return (c & 0xc0) == 0x80;
 }
@@ -390,7 +390,7 @@ static inline int is_utf8_continuation(unsigned char c)
  * Returns: true if the utf8 character @c is a multibyte continuation character
  * and the terminal is in unicode mode.
  */
-static inline int is_continuation(unsigned char c, const struct tty_struct *tty)
+static inline int is_continuation(u8 c, const struct tty_struct *tty)
 {
 	return I_IUTF8(tty) && is_utf8_continuation(c);
 }
@@ -414,7 +414,7 @@ static inline int is_continuation(unsigned char c, const struct tty_struct *tty)
  * Locking: should be called under the %output_lock to protect the column state
  * and space left in the buffer.
  */
-static int do_output_char(unsigned char c, struct tty_struct *tty, int space)
+static int do_output_char(u8 c, struct tty_struct *tty, int space)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 	int	spaces;
@@ -488,7 +488,7 @@ static int do_output_char(unsigned char c, struct tty_struct *tty, int space)
  * Locking: %output_lock to protect column state and space left (also, this is
  *called from n_tty_write() under the tty layer write lock).
  */
-static int process_output(unsigned char c, struct tty_struct *tty)
+static int process_output(u8 c, struct tty_struct *tty)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 	int	space, retval;
@@ -524,12 +524,12 @@ static int process_output(unsigned char c, struct tty_struct *tty)
  * called from n_tty_write() under the tty layer write lock).
  */
 static ssize_t process_output_block(struct tty_struct *tty,
-				    const unsigned char *buf, unsigned int nr)
+				    const u8 *buf, unsigned int nr)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 	int	space;
 	int	i;
-	const unsigned char *cp;
+	const u8 *cp;
 
 	mutex_lock(&ldata->output_lock);
 
@@ -542,7 +542,7 @@ static ssize_t process_output_block(struct tty_struct *tty,
 		nr = space;
 
 	for (i = 0, cp = buf; i < nr; i++, cp++) {
-		unsigned char c = *cp;
+		u8 c = *cp;
 
 		switch (c) {
 		case '\n':
@@ -609,7 +609,7 @@ static size_t __process_echoes(struct tty_struct *tty)
 	struct n_tty_data *ldata = tty->disc_data;
 	int	space, old_space;
 	size_t tail;
-	unsigned char c;
+	u8 c;
 
 	old_space = space = tty_write_room(tty);
 
@@ -617,7 +617,7 @@ static size_t __process_echoes(struct tty_struct *tty)
 	while (MASK(ldata->echo_commit) != MASK(tail)) {
 		c = echo_buf(ldata, tail);
 		if (c == ECHO_OP_START) {
-			unsigned char op;
+			u8 op;
 			bool space_left = true;
 
 			/*
@@ -818,7 +818,7 @@ static void flush_echoes(struct tty_struct *tty)
  *
  * Add a character or operation byte to the echo buffer.
  */
-static inline void add_echo_byte(unsigned char c, struct n_tty_data *ldata)
+static inline void add_echo_byte(u8 c, struct n_tty_data *ldata)
 {
 	*echo_buf_addr(ldata, ldata->echo_head) = c;
 	smp_wmb(); /* Matches smp_rmb() in echo_buf(). */
@@ -889,7 +889,7 @@ static void echo_erase_tab(unsigned int num_chars, int after_tab,
  *
  * This variant does not treat control characters specially.
  */
-static void echo_char_raw(unsigned char c, struct n_tty_data *ldata)
+static void echo_char_raw(u8 c, struct n_tty_data *ldata)
 {
 	if (c == ECHO_OP_START) {
 		add_echo_byte(ECHO_OP_START, ldata);
@@ -910,7 +910,7 @@ static void echo_char_raw(unsigned char c, struct n_tty_data *ldata)
  * This variant tags control characters to be echoed as "^X" (where X is the
  * letter representing the control char).
  */
-static void echo_char(unsigned char c, const struct tty_struct *tty)
+static void echo_char(u8 c, const struct tty_struct *tty)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 
@@ -948,7 +948,7 @@ static inline void finish_erasing(struct n_tty_data *ldata)
  * Locking: n_tty_receive_buf()/producer path:
  *	caller holds non-exclusive %termios_rwsem
  */
-static void eraser(unsigned char c, const struct tty_struct *tty)
+static void eraser(u8 c, const struct tty_struct *tty)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 	enum { ERASE, WERASE, KILL } kill_type;
@@ -1188,7 +1188,7 @@ static void n_tty_receive_overrun(const struct tty_struct *tty)
  * 	caller holds non-exclusive %termios_rwsem
  */
 static void n_tty_receive_parity_error(const struct tty_struct *tty,
-				       unsigned char c)
+				       u8 c)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 
@@ -1206,7 +1206,7 @@ static void n_tty_receive_parity_error(const struct tty_struct *tty,
 }
 
 static void
-n_tty_receive_signal_char(struct tty_struct *tty, int signal, unsigned char c)
+n_tty_receive_signal_char(struct tty_struct *tty, int signal, u8 c)
 {
 	isig(signal, tty);
 	if (I_IXON(tty))
@@ -1218,7 +1218,7 @@ n_tty_receive_signal_char(struct tty_struct *tty, int signal, unsigned char c)
 		process_echoes(tty);
 }
 
-static bool n_tty_is_char_flow_ctrl(struct tty_struct *tty, unsigned char c)
+static bool n_tty_is_char_flow_ctrl(struct tty_struct *tty, u8 c)
 {
 	return c == START_CHAR(tty) || c == STOP_CHAR(tty);
 }
@@ -1238,7 +1238,7 @@ static bool n_tty_is_char_flow_ctrl(struct tty_struct *tty, unsigned char c)
  * Returns true if @c is consumed as flow-control character, the character
  * must not be treated as normal character.
  */
-static bool n_tty_receive_char_flow_ctrl(struct tty_struct *tty, unsigned char c,
+static bool n_tty_receive_char_flow_ctrl(struct tty_struct *tty, u8 c,
 					 bool lookahead_done)
 {
 	if (!n_tty_is_char_flow_ctrl(tty, c))
@@ -1354,7 +1354,7 @@ static bool n_tty_receive_char_canon(struct tty_struct *tty, u8 c)
 	return false;
 }
 
-static void n_tty_receive_char_special(struct tty_struct *tty, unsigned char c,
+static void n_tty_receive_char_special(struct tty_struct *tty, u8 c,
 				       bool lookahead_done)
 {
 	struct n_tty_data *ldata = tty->disc_data;
@@ -1423,7 +1423,7 @@ static void n_tty_receive_char_special(struct tty_struct *tty, unsigned char c,
  *	caller holds non-exclusive %termios_rwsem
  *	publishes canon_head if canonical mode is active
  */
-static void n_tty_receive_char(struct tty_struct *tty, unsigned char c)
+static void n_tty_receive_char(struct tty_struct *tty, u8 c)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 
@@ -1445,7 +1445,7 @@ static void n_tty_receive_char(struct tty_struct *tty, unsigned char c)
 	put_tty_queue(c, ldata);
 }
 
-static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c,
+static void n_tty_receive_char_closing(struct tty_struct *tty, u8 c,
 				       bool lookahead_done)
 {
 	if (I_ISTRIP(tty))
@@ -1465,7 +1465,7 @@ static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c,
 }
 
 static void
-n_tty_receive_char_flagged(struct tty_struct *tty, unsigned char c, char flag)
+n_tty_receive_char_flagged(struct tty_struct *tty, u8 c, u8 flag)
 {
 	switch (flag) {
 	case TTY_BREAK:
@@ -1479,13 +1479,13 @@ n_tty_receive_char_flagged(struct tty_struct *tty, unsigned char c, char flag)
 		n_tty_receive_overrun(tty);
 		break;
 	default:
-		tty_err(tty, "unknown flag %d\n", flag);
+		tty_err(tty, "unknown flag %u\n", flag);
 		break;
 	}
 }
 
 static void
-n_tty_receive_char_lnext(struct tty_struct *tty, unsigned char c, char flag)
+n_tty_receive_char_lnext(struct tty_struct *tty, u8 c, u8 flag)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 
@@ -1505,7 +1505,7 @@ static void n_tty_lookahead_flow_ctrl(struct tty_struct *tty, const u8 *cp,
 				      const u8 *fp, size_t count)
 {
 	struct n_tty_data *ldata = tty->disc_data;
-	unsigned char flag = TTY_NORMAL;
+	u8 flag = TTY_NORMAL;
 
 	ldata->lookahead_count += count;
 
@@ -1562,7 +1562,7 @@ static void
 n_tty_receive_buf_closing(struct tty_struct *tty, const u8 *cp, const u8 *fp,
 			  int count, bool lookahead_done)
 {
-	char flag = TTY_NORMAL;
+	u8 flag = TTY_NORMAL;
 
 	while (count--) {
 		if (fp)
@@ -1955,7 +1955,7 @@ static inline int input_available_p(const struct tty_struct *tty, int poll)
  *		read_tail published
  */
 static bool copy_from_read_buf(const struct tty_struct *tty,
-				      unsigned char **kbp,
+				      u8 **kbp,
 				      size_t *nr)
 
 {
@@ -1968,7 +1968,7 @@ static bool copy_from_read_buf(const struct tty_struct *tty,
 	n = min(head - ldata->read_tail, N_TTY_BUF_SIZE - tail);
 	n = min(*nr, n);
 	if (n) {
-		unsigned char *from = read_buf_addr(ldata, tail);
+		u8 *from = read_buf_addr(ldata, tail);
 		memcpy(*kbp, from, n);
 		is_eof = n == 1 && *from == EOF_CHAR(tty);
 		tty_audit_add_data(tty, from, n);
@@ -2010,7 +2010,7 @@ static bool copy_from_read_buf(const struct tty_struct *tty,
  *	read_tail published
  */
 static bool canon_copy_from_read_buf(const struct tty_struct *tty,
-				     unsigned char **kbp,
+				     u8 **kbp,
 				     size_t *nr)
 {
 	struct n_tty_data *ldata = tty->disc_data;
@@ -2229,7 +2229,7 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file, u8 *kbuf,
 	while (nr) {
 		/* First test for status change. */
 		if (packet && tty->link->ctrl.pktstatus) {
-			unsigned char cs;
+			u8 cs;
 			if (kb != kbuf)
 				break;
 			spin_lock_irq(&tty->link->ctrl.lock);
-- 
2.42.0

[PATCH v2 12/14] tty: n_tty: unify counts to size_t

[Jiri Slaby (SUSE)]

Some count types are already 'size_t' for a long time. Some were
switched to 'size_t' recently. Unify the rest with those now.

This allows for some min_t()s to become min()s. And make one min()
an explicit min_t() as we are comparing signed 'room' to unsigned
'count'.

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

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index cf42b8b4ad2e..bd6cd28f734f 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1523,27 +1523,27 @@ static void n_tty_lookahead_flow_ctrl(struct tty_struct *tty, const u8 *cp,
 
 static void
 n_tty_receive_buf_real_raw(const struct tty_struct *tty, const u8 *cp,
-			   int count)
+			   size_t count)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 	size_t n, head;
 
 	head = MASK(ldata->read_head);
-	n = min_t(size_t, count, N_TTY_BUF_SIZE - head);
+	n = min(count, N_TTY_BUF_SIZE - head);
 	memcpy(read_buf_addr(ldata, head), cp, n);
 	ldata->read_head += n;
 	cp += n;
 	count -= n;
 
 	head = MASK(ldata->read_head);
-	n = min_t(size_t, count, N_TTY_BUF_SIZE - head);
+	n = min(count, N_TTY_BUF_SIZE - head);
 	memcpy(read_buf_addr(ldata, head), cp, n);
 	ldata->read_head += n;
 }
 
 static void
 n_tty_receive_buf_raw(struct tty_struct *tty, const u8 *cp, const u8 *fp,
-		      int count)
+		      size_t count)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 	u8 flag = TTY_NORMAL;
@@ -1560,7 +1560,7 @@ n_tty_receive_buf_raw(struct tty_struct *tty, const u8 *cp, const u8 *fp,
 
 static void
 n_tty_receive_buf_closing(struct tty_struct *tty, const u8 *cp, const u8 *fp,
-			  int count, bool lookahead_done)
+			  size_t count, bool lookahead_done)
 {
 	u8 flag = TTY_NORMAL;
 
@@ -1573,7 +1573,7 @@ n_tty_receive_buf_closing(struct tty_struct *tty, const u8 *cp, const u8 *fp,
 }
 
 static void n_tty_receive_buf_standard(struct tty_struct *tty, const u8 *cp,
-				       const u8 *fp, int count,
+				       const u8 *fp, size_t count,
 				       bool lookahead_done)
 {
 	struct n_tty_data *ldata = tty->disc_data;
@@ -1612,11 +1612,11 @@ static void n_tty_receive_buf_standard(struct tty_struct *tty, const u8 *cp,
 }
 
 static void __receive_buf(struct tty_struct *tty, const u8 *cp, const u8 *fp,
-			  int count)
+			  size_t count)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 	bool preops = I_ISTRIP(tty) || (I_IUCLC(tty) && L_IEXTEN(tty));
-	size_t la_count = min_t(size_t, ldata->lookahead_count, count);
+	size_t la_count = min(ldata->lookahead_count, count);
 
 	if (ldata->real_raw)
 		n_tty_receive_buf_real_raw(tty, cp, count);
@@ -1687,11 +1687,11 @@ static void __receive_buf(struct tty_struct *tty, const u8 *cp, const u8 *fp,
  */
 static size_t
 n_tty_receive_buf_common(struct tty_struct *tty, const u8 *cp, const u8 *fp,
-			 int count, bool flow)
+			 size_t count, bool flow)
 {
 	struct n_tty_data *ldata = tty->disc_data;
-	size_t rcvd = 0;
-	int room, n, overflow;
+	size_t n, rcvd = 0;
+	int room, overflow;
 
 	down_read(&tty->termios_rwsem);
 
@@ -1724,7 +1724,7 @@ n_tty_receive_buf_common(struct tty_struct *tty, const u8 *cp, const u8 *fp,
 		} else
 			overflow = 0;
 
-		n = min(count, room);
+		n = min_t(size_t, count, room);
 		if (!n)
 			break;
 
@@ -1954,9 +1954,8 @@ static inline int input_available_p(const struct tty_struct *tty, int poll)
  *		caller holds non-exclusive %termios_rwsem;
  *		read_tail published
  */
-static bool copy_from_read_buf(const struct tty_struct *tty,
-				      u8 **kbp,
-				      size_t *nr)
+static bool copy_from_read_buf(const struct tty_struct *tty, u8 **kbp,
+			       size_t *nr)
 
 {
 	struct n_tty_data *ldata = tty->disc_data;
@@ -2009,8 +2008,7 @@ static bool copy_from_read_buf(const struct tty_struct *tty,
  *	caller holds non-exclusive %termios_rwsem;
  *	read_tail published
  */
-static bool canon_copy_from_read_buf(const struct tty_struct *tty,
-				     u8 **kbp,
+static bool canon_copy_from_read_buf(const struct tty_struct *tty, u8 **kbp,
 				     size_t *nr)
 {
 	struct n_tty_data *ldata = tty->disc_data;
-- 
2.42.0

[PATCH v2 13/14] tty: n_tty: extract ECHO_OP processing to a separate function

[Jiri Slaby (SUSE)]

__process_echoes() contains ECHO_OPs processing. It is stuffed in a
while loop and the whole function is barely readable. Separate it to a
new function: n_tty_process_echo_ops().

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

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index bd6cd28f734f..edf59f6fc669 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -582,6 +582,100 @@ static ssize_t process_output_block(struct tty_struct *tty,
 	return i;
 }
 
+static int n_tty_process_echo_ops(struct tty_struct *tty, size_t *tail,
+				  int space)
+{
+	struct n_tty_data *ldata = tty->disc_data;
+	u8 op;
+
+	/*
+	 * Since add_echo_byte() is called without holding output_lock, we
+	 * might see only portion of multi-byte operation.
+	 */
+	if (MASK(ldata->echo_commit) == MASK(*tail + 1))
+		return -ENODATA;
+
+	/*
+	 * If the buffer byte is the start of a multi-byte operation, get the
+	 * next byte, which is either the op code or a control character value.
+	 */
+	op = echo_buf(ldata, *tail + 1);
+
+	switch (op) {
+	case ECHO_OP_ERASE_TAB: {
+		unsigned int num_chars, num_bs;
+
+		if (MASK(ldata->echo_commit) == MASK(*tail + 2))
+			return -ENODATA;
+
+		num_chars = echo_buf(ldata, *tail + 2);
+
+		/*
+		 * Determine how many columns to go back in order to erase the
+		 * tab. This depends on the number of columns used by other
+		 * characters within the tab area. If this (modulo 8) count is
+		 * from the start of input rather than from a previous tab, we
+		 * offset by canon column. Otherwise, tab spacing is normal.
+		 */
+		if (!(num_chars & 0x80))
+			num_chars += ldata->canon_column;
+		num_bs = 8 - (num_chars & 7);
+
+		if (num_bs > space)
+			return -ENOSPC;
+
+		space -= num_bs;
+		while (num_bs--) {
+			tty_put_char(tty, '\b');
+			if (ldata->column > 0)
+				ldata->column--;
+		}
+		*tail += 3;
+		break;
+	}
+	case ECHO_OP_SET_CANON_COL:
+		ldata->canon_column = ldata->column;
+		*tail += 2;
+		break;
+
+	case ECHO_OP_MOVE_BACK_COL:
+		if (ldata->column > 0)
+			ldata->column--;
+		*tail += 2;
+		break;
+
+	case ECHO_OP_START:
+		/* This is an escaped echo op start code */
+		if (!space)
+			return -ENOSPC;
+
+		tty_put_char(tty, ECHO_OP_START);
+		ldata->column++;
+		space--;
+		*tail += 2;
+		break;
+
+	default:
+		/*
+		 * If the op is not a special byte code, it is a ctrl char
+		 * tagged to be echoed as "^X" (where X is the letter
+		 * representing the control char). Note that we must ensure
+		 * there is enough space for the whole ctrl pair.
+		 */
+		if (space < 2)
+			return -ENOSPC;
+
+		tty_put_char(tty, '^');
+		tty_put_char(tty, op ^ 0100);
+		ldata->column += 2;
+		space -= 2;
+		*tail += 2;
+		break;
+	}
+
+	return space;
+}
+
 /**
  * __process_echoes	-	write pending echo characters
  * @tty: terminal device
@@ -617,104 +711,12 @@ static size_t __process_echoes(struct tty_struct *tty)
 	while (MASK(ldata->echo_commit) != MASK(tail)) {
 		c = echo_buf(ldata, tail);
 		if (c == ECHO_OP_START) {
-			u8 op;
-			bool space_left = true;
-
-			/*
-			 * Since add_echo_byte() is called without holding
-			 * output_lock, we might see only portion of multi-byte
-			 * operation.
-			 */
-			if (MASK(ldata->echo_commit) == MASK(tail + 1))
+			int ret = n_tty_process_echo_ops(tty, &tail, space);
+			if (ret == -ENODATA)
 				goto not_yet_stored;
-			/*
-			 * If the buffer byte is the start of a multi-byte
-			 * operation, get the next byte, which is either the
-			 * op code or a control character value.
-			 */
-			op = echo_buf(ldata, tail + 1);
-
-			switch (op) {
-			case ECHO_OP_ERASE_TAB: {
-				unsigned int num_chars, num_bs;
-
-				if (MASK(ldata->echo_commit) == MASK(tail + 2))
-					goto not_yet_stored;
-				num_chars = echo_buf(ldata, tail + 2);
-
-				/*
-				 * Determine how many columns to go back
-				 * in order to erase the tab.
-				 * This depends on the number of columns
-				 * used by other characters within the tab
-				 * area.  If this (modulo 8) count is from
-				 * the start of input rather than from a
-				 * previous tab, we offset by canon column.
-				 * Otherwise, tab spacing is normal.
-				 */
-				if (!(num_chars & 0x80))
-					num_chars += ldata->canon_column;
-				num_bs = 8 - (num_chars & 7);
-
-				if (num_bs > space) {
-					space_left = false;
-					break;
-				}
-				space -= num_bs;
-				while (num_bs--) {
-					tty_put_char(tty, '\b');
-					if (ldata->column > 0)
-						ldata->column--;
-				}
-				tail += 3;
-				break;
-			}
-			case ECHO_OP_SET_CANON_COL:
-				ldata->canon_column = ldata->column;
-				tail += 2;
-				break;
-
-			case ECHO_OP_MOVE_BACK_COL:
-				if (ldata->column > 0)
-					ldata->column--;
-				tail += 2;
-				break;
-
-			case ECHO_OP_START:
-				/* This is an escaped echo op start code */
-				if (!space) {
-					space_left = false;
-					break;
-				}
-				tty_put_char(tty, ECHO_OP_START);
-				ldata->column++;
-				space--;
-				tail += 2;
-				break;
-
-			default:
-				/*
-				 * If the op is not a special byte code,
-				 * it is a ctrl char tagged to be echoed
-				 * as "^X" (where X is the letter
-				 * representing the control char).
-				 * Note that we must ensure there is
-				 * enough space for the whole ctrl pair.
-				 *
-				 */
-				if (space < 2) {
-					space_left = false;
-					break;
-				}
-				tty_put_char(tty, '^');
-				tty_put_char(tty, op ^ 0100);
-				ldata->column += 2;
-				space -= 2;
-				tail += 2;
-			}
-
-			if (!space_left)
+			if (ret < 0)
 				break;
+			space = ret;
 		} else {
 			if (O_OPOST(tty)) {
 				int retval = do_output_char(c, tty, space);
-- 
2.42.0

[PATCH v2 14/14] tty: n_tty: deduplicate copy code in n_tty_receive_buf_real_raw()

[Jiri Slaby (SUSE)]

The code is duplicated to perform the copy twice -- to handle buffer
wrap-around. Instead of the duplication, roll this into the loop.

(And add some blank lines around to have the code a bit more readable.)

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

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index edf59f6fc669..6c9a408d67cd 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1528,19 +1528,18 @@ n_tty_receive_buf_real_raw(const struct tty_struct *tty, const u8 *cp,
 			   size_t count)
 {
 	struct n_tty_data *ldata = tty->disc_data;
-	size_t n, head;
-
-	head = MASK(ldata->read_head);
-	n = min(count, N_TTY_BUF_SIZE - head);
-	memcpy(read_buf_addr(ldata, head), cp, n);
-	ldata->read_head += n;
-	cp += n;
-	count -= n;
-
-	head = MASK(ldata->read_head);
-	n = min(count, N_TTY_BUF_SIZE - head);
-	memcpy(read_buf_addr(ldata, head), cp, n);
-	ldata->read_head += n;
+
+	/* handle buffer wrap-around by a loop */
+	for (unsigned int i = 0; i < 2; i++) {
+		size_t head = MASK(ldata->read_head);
+		size_t n = min(count, N_TTY_BUF_SIZE - head);
+
+		memcpy(read_buf_addr(ldata, head), cp, n);
+
+		ldata->read_head += n;
+		cp += n;
+		count -= n;
+	}
 }
 
 static void
-- 
2.42.0