[PATCH 0/4] tty: n_tty: couple of random fixes

[PATCH 0/4] tty: n_tty: couple of random fixes

[Jiri Slaby (SUSE)]

This is a small series of minor fixes/cleanups in n_tty.

Jiri Slaby (SUSE) (4):
  n_tty: drop fp from n_tty_receive_buf_real_raw()
  n_tty: simplify and sanitize zero_buffer()
  n_tty: pass ldata to canon_skip_eof() directly
  n_tty: make many tty parameters const

 drivers/tty/n_tty.c | 48 ++++++++++++++++++++++-----------------------
 1 file changed, 23 insertions(+), 25 deletions(-)

-- 
2.41.0

[PATCH 1/4] n_tty: drop fp from n_tty_receive_buf_real_raw()

[Jiri Slaby (SUSE)]

The 'fp' parameter of n_tty_receive_buf_real_raw() is unused, so drop
it.

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 552e8a741562..1599012f20c8 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1502,7 +1502,7 @@ static void n_tty_lookahead_flow_ctrl(struct tty_struct *tty, const unsigned cha
 
 static void
 n_tty_receive_buf_real_raw(struct tty_struct *tty, const unsigned char *cp,
-			   const char *fp, int count)
+			   int count)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 	size_t n, head;
@@ -1597,7 +1597,7 @@ static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
 	size_t la_count = min_t(size_t, ldata->lookahead_count, count);
 
 	if (ldata->real_raw)
-		n_tty_receive_buf_real_raw(tty, cp, fp, count);
+		n_tty_receive_buf_real_raw(tty, cp, count);
 	else if (ldata->raw || (L_EXTPROC(tty) && !preops))
 		n_tty_receive_buf_raw(tty, cp, fp, count);
 	else if (tty->closing && !L_EXTPROC(tty)) {
-- 
2.41.0

[PATCH 2/4] n_tty: simplify and sanitize zero_buffer()

[Jiri Slaby (SUSE)]

* Make 'tty' parameter const as we only look at tty flags here.
* Make 'size' parameter of size_t type as everyone passes that and
  memset() (the consumer) expects size_t too. So be consistent.
* Remove redundant local variables, place the content directly to the
  'if'.
* Use 0 instead of 0x00 in memset(). The former is more obvious.

No functional changes expected.

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

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 1599012f20c8..c1859ae263eb 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -158,13 +158,10 @@ static inline unsigned char *echo_buf_addr(struct n_tty_data *ldata, size_t i)
 }
 
 /* If we are not echoing the data, perhaps this is a secret so erase it */
-static void zero_buffer(struct tty_struct *tty, u8 *buffer, int size)
+static void zero_buffer(const struct tty_struct *tty, u8 *buffer, size_t size)
 {
-	bool icanon = !!L_ICANON(tty);
-	bool no_echo = !L_ECHO(tty);
-
-	if (icanon && no_echo)
-		memset(buffer, 0x00, size);
+	if (L_ICANON(tty) && !L_ECHO(tty))
+		memset(buffer, 0, size);
 }
 
 static void tty_copy(struct tty_struct *tty, void *to, size_t tail, size_t n)
-- 
2.41.0

[PATCH 3/4] n_tty: pass ldata to canon_skip_eof() directly

[Jiri Slaby (SUSE)]

'tty' is not needed in canon_skip_eof(), so we can pass 'ldata' directly
instead.

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

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index c1859ae263eb..bab7005ef520 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -2053,9 +2053,8 @@ static bool canon_copy_from_read_buf(struct tty_struct *tty,
  * EOF (special EOL character that's a __DISABLED_CHAR)
  * in the stream, silently eat the EOF.
  */
-static void canon_skip_eof(struct tty_struct *tty)
+static void canon_skip_eof(struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	size_t tail, canon_head;
 
 	canon_head = smp_load_acquire(&ldata->canon_head);
@@ -2153,7 +2152,7 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
 			 * releasing the lock and returning done.
 			 */
 			if (!nr)
-				canon_skip_eof(tty);
+				canon_skip_eof(ldata);
 			else if (canon_copy_from_read_buf(tty, &kb, &nr))
 				return kb - kbuf;
 		} else {
-- 
2.41.0

[PATCH 4/4] n_tty: make many tty parameters const

[Jiri Slaby (SUSE)]

In many n_tty functions, the 'tty' parameter is used to either obtain
'ldata', or test the tty flags. So mark 'tty' in them const to make
obvious that it is only read.

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

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index bab7005ef520..0043cc84b91a 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -164,7 +164,8 @@ static void zero_buffer(const struct tty_struct *tty, u8 *buffer, size_t size)
 		memset(buffer, 0, size);
 }
 
-static void tty_copy(struct tty_struct *tty, void *to, size_t tail, size_t n)
+static void tty_copy(const struct tty_struct *tty, void *to, size_t tail,
+		     size_t n)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 	size_t size = N_TTY_BUF_SIZE - tail;
@@ -195,7 +196,7 @@ static void tty_copy(struct tty_struct *tty, void *to, size_t tail, size_t n)
  *  * n_tty_read()/consumer path:
  *	holds non-exclusive %termios_rwsem
  */
-static void n_tty_kick_worker(struct tty_struct *tty)
+static void n_tty_kick_worker(const struct tty_struct *tty)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 
@@ -215,9 +216,9 @@ static void n_tty_kick_worker(struct tty_struct *tty)
 	}
 }
 
