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/[email protected]/
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
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) <[email protected]>
---
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 4c2f77996ad3..11becd2dda2d 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.41.0
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) <[email protected]>
---
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 996cad23e385..0b6eeeb94920 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.41.0
'tty' is not needed in canon_skip_eof(), so we can pass 'ldata' directly
instead.
Signed-off-by: Jiri Slaby (SUSE) <[email protected]>
---
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
We compile with -funsigned-char, so all character constants are already
unsigned chars. Therefore, remove superfluous casts.
Signed-off-by: Jiri Slaby (SUSE) <[email protected]>
---
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 ab3e7c20fbef..875a2bbb51c3 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.41.0
Bah, this series intermixed with old patches in one dir.
Patches 1/4 2/4 3/4 4/4 are to be ignored.
OTOH, 01/14 ... 04/14 are a correct part of this series.
Do you want me to resend?
On 16. 08. 23, 12:58, Jiri Slaby (SUSE) wrote:
> 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/[email protected]/
>
> 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(-)
>
--
js
suse labs
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) <[email protected]>
---
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 d4d00c530c58..646c67e1547b 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.41.0
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) <[email protected]>
---
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 875a2bbb51c3..3815201c220b 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.41.0
* 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) <[email protected]>
---
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
Unify with the tty layer and use u8 for both chars and flags.
Signed-off-by: Jiri Slaby (SUSE) <[email protected]>
---
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 3815201c220b..4c2f77996ad3 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.41.0
__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) <[email protected]>
---
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 11becd2dda2d..d4d00c530c58 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.41.0
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) <[email protected]>
---
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
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) <[email protected]>
---
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.41.0