Re: [PATCH] TTY: Fix loss of echoed characters

[Joe Peterson]

Joe Peterson wrote:
> Andrew, attached is a patch that fixes the loss of echoed characters in
> two common cases: 1) tty output buffer is full due to lots of output and
> 2) tty is stopped.

Attached is a follow-on patch.  It just does some code style cleanup and
minor code efficiency tweaks.  Please apply it after the main patch
(probably should be combined into one patch).

						Thanks, Joe
Minor code efficiency and style cleanup.  No behavior changes.
Apply after tty-fix-loss-of-echoed-characters.patch.

Signed-off-by: Joe Peterson <joe@xxxxxxxxxxx>
---

--- linux/drivers/char/n_tty.c.orig	2008-08-20 18:19:06.000000000 -0600
+++ linux/drivers/char/n_tty.c	2008-08-25 13:41:30.000000000 -0600
@@ -446,8 +446,6 @@ static ssize_t process_output_block(stru
 		}
 	}
 break_out:
-	//if (tty->ops->flush_chars)
-	//	tty->ops->flush_chars(tty);
 	i = tty->ops->write(tty, buf, i);
 
 	unlock_kernel();
@@ -455,11 +453,11 @@ break_out:
 }
 
 /**
- *	process_echoes	-	write pending echoed characters
+ *	process_echoes	-	write pending echo characters
  *	@tty: terminal device
  *
- *	Write echoed (and other ldisc-generated) characters to the
- *	tty that have been stored previously in the echo buffer.
+ *	Write previously buffered echo (and other ldisc-generated)
+ *	characters to the tty.
  *
  *	Characters generated by the ldisc (including echoes) need to
  *	be buffered because the driver's write buffer can fill during
@@ -488,11 +486,11 @@ break_out:
 
 static void process_echoes(struct tty_struct *tty)
 {
-	int	space, num, nr;
+	int	space, nr;
 	unsigned char c;
 	unsigned char *cp, *buf_end;
 
-	if (!(num = tty->echo_cnt))
+	if (!tty->echo_cnt)
 		return;
 
 	lock_kernel();
@@ -501,7 +499,7 @@ static void process_echoes(struct tty_st
 
 	buf_end = tty->echo_buf + N_TTY_BUF_SIZE;
 	cp = tty->echo_buf + tty->echo_pos;
-	nr = num;
+	nr = tty->echo_cnt;
 	while (nr > 0) {
 		c = *cp;
 		if (c == ECHO_OP_START) {
@@ -635,14 +633,15 @@ static void process_echoes(struct tty_st
 			cp -= N_TTY_BUF_SIZE;
 	}
 
-	tty->echo_cnt = nr;
-	if (tty->echo_cnt == 0) {
+	if (nr == 0) {
 		tty->echo_pos = 0;
+		tty->echo_cnt = 0;
 		tty->echo_overrun = 0;
 	} else {
-		int num_processed = (num - nr);
+		int num_processed = tty->echo_cnt - nr;
 		tty->echo_pos += num_processed;
-		tty->echo_pos &= (N_TTY_BUF_SIZE - 1);
+		tty->echo_pos &= N_TTY_BUF_SIZE - 1;
+		tty->echo_cnt = nr;
 		if (num_processed > 0)
 			tty->echo_overrun = 0;
 	}
@@ -663,42 +662,43 @@ static void process_echoes(struct tty_st
 
 static void add_echo_byte(unsigned char c, struct tty_struct *tty)
 {
-	int 	add_char_pos;
+	int	new_byte_pos;
 	unsigned long flags;
 
 	spin_lock_irqsave(&tty->echo_lock, flags);
 
-	add_char_pos = tty->echo_pos + tty->echo_cnt;
-	if (add_char_pos >= N_TTY_BUF_SIZE)
-		add_char_pos -= N_TTY_BUF_SIZE;
-
-	/* Detect overrun */
 	if (tty->echo_cnt == N_TTY_BUF_SIZE) {
+		/* Circular buffer is already at capacity */
+		new_byte_pos = tty->echo_pos;
+
 		/*
-		 * If the start position pointer needs to be advanced
-		 * due to running out of buffer space, be sure it is
-		 * done in such a way as to remove whole
-		 * operation byte groups.
+		 * Since the buffer start position needs to be advanced,
+		 * be sure to step by a whole operation byte group.
 		 */
-		if (*(tty->echo_buf +
-		      (tty->echo_pos & (N_TTY_BUF_SIZE - 1))) == ECHO_OP_START)
+		if (tty->echo_buf[tty->echo_pos] == ECHO_OP_START)
 		{
-			if (*(tty->echo_buf +
-			      ((tty->echo_pos + 1) & (N_TTY_BUF_SIZE - 1))) == ECHO_OP_TAB_ERASE) {
+			if (tty->echo_buf[(tty->echo_pos + 1) &
+					  (N_TTY_BUF_SIZE - 1)] ==
+						ECHO_OP_TAB_ERASE) {
 				tty->echo_pos += 4;
 				tty->echo_cnt -= 3;
 			} else {
 				tty->echo_pos += 2;
 				tty->echo_cnt -= 1;
 			}
-		} else
+		} else {
 			tty->echo_pos++;
-		tty->echo_pos &= (N_TTY_BUF_SIZE - 1);
+		}
+		tty->echo_pos &= N_TTY_BUF_SIZE - 1;
+
 		tty->echo_overrun = 1;
-	} else
+	} else {
+		new_byte_pos = tty->echo_pos + tty->echo_cnt;
+		new_byte_pos &= N_TTY_BUF_SIZE - 1;
 		tty->echo_cnt++;
+	}
 
-	tty->echo_buf[add_char_pos] = c;
+	tty->echo_buf[new_byte_pos] = c;
 
 	spin_unlock_irqrestore(&tty->echo_lock, flags);
 }
@@ -762,8 +762,9 @@ static void echo_char_raw(unsigned char 
 	if (c == ECHO_OP_START) {
 		add_echo_byte(ECHO_OP_START, tty);
 		add_echo_byte(ECHO_OP_START, tty);
-	} else
+	} else {
 		add_echo_byte(c, tty);
+	}
 }
 
 /**