-static ssize_t chars_in_buffer(struct tty_struct *tty)
+static ssize_t chars_in_buffer(const struct tty_struct *tty)
 {
-	struct n_tty_data *ldata = tty->disc_data;
+	const struct n_tty_data *ldata = tty->disc_data;
 	ssize_t n = 0;
 
 	if (!ldata->icanon)
@@ -393,7 +394,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, struct tty_struct *tty)
+static inline int is_continuation(unsigned char c, const struct tty_struct *tty)
 {
 	return I_IUTF8(tty) && is_utf8_continuation(c);
 }
@@ -913,7 +914,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, struct tty_struct *tty)
+static void echo_char(unsigned char c, const struct tty_struct *tty)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 
@@ -951,7 +952,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, struct tty_struct *tty)
+static void eraser(unsigned char c, const struct tty_struct *tty)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 	enum { ERASE, WERASE, KILL } kill_type;
@@ -1167,7 +1168,7 @@ static void n_tty_receive_break(struct tty_struct *tty)
  * Called from the receive_buf path so single threaded. Does not need locking
  * as num_overrun and overrun_time are function private.
  */
-static void n_tty_receive_overrun(struct tty_struct *tty)
+static void n_tty_receive_overrun(const struct tty_struct *tty)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 
@@ -1191,7 +1192,8 @@ static void n_tty_receive_overrun(struct tty_struct *tty)
  * Locking: n_tty_receive_buf()/producer path:
  * 	caller holds non-exclusive %termios_rwsem
  */
-static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c)
+static void n_tty_receive_parity_error(const struct tty_struct *tty,
+				       unsigned char c)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 
@@ -1498,8 +1500,8 @@ static void n_tty_lookahead_flow_ctrl(struct tty_struct *tty, const unsigned cha
 }
 
 static void
-n_tty_receive_buf_real_raw(struct tty_struct *tty, const unsigned char *cp,
-			   int count)
+n_tty_receive_buf_real_raw(const struct tty_struct *tty,
+			   const unsigned char *cp, int count)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 	size_t n, head;
@@ -1900,9 +1902,9 @@ static int n_tty_open(struct tty_struct *tty)
 	return 0;
 }
 
-static inline int input_available_p(struct tty_struct *tty, int poll)
+static inline int input_available_p(const struct tty_struct *tty, int poll)
 {
-	struct n_tty_data *ldata = tty->disc_data;
+	const struct n_tty_data *ldata = tty->disc_data;
 	int amt = poll && !TIME_CHAR(tty) && MIN_CHAR(tty) ? MIN_CHAR(tty) : 1;
 
 	if (ldata->icanon && !L_EXTPROC(tty))
@@ -1929,7 +1931,7 @@ static inline int input_available_p(struct tty_struct *tty, int poll)
  *		caller holds non-exclusive %termios_rwsem;
  *		read_tail published
  */
-static bool copy_from_read_buf(struct tty_struct *tty,
+static bool copy_from_read_buf(const struct tty_struct *tty,
 				      unsigned char **kbp,
 				      size_t *nr)
 
@@ -1984,7 +1986,7 @@ static bool copy_from_read_buf(struct tty_struct *tty,
  *	caller holds non-exclusive %termios_rwsem;
  *	read_tail published
  */
-static bool canon_copy_from_read_buf(struct tty_struct *tty,
+static bool canon_copy_from_read_buf(const struct tty_struct *tty,
 				     unsigned char **kbp,
 				     size_t *nr)
 {
-- 
2.41.0

[PATCH 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.

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 'retval' for writes' retvals
  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 | 551 +++++++++++++++++++++++---------------------
 1 file changed, 284 insertions(+), 267 deletions(-)

-- 
2.41.0

[PATCH 1/4] n_tty: drop fp from n_tty_receive_buf_real_raw()

[Jiri Slaby (SUSE)]

The 'fp' parameter of n_tty_receive_buf_real_raw() is unused, so drop
it.

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 552e8a741562..1599012f20c8 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1502,7 +1502,7 @@ static void n_tty_lookahead_flow_ctrl(struct tty_struct *tty, const unsigned cha
 
 static void
 n_tty_receive_buf_real_raw(struct tty_struct *tty, const unsigned char *cp,
-			   const char *fp, int count)
+			   int count)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 	size_t n, head;
@@ -1597,7 +1597,7 @@ static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
 	size_t la_count = min_t(size_t, ldata->lookahead_count, count);
 
 	if (ldata->real_raw)
-		n_tty_receive_buf_real_raw(tty, cp, fp, count);
+		n_tty_receive_buf_real_raw(tty, cp, count);
 	else if (ldata->raw || (L_EXTPROC(tty) && !preops))
 		n_tty_receive_buf_raw(tty, cp, fp, count);
 	else if (tty->closing && !L_EXTPROC(tty)) {
-- 
2.41.